On Wed, Dec 08, 2021 at 05:39:39PM -0800, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx> > writes: > > > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > > > > This seems to be required to define `FLEX_ARRAY` in a manner that MSVC > > in C11 mode accepts. Without this fix, building Git for Windows' > > experimental FSCache code would fail thusly: > > So, what's the final verdict on this one, after a few back and forth > to clarify the "seems to be required" above? > > In the meantime, I am tempted to queue the following as a pure > clean-up. > > Thoughts? > > ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ----- > Subject: [PATCH] flex-array: simplify compiler-specific workaround > > We use "type array[];" syntax for the flex-array member at the end > of a struct under C99 or later, except when we are building with > older SUNPRO_C compilers. As we find more vendor compilers that > claim to grok C99 but not understand ts flex-array syntax, the > existing "If we are using C99, but not with these compilers..." > conditional will keep growing. > > Make it more manageable by listing vendor-specific exceptions > earlier, with the expectation that new exceptions will not be > combined into existing ones to make the condition longer, and > instead will be implemented as a new "#elif" in the cascade of > #if/#elif/#else/#endif. E.g. if MSC is found to have a quirk > similar to old SUNPRO_C, we can just add a single line > > #elif defined(_MSC_VER) > > immediately before "#elif defined(__GNUC__)" to cause us to fallback > to the safer but a bit wasteful version. > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > git-compat-util.h | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git c/git-compat-util.h w/git-compat-util.h > index c6bd2a84e5..9ba047a58e 100644 > --- c/git-compat-util.h > +++ w/git-compat-util.h > @@ -33,14 +33,23 @@ > /* > * See if our compiler is known to support flexible array members. > */ > -#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) && (!defined(__SUNPRO_C) || (__SUNPRO_C > 0x580)) > -# define FLEX_ARRAY /* empty */ > + > +/* > + * Check vendor specific quirks first, before checking the > + * __STDC_VERSION__, as vendor compilers can lie and we need to be > + * able to work them around. Note that by not defining FLEX_ARRAY > + * here, we can fall back to use the "safer but a bit wasteful" one > + * later. > + */ > +#if defined(__SUNPRO_C) && (__SUNPRO_C <= 0x580) > #elif defined(__GNUC__) > # if (__GNUC__ >= 3) > # define FLEX_ARRAY /* empty */ > # else > # define FLEX_ARRAY 0 /* older GNU extension */ > # endif > +#elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) > +# define FLEX_ARRAY /* empty */ > #endif > > /* > > Your change is an improvement that should be easier to read and avoid conflicts when adding quirks for compilers. LGTM. I hope on Dscho's side that they change the fscache code to use the union, since I don't believe the current code is C-standards compliant and at least in this case MSVC is not actually broken. -Neeraj
