Re: [PATCH] git-compat-util(msvc): C11 does not imply support for zero-sized arrays

2 messages, 2 authors, 2021-12-09 · open the first message on its own page

Re: [PATCH] git-compat-util(msvc): C11 does not imply support for zero-sized arrays

From: Junio C Hamano <hidden>
Date: 2021-12-09 01:39:46

"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:
From: Johannes Schindelin <redacted>

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 <redacted>
---
 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
 
 /*

Re: [PATCH] git-compat-util(msvc): C11 does not imply support for zero-sized arrays

From: Neeraj Singh <hidden>
Date: 2021-12-09 01:49:19

On Wed, Dec 08, 2021 at 05:39:39PM -0800, Junio C Hamano wrote:
quoted hunk
"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:
quoted
From: Johannes Schindelin <redacted>

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 <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help