| Message ID | xmqq1r15szpg.fsf_-_@gitster.g (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2b95d94b056ab3fd01f493f116381a3cd43c3433 |
| Headers | show |
| Series | Makefile: FreeBSD cannot do C99-or-below build | expand |
The approach of building with C11 on FreeBSD is a good one
compared to trying to hack around the headers.
It appears more like a compiler bug that's being worked around
here. The FreeBSD header supposedly uses a GCC extension if the
C standard version is less than C11. See:
https://github.com/freebsd/freebsd-src/blob/1e7b5f950b2d54ddb257d008592563c4d753aa54/sys/sys/cdefs.h#L317
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
On Tue, Jan 18 2022, Neeraj Singh wrote: > The approach of building with C11 on FreeBSD is a good one > compared to trying to hack around the headers. The "hack around the headers" suggests that you've seen my alternate[1].... > It appears more like a compiler bug that's being worked around > here. The FreeBSD header supposedly uses a GCC extension if the > C standard version is less than C11. See: > https://github.com/freebsd/freebsd-src/blob/1e7b5f950b2d54ddb257d008592563c4d753aa54/sys/sys/cdefs.h#L317 ...which discusses how the line you're linking to here and __has_extension() interact, it's not a compiler bug, unless I'm missing something. I.e. it's just a FreeBSD include asking the compiler the wrong question and/or not suppressing the warning (e.g. via "#pragma clang system_header"). > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Accidentally addition? 1. https://lore.kernel.org/git/patch-1.1-06cc12be94d-20220118T151234Z-avarab@gmail.com/
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Tue, Jan 18 2022, Neeraj Singh wrote: > >> The approach of building with C11 on FreeBSD is a good one >> compared to trying to hack around the headers. > > The "hack around the headers" suggests that you've seen my > alternate[1].... > >> It appears more like a compiler bug that's being worked around >> here. The FreeBSD header supposedly uses a GCC extension if the >> C standard version is less than C11. See: >> https://github.com/freebsd/freebsd-src/blob/1e7b5f950b2d54ddb257d008592563c4d753aa54/sys/sys/cdefs.h#L317 > > ...which discusses how the line you're linking to here and > __has_extension() interact, it's not a compiler bug, unless I'm missing > something. > > I.e. it's just a FreeBSD include asking the compiler the wrong question > and/or not suppressing the warning (e.g. via "#pragma clang > system_header"). Yup, I do agree with you that this looks more like a bug in the header files.
diff --git a/config.mak.dev b/config.mak.dev index d4afac6b51..3deb076d5e 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -20,9 +20,14 @@ endif endif endif +ifneq ($(uname_S),FreeBSD) ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang7,$(COMPILER_FEATURES))),) DEVELOPER_CFLAGS += -std=gnu99 endif +else +# FreeBSD cannot limit to C99 because its system headers unconditionally +# rely on C11 features. +endif DEVELOPER_CFLAGS += -Wdeclaration-after-statement DEVELOPER_CFLAGS += -Wformat-security
In "make DEVELOPER=YesPlease" builds, we try to help developers to catch as many potential issues as they can by using -Wall and turning compilation warnings into errors. In the same spirit, we recently started adding -std=gnu99 to their CFLAGS, so that they can notice when they accidentally used language features beyond C99. It however turns out that FreeBSD 13.0 mistakenly uses C11 extension in its system header files regardless of what __STDC_VERSION__ says, which means that the platform (unless we tweak their system headers) cannot be used for this purpose. It seems that -std=gnu99 is only added conditionally even in today's config.mak.dev, so it is fine if we dropped -std=gnu99 from there. Which means that developers on FreeBSD cannot participate in vetting use of features beyond C99, but there are developers on other platforms who will, so it's not too bad. We might want a more "fundamental" fix to make the platform capable of taking -std=gnu99, like working around the use of unconditional C11 extension in its system header files by supplying a set of "replacement" definitions in our header files. We chose not to pursue such an approach for two reasons at this point: (1) The fix belongs to the FreeBSD project, not this project, and such an upstream fix may happen hopefully in a not-too-distant future. (2) Fixing such a bug in system header files and working it around can lead to unexpected breakages (other parts of their system header files may not be expecting to see and do not work well with our "replacement" definitions). This close to the final release of this cycle, we have no time for that. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- config.mak.dev | 5 +++++ 1 file changed, 5 insertions(+)