Thread: Build failure with GCC 15 (defaults to -std=gnu23)
Hi,
postgres-17.1 fails to build with upcoming GCC 15 which defaults to
-std=gnu23 as follows:
```
In file included from ../../src/include/postgres_fe.h:25,
from checksum_helper.c:17:
../../src/include/c.h:456:23: error: two or more data types in declaration specifiers
456 | typedef unsigned char bool;
| ^~~~
../../src/include/c.h:456:1: warning: useless type name in empty declaration
456 | typedef unsigned char bool;
| ^~~~~~~
```
src/include/c.h does attempt to check for whether bool is already defined but
the check doesn't work.
There may be more issues afterwards but I haven't tried papering over
the above issue. It should be possible to reproduce w/
CFLAGS="... -std=gnu23" or CFLAGS="... -std=c23" on older GCC or Clang.
thanks,
sam
Sam James <sam@gentoo.org> writes:
> postgres-17.1 fails to build with upcoming GCC 15 which defaults to
> -std=gnu23 as follows:
I do not think we claim to support C23 yet.
Having said that, I can reproduce this on gcc 14 using -std=gnu23.
It appears that configure is deciding that <stdbool.h> is not
conformant to C99 because it doesn't declare "bool" as a macro.
Did C23 really remove that !?
regards, tom lane
On Mon, Nov 18, 2024 at 7:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Sam James <sam@gentoo.org> writes: > > postgres-17.1 fails to build with upcoming GCC 15 which defaults to > > -std=gnu23 as follows: > > I do not think we claim to support C23 yet. > > Having said that, I can reproduce this on gcc 14 using -std=gnu23. > It appears that configure is deciding that <stdbool.h> is not > conformant to C99 because it doesn't declare "bool" as a macro. > Did C23 really remove that !? Yes, seems to be a general pattern: features introduced as keyword _Xxx with a library macro xxx -> _Xxx (usually where xxx is already a keyword in C++ but the C committee was afraid to unleash a new keyword directly on the world, I guess?), and now xxx is finally graduating to real keyword status. Other examples: static_assert, thread_local, alignas.
Thomas Munro <thomas.munro@gmail.com> writes:
> On Mon, Nov 18, 2024 at 7:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Having said that, I can reproduce this on gcc 14 using -std=gnu23.
>> It appears that configure is deciding that <stdbool.h> is not
>> conformant to C99 because it doesn't declare "bool" as a macro.
>> Did C23 really remove that !?
> Yes, seems to be a general pattern: features introduced as keyword
> _Xxx with a library macro xxx -> _Xxx (usually where xxx is already a
> keyword in C++ but the C committee was afraid to unleash a new keyword
> directly on the world, I guess?), and now xxx is finally graduating to
> real keyword status. Other examples: static_assert, thread_local,
> alignas.
Fun. Well, now that we insist on C99 support in all branches,
I wonder whether we can just remove all the non-stdbool support.
The one thing that looks tricky is that we insist on sizeof(bool)
being 1, but are there any remaining supported platforms where
it isn't? The buildfarm doesn't have any examples.
regards, tom lane
On Mon, Nov 18, 2024 at 9:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Fun. Well, now that we insist on C99 support in all branches, > I wonder whether we can just remove all the non-stdbool support. > The one thing that looks tricky is that we insist on sizeof(bool) > being 1, but are there any remaining supported platforms where > it isn't? The buildfarm doesn't have any examples. So far I have found only Apple/Darwin PPC (RIP), where this was occasionally an issue. Some projects would apparently compile with -mone-byte-bool to unbreak local assumptions, but risk breaking ABI with other libraries (as we do?). GCC filed that switch under Darwin options rather than somewhere more general... can we call that a clue that it was highly unusual? https://gcc.gnu.org/onlinedocs/gcc/Darwin-Options.html There may be a systematic way to survey ABIs from the LLVM or GCC source trees... hmm, I am no expert so take this with a grain of salt but I found where the LLVM project defines BoolWidth and BoolAlign, starting from the commit where they removed Darwin PPC support (2fe49ea0), and it looks like it was the only target ABI that overrode the default of 8 there (it had 32, meaning bits). BTW animal "alligator" has just shown the failure. Ah, yes, due to this GCC switch being flipped a couple of days ago: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=55e3bd376b2214e200fa76d12b67ff259b06c212
Thomas Munro <thomas.munro@gmail.com> writes:
> On Mon, Nov 18, 2024 at 9:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Fun. Well, now that we insist on C99 support in all branches,
>> I wonder whether we can just remove all the non-stdbool support.
>> The one thing that looks tricky is that we insist on sizeof(bool)
>> being 1, but are there any remaining supported platforms where
>> it isn't? The buildfarm doesn't have any examples.
> So far I have found only Apple/Darwin PPC (RIP), where this was
> occasionally an issue.
Yeah. Well, what say we leave the "typedef unsigned char bool"
pathway in place, but set things up to use that only if sizeof
the stdbool type isn't 1 --- and then it's up to any hypothetical
users of that pathway to choose a compiler and compiler options
that won't choke on it.
regards, tom lane
On Mon, Nov 18, 2024 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Yeah. Well, what say we leave the "typedef unsigned char bool" > pathway in place, but set things up to use that only if sizeof > the stdbool type isn't 1 --- and then it's up to any hypothetical > users of that pathway to choose a compiler and compiler options > that won't choke on it. It sounds like we should stop using the old and broken AC_CHECK_HEADER_STDBOOL macro. I think it was doing two jobs in old times: there were some systems that shipped a defective/pre-standard stdbool.h, and some systems without it. I think both classes of system are gone from the universe. Later autoconf versions check for C99 "or later", but we're stuck with the old one and I doubt we are going to upgrade it. Found in their NEWS: *** AC_HEADER_STDBOOL, AC_CHECK_HEADER_STDBOOL are obsolescent and less picky. These macros are now obsolescent, as programs can simply include stdbool.h unconditionally. If you use these macros, they now accept a stdbool.h that exists but does nothing, so long as ‘bool’, ‘true’, and ‘false’ work anyway. This is for compatibility with C 2023 and with C++.
Thomas Munro <thomas.munro@gmail.com> writes:
> It sounds like we should stop using the old and broken
> AC_CHECK_HEADER_STDBOOL macro.
Yeah, that's what I was imagining: assume that <stdbool.h> exists
and works, and check only to see if sizeof(bool) is acceptable.
> ... Later autoconf versions check for
> C99 "or later", but we're stuck with the old one and I doubt we are
> going to upgrade it.
I'm not sure --- there was some discussion a week or two ago about
upgrading autoconf after all. But whether we do or not, it's hard
to see what AC_HEADER_STDBOOL is buying us.
regards, tom lane
Thomas Munro <thomas.munro@gmail.com> writes:
> I found a few lines we could just delete in master. I wonder if we
> should also just require sizeof(bool) == 1 more explicitly going
> forward with an error, since we don't have coverage or any expectation
> of ever getting any for the alternative code AFAICS, even if it is
> small.
Yeah, that's a fair criticism. I don't think we've tested that code
path since I retired prairiedog, so who's to say that it works even
now? Maybe it's best to just delete that code, and if we ever find a
new platform with wider bool, figure out what to do at that time.
regards, tom lane
On 18.11.24 02:30, Thomas Munro wrote: > On Mon, Nov 18, 2024 at 11:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Thomas Munro <thomas.munro@gmail.com> writes: >>> It sounds like we should stop using the old and broken >>> AC_CHECK_HEADER_STDBOOL macro. >> >> Yeah, that's what I was imagining: assume that <stdbool.h> exists >> and works, and check only to see if sizeof(bool) is acceptable. > > I think this is the minimal change, which I'd push back to 13 post-freeze. Note that if we backpatch C23 support, we also need to backpatch at least commits a67a49648d9 and d2b4b4c2259.
Peter Eisentraut <peter@eisentraut.org> writes:
> Note that if we backpatch C23 support, we also need to backpatch at
> least commits a67a49648d9 and d2b4b4c2259.
Yeah. Our normal theory for this kind of thing is "people are
likely to build our old branches with modern toolchains", so
we are going to have to back-patch C23 compatibility sooner or
later. In fact, we'll have to back-patch to 9.2, or else
decide that those branches are unbuildable on modern platforms
and hence out of scope for compatibility testing.
We have a little bit of grace time before this needs to happen,
but perhaps not very much.
regards, tom lane
On 20.11.24 16:32, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> Note that if we backpatch C23 support, we also need to backpatch at >> least commits a67a49648d9 and d2b4b4c2259. > > Yeah. Our normal theory for this kind of thing is "people are > likely to build our old branches with modern toolchains", so > we are going to have to back-patch C23 compatibility sooner or > later. In fact, we'll have to back-patch to 9.2, or else > decide that those branches are unbuildable on modern platforms > and hence out of scope for compatibility testing. I have checked that with this patch and the two above (well, one is just to remove a warning), you can get PG16 and up building cleanly with gcc-14 -std=gnu23. Before that, you get a ton of warnings and errors related to the node tree walker routines. This is presumably related to commit 1c27d16e6e5. Going further back, the bool patch proposed here assumes that stdbool.h exists unconditionally, which is C99, which is not the baseline for older branches. I think for those it's probably best to leave it alone and just use gcc-15 -std=gnu89 or whatever.