[PATCH] D157331: [clang] Implement C23 <stdckdint.h>

Thu Oct 19 10:26:50 PDT 2023

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