aaron.ballman added a comment. In D157331#4654362 <https://reviews.llvm.org/D157331#4654362>, @jrtc27 wrote:
> In D157331#4654353 <https://reviews.llvm.org/D157331#4654353>, @aaron.ballman > wrote: > >> In D157331#4654265 <https://reviews.llvm.org/D157331#4654265>, @mstorsjo >> wrote: >> >>> This change broke building a recent version of gettext. Gettext uses gnulib >>> for polyfilling various utility functions. Since not long ago, these >>> functions internally use `<stdckdint.h>`, >>> https://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/malloca.c?id=ef5a4088d9236a55283d1eb576f560aa39c09e6f. >>> If the toolchain doesn't provide the header, gnulib provides a fallback >>> version of its own: >>> https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/stdckdint.in.h >>> >>> Now since Clang provides it since this commit, gnulib doesn't provide their >>> own fallback, but goes on to use `ckd_add`. This fails with Clang's >>> `<stdckdint.h>` implementation, since gnulib isn't built in C23 mode but >>> still wants to use `<stdckdint.h>`: >>> >>> i686-w64-mingw32-clang -DHAVE_CONFIG_H -DEXEEXT=\".exe\" -I. >>> -I../../../gettext-runtime/gnulib-lib -I.. -I../intl >>> -I../../../gettext-runtime/intl -DDEPENDS_ON_LIBICONV=1 >>> -DDEPENDS_ON_LIBINTL=1 >>> -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include >>> -Wno-cast-qual -Wno-conversion -Wno-float-equal -Wno-sign-compare >>> -Wno-undef -Wno-unused-function -Wno-unused-parameter -Wno-float-conversion >>> -Wimplicit-fallthrough -Wno-pedantic -Wno-sign-conversion -Wno-type-limits >>> -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include -g -O2 -c -o >>> libgrt_a-malloca.o `test -f 'malloca.c' || echo >>> '../../../gettext-runtime/gnulib-lib/'`malloca.c >>> ../../../gettext-runtime/gnulib-lib/malloca.c:53:8: error: call to >>> undeclared function 'ckd_add'; ISO C99 and later do not support implicit >>> function declarations [-Wimplicit-function-declaration] >>> 53 | if (!ckd_add (&nplus, n, plus) && !xalloc_oversized (nplus, 1)) >>> | ^ >>> 1 error generated. >>> make: *** [Makefile:2246: libgrt_a-malloca.o] Error 1 >>> >>> It seems like GCC's version of this header exposes the functionality even >>> if compiled in an older language mode: >>> https://github.com/gcc-mirror/gcc/blob/f019251ac9be017aaf3c58f87f43d88b974d21cf/gcc/ginclude/stdckdint.h >>> >>> Would you consider changing the Clang version to do the same? Otherwise we >>> would need to convince gnulib to not try to use/polyfill this header unless >>> gnulib itself is compiled in C23 mode. >> >> Well that's a fun situation. The identifiers exposed by the header are not >> in a reserved namespace and they're macros, which is usually not something >> we'd want to expose in older language modes because of the risk of breaking >> code. But in this case, there's an explicit opt-in to a brand-new header >> file so it shouldn't break code to expose the contents in older language >> modes. But I expect there to be changes to this file in C2y, and those >> additions could break code if we're not careful. >> >> CC @jrtc27 @jyknight @efriedma for opinions from others, but my thinking is >> that we should expose the contents in earlier language modes. We support the >> builtin in those modes, and the user is explicitly asking for the header to >> be included -- it seems pretty rare for folks to want the header to be >> empty, especially given that `__has_include` will report "sure do!". I don't >> think there's a conformance issue here. > > Yes. That's also what we do for stdalign.h, which is the same case. It does, but I'd be surprised if stdalign.h got new interfaces added to it, so it's a bit different. But even still, I think we should go ahead and expose these facilities in pre-C23 modes. Thanks for the second opinion! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157331/new/ https://reviews.llvm.org/D157331 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
