[v3,2/7] malloc/obstack.h: Assume GCC supports __extension__
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
GCC supports __extension__ since at least GCC 2.8.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
malloc/obstack.h | 4 ----
1 file changed, 4 deletions(-)
Comments
On 2025-11-10 06:17, Alejandro Colomar wrote:
> -# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
> -# define __extension__
> -# endif
If we're dropping support for GCC 2.7 and earlier in public-facing
include files, this should be accompanied by an item in NEWS.
Also, there should be a section in the manual that says what user
compilers are supported. For GCC we can simply say something like "2.8
or later"; for other compilers we needn't list specific compilers or
versions, merely what C features are assumed. I suggest doing this by
uncommenting the last paragraph in maint.texi (and thus bringing back an
old compatibility appendix) and bringing it up to date. For the patches
in this series, the following features should be documented in that part
of the manual:
* #warning
* statement expressions (assuming we insist on them)
* long long and associated macros like LLONG_MAX
* a stdarg.h that defines va_list
Doing this documentation would perhaps be the most painful and important
part of the whole exercise.
On Mon, 10 Nov 2025, Alejandro Colomar wrote:
> GCC supports __extension__ since at least GCC 2.8.
This is a file shared with gnulib, but __extension__ is generally only
relevant with -pedantic; there's no problem with supporting its absence
with non-GCC compilers, so no real advantage in removing this code.
In fact, GCC 2.7 (the oldest version supporting ELF for any target
supported now by glibc) *does* support __extension__, so the more
interesting question about such a condition is whether there are specific
sub-cases of __extension__ that are used in installed headers and are the
reason for the 2.8 version here.
On Mon, 10 Nov 2025, Paul Eggert wrote:
> Also, there should be a section in the manual that says what user compilers
> are supported. For GCC we can simply say something like "2.8 or later"; for
> other compilers we needn't list specific compilers or versions, merely what C
> features are assumed. I suggest doing this by uncommenting the last paragraph
> in maint.texi (and thus bringing back an old compatibility appendix) and
> bringing it up to date. For the patches in this series, the following features
> should be documented in that part of the manual:
>
> * #warning
> * statement expressions (assuming we insist on them)
> * long long and associated macros like LLONG_MAX
> * a stdarg.h that defines va_list
>
> Doing this documentation would perhaps be the most painful and important part
> of the whole exercise.
Note there are separate considerations here:
* For users: certain language features (on top of C90 / C++98) are
required to use the headers, or certain features in the headers. In some
cases, the language feature is only needed for a very specific, limited
part of the headers (for example, <complex.h> requires __builtin_complex
for the CMPLX macros, but everything else in the installed headers should
work without that feature).
This does *not* need to mention features only relevant for optimizations
or extra diagnostics, because the relevant parts of the headers can be
conditional without affecting the API provided to users. It's only
language / compiler features that are needed to provide a particular
public API that are relevant here. On that basis, I don't see any need to
require #warning, and I doubt there's a need for statement expressions
(but maybe a detailed analysis could show some feature to be hard to
provided in the headers without them).
The interactions between compiler and library headers (e.g.
__need___va_list) are a tricky sub-part of this. I think the aim in such
cases is that the fallback, if the header doesn't support the __need_*
protocol from the GCC headers, is that the glibc headers will still work
but might expose more declarations from the compiler headers than that
particular header is meant to (e.g. __need___va_list is meant to avoid
exposing names such as va_list, just declaring __gnuc_va_list instead).
* For glibc development: there is a *rationale* for each feature for why
some things are problematic to support without that feature, typically for
ABI reasons. Documenting that makes sense as well, but for glibc
developers rather than for end users as the target audience.
Hi Paul,
On Mon, Nov 10, 2025 at 08:39:03AM -0800, Paul Eggert wrote:
> On 2025-11-10 06:17, Alejandro Colomar wrote:
> > -# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
> > -# define __extension__
> > -# endif
>
> If we're dropping support for GCC 2.7 and earlier in public-facing include
> files, this should be accompanied by an item in NEWS.
>
> Also, there should be a section in the manual that says what user compilers
> are supported. For GCC we can simply say something like "2.8 or later"; for
> other compilers we needn't list specific compilers or versions, merely what
> C features are assumed. I suggest doing this by uncommenting the last
> paragraph in maint.texi (and thus bringing back an old compatibility
> appendix) and bringing it up to date. For the patches in this series, the
> following features should be documented in that part of the manual:
>
> * #warning
I'll discard this, as if affects gnulib.
> * statement expressions (assuming we insist on them)
I'll drop this for now.
> * long long and associated macros like LLONG_MAX
Yup, I'll document this. I'll start with long long, which I guess has
less controversy.
> * a stdarg.h that defines va_list
The relevant change is:
#if defined __USE_XOPEN2K || defined __USE_XOPEN2K8
-# ifdef __GNUC__
-# ifndef _VA_LIST_DEFINED
-typedef __gnuc_va_list va_list;
-# define _VA_LIST_DEFINED
-# endif
-# else
+# ifndef __GNUC__
# include <stdarg.h>
# endif
#endif
I'm only changing the assumption if __GNUC__ is defined. This change
assumes that GCC provides va_list in its own header. However, I'm not
changing any assumptions for non-__GNUC__ compilers.
> Doing this documentation would perhaps be the most painful and important
> part of the whole exercise.
Have a lovely night!
Alex
Hi Joseph,
On Mon, Nov 10, 2025 at 06:32:38PM +0000, Joseph Myers wrote:
> On Mon, 10 Nov 2025, Alejandro Colomar wrote:
>
> > GCC supports __extension__ since at least GCC 2.8.
>
> This is a file shared with gnulib, but __extension__ is generally only
> relevant with -pedantic; there's no problem with supporting its absence
> with non-GCC compilers, so no real advantage in removing this code.
>
> In fact, GCC 2.7 (the oldest version supporting ELF for any target
> supported now by glibc) *does* support __extension__, so the more
> interesting question about such a condition is whether there are specific
> sub-cases of __extension__ that are used in installed headers and are the
> reason for the 2.8 version here.
git-blame(1) in gnulib points to this:
commit 6421fd46004655f39afca254ad02b72ab97f5ecb
Author: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu Mar 27 13:38:41 2014 -0700
obstack: Remove ancient NeXTSTEP gcc support conditional
This change will ease merging with glibc. The "#if ... __NEXT__"
causes a warning with -Wundef which glibc now enables by default.
Problem reported by Will Newton in
<http://lists.gnu.org/archive/html/bug-gnulib/2014-03/msg00032.html>.
glibc <sys/cdefs.h> now uses __extension__ for GCC 2.8 or later,
so go with that.
* lib/obstack.h (__extension__):
[...]
diff --git a/lib/obstack.h b/lib/obstack.h
index f847a53706..f92492fc68 100644
--- a/lib/obstack.h
+++ b/lib/obstack.h
@@ -258,10 +258,7 @@ extern int obstack_exit_failure;
#define obstack_memory_used(h) _obstack_memory_used (h)
#if defined __GNUC__
-/* NextStep 2.0 cc is really gcc 1.93 but it defines __GNUC__ = 2 and
- does not implement __extension__. But that compiler doesn't define
- __GNUC_MINOR__. */
-# if __GNUC__ < 2 || (__NeXT__ && !__GNUC_MINOR__)
+# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
# define __extension__
# endif
That commit message probably refers to this in glibc:
$ cat ./misc/sys/cdefs.h | grep -C2 __extension__
/* It is possible to compile containing GCC extensions even if GCC is
run in pedantic mode if the uses are carefully marked using the
`__extension__' keyword. But this is not generally available before
version 2.8. */
#if !(__GNUC_PREREQ (2,8) || defined __clang__)
# define __extension__ /* Ignore */
#endif
However, it seems to ignore this:
commit bc244b778ac3e17089454050ac86662ad51f7fd0
Author: Jim Meyering <jim@meyering.net>
Date: Fri Nov 4 16:50:15 1994 +0000
merge with 3.9u1
[...]
diff --git a/lib/obstack.h b/lib/obstack.h
index 01767195da..840a63baaa 100644
--- a/lib/obstack.h
+++ b/lib/obstack.h
[...]
@@ -269,7 +269,10 @@ int obstack_chunk_size (struct obstack *obstack);
#define obstack_blank_fast(h,n) ((h)->next_free += (n))
^L
#if defined (__GNUC__) && defined (__STDC__)
-#if __GNUC__ < 2
+/* NextStep 2.0 cc is really gcc 1.93 but it defines __GNUC__ = 2 and
+ does not implement __extension__. But that compiler doesn't define
+ __GNUC_MINOR__. */
+#if __GNUC__ < 2 || !__GNUC_MINOR__
#define __extension__
#endif
Which claims that GCC 2 is fine with __extension__.
The commit that added the 2.8 check in <sys/cdefs.h> is
8325d82cf391 (1998-10-05; "Update."), whose ChangeLog only says:
1998-10-05 Ulrich Drepper <drepper@cygnus.com>
* misc/sys/cdefs.h (__extension__): Define as empty if no gcc or
gcc before 2.8 is used.
so, I don't really know why Ulrich did that. There's some possibility
that he was wrong. Or maybe it was really not widely available on some
GCC forks that claimed different __GNUC__ and __GNUC_MINOR__ values.
Who knows.
Have a lovely night!
Alex
On Tue, 11 Nov 2025, Alejandro Colomar wrote:
> > * a stdarg.h that defines va_list
>
> The relevant change is:
>
> #if defined __USE_XOPEN2K || defined __USE_XOPEN2K8
> -# ifdef __GNUC__
> -# ifndef _VA_LIST_DEFINED
> -typedef __gnuc_va_list va_list;
> -# define _VA_LIST_DEFINED
> -# endif
> -# else
> +# ifndef __GNUC__
> # include <stdarg.h>
> # endif
> #endif
>
> I'm only changing the assumption if __GNUC__ is defined. This change
> assumes that GCC provides va_list in its own header. However, I'm not
> changing any assumptions for non-__GNUC__ compilers.
I think this change (to <stdio.h>) is actually incorrect for modern GCC
(indeed, I don't see how the patched glibc would have passed the conform/
tests).
Early on, the header does:
#define __need___va_list
#include <stdarg.h>
The point of that is, if the __need_* protocol is working correctly, to
define __gnuc_va_list but *not* va_list (because ISO C, for example, does
not allow <stdio.h> to define va_list, despite having interfaces such as
vprintf that involve va_list in their types). So with GCC, or compatible
compilers, __gnuc_va_list will be defined, but not va_list - unless the
user had previously included <stdarg.h> themself without __need___va_list
defined.
But then there is the POSIX requirement (CX-tagged in the 2008 and 2024
editions, XSI-tagged in the 2004 edition) to define va_list in <stdio.h>.
That's what the code you're changing implements. And your change would
cause <stdio.h> to fail to implement that requirement with current GCC.
@@ -258,10 +258,6 @@ extern int obstack_exit_failure;
#define obstack_memory_used(h) _obstack_memory_used (h)
#if defined __GNUC__
-# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
-# define __extension__
-# endif
-
/* For GNU C, if not -traditional,
we can define these macros to compute all args only once
without using a global variable.