diff mbox series

ginclude: stdalign.h should define __xxx_is_defined macros for C++

Message ID 20241023143959.1195051-1-jwakely@redhat.com
State New
Headers show
Series ginclude: stdalign.h should define __xxx_is_defined macros for C++ | expand

Commit Message

Jonathan Wakely Oct. 23, 2024, 2:39 p.m. UTC 
The __alignas_is_defined macro has been required by C++ since C++11, and
C++ Library DR 4036 clarified that __alignof_is_defined should be
defined too.

The macros alignas and alignof should not be defined, as they're
keywords in C++.

Technically it's implementation-defined whether __STDC_VERSION__ is
defined by a C++ compiler, but G++ does not define it. Adjusting the
first #if this way works as intended: A C23 compiler will not enter the
outer if-group and so will not define any of the macros, a C17 compiler
will enter both if-groups and so define all the macros, and a C++
compiler will enter the outer if-group but not the inner if-group.

gcc/ChangeLog:

	* ginclude/stdalign.h (__alignas_is_defined): Define for C++.
	(__alignof_is_defined): Likewise.

libstdc++-v3/ChangeLog:

	* testsuite/18_support/headers/cstdalign/macros.cc: New test.
---

The libc++ devs noticed recently that GCC's <stdalign.h> doesn't conform
to the C++ requirements.

Tested x86_64-linux.

OK for trunk?

 gcc/ginclude/stdalign.h                       |  5 ++--
 .../18_support/headers/cstdalign/macros.cc    | 24 +++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc

Comments

Jason Merrill Oct. 23, 2024, 3:01 p.m. UTC | #1 
On 10/23/24 10:39 AM, Jonathan Wakely wrote:
> The __alignas_is_defined macro has been required by C++ since C++11, and
> C++ Library DR 4036 clarified that __alignof_is_defined should be
> defined too.
> 
> The macros alignas and alignof should not be defined, as they're
> keywords in C++.
> 
> Technically it's implementation-defined whether __STDC_VERSION__ is
> defined by a C++ compiler, but G++ does not define it. Adjusting the
> first #if this way works as intended: A C23 compiler will not enter the
> outer if-group and so will not define any of the macros, a C17 compiler
> will enter both if-groups and so define all the macros, and a C++
> compiler will enter the outer if-group but not the inner if-group.
> 
> gcc/ChangeLog:
> 
> 	* ginclude/stdalign.h (__alignas_is_defined): Define for C++.
> 	(__alignof_is_defined): Likewise.

Do we want to note somehow that these macros are deprecated since C++17?

> libstdc++-v3/ChangeLog:
> 
> 	* testsuite/18_support/headers/cstdalign/macros.cc: New test.
> ---
> 
> The libc++ devs noticed recently that GCC's <stdalign.h> doesn't conform
> to the C++ requirements.
> 
> Tested x86_64-linux.
> 
> OK for trunk?
> 
>   gcc/ginclude/stdalign.h                       |  5 ++--
>   .../18_support/headers/cstdalign/macros.cc    | 24 +++++++++++++++++++
>   2 files changed, 27 insertions(+), 2 deletions(-)
>   create mode 100644 libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
> 
> diff --git a/gcc/ginclude/stdalign.h b/gcc/ginclude/stdalign.h
> index 5f82f2d68f2..af73c322624 100644
> --- a/gcc/ginclude/stdalign.h
> +++ b/gcc/ginclude/stdalign.h
> @@ -26,11 +26,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>   #ifndef _STDALIGN_H
>   #define _STDALIGN_H
>   
> -#if (!defined __cplusplus						\
> -     && !(defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L))
> +#if !(defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L)
>   
> +#ifndef __cplusplus
>   #define alignas _Alignas
>   #define alignof _Alignof
> +#endif
>   
>   #define __alignas_is_defined 1
>   #define __alignof_is_defined 1
> diff --git a/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc b/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
> new file mode 100644
> index 00000000000..c50c921cd59
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
> @@ -0,0 +1,24 @@
> +// { dg-options "-D_GLIBCXX_USE_DEPRECATED=1 -Wno-deprecated" }
> +// { dg-do preprocess { target c++11 } }
> +
> +#include <cstdalign>

Should there also/instead be a test with <stdalign.h>?

> +
> +#ifndef __alignas_is_defined
> +# error "The header <cstdalign> fails to define a macro named  __alignas_is_defined"
> +#elif __alignas_is_defined != 1
> +# error "__alignas_is_defined is not defined to 1 in <cstdalign>"
> +#endif
> +
> +#ifndef __alignof_is_defined
> +# error "The header <cstdalign> fails to define a macro named __alignof_is_defined"
> +#elif __alignof_is_defined != 1
> +# error "__alignof_is_defined is not defined to 1 in <cstdalign>"
> +#endif
> +
> +#ifdef alignas
> +# error "The header <cstdalign> defines a macro named alignas"
> +#endif
> +
> +#ifdef alignof
> +# error "The header <cstdalign> defines a macro named alignof"
> +#endif
Jonathan Wakely Oct. 23, 2024, 6:41 p.m. UTC | #2 
On Wed, 23 Oct 2024 at 16:02, Jason Merrill <jason@redhat.com> wrote:
>
> On 10/23/24 10:39 AM, Jonathan Wakely wrote:
> > The __alignas_is_defined macro has been required by C++ since C++11, and
> > C++ Library DR 4036 clarified that __alignof_is_defined should be
> > defined too.
> >
> > The macros alignas and alignof should not be defined, as they're
> > keywords in C++.
> >
> > Technically it's implementation-defined whether __STDC_VERSION__ is
> > defined by a C++ compiler, but G++ does not define it. Adjusting the
> > first #if this way works as intended: A C23 compiler will not enter the
> > outer if-group and so will not define any of the macros, a C17 compiler
> > will enter both if-groups and so define all the macros, and a C++
> > compiler will enter the outer if-group but not the inner if-group.
> >
> > gcc/ChangeLog:
> >
> >       * ginclude/stdalign.h (__alignas_is_defined): Define for C++.
> >       (__alignof_is_defined): Likewise.
>
> Do we want to note somehow that these macros are deprecated since C++17?

The <cstdalign> header is deprecated in C++17, but the macros aren't
if you get them via <stdalign.h>. They were deprecated for C++23 by my
NB comment https://cplusplus.github.io/LWG/issue3827

Would you like it to be noted in the commit message, or in the header
itself, something like this?

--- a/gcc/ginclude/stdalign.h
+++ b/gcc/ginclude/stdalign.h
@@ -33,6 +33,7 @@ see the files COPYING3 and COPYING.RUNTIME
respectively.  If not, see
#define alignof _Alignof
#endif

+/* These are defined for C++, but deprecated in C++23.  */
#define __alignas_is_defined 1
#define __alignof_is_defined 1

I expect <stdalign.h> to be removed completely for C++26, if my P3348
is approved. I'm not sure if we want to add deprecation warnings to
<stdalign.h> for C++26, or just make it empty. That's for another day
anyway.


> > libstdc++-v3/ChangeLog:
> >
> >       * testsuite/18_support/headers/cstdalign/macros.cc: New test.
> > ---
> >
> > The libc++ devs noticed recently that GCC's <stdalign.h> doesn't conform
> > to the C++ requirements.
> >
> > Tested x86_64-linux.
> >
> > OK for trunk?
> >
> >   gcc/ginclude/stdalign.h                       |  5 ++--
> >   .../18_support/headers/cstdalign/macros.cc    | 24 +++++++++++++++++++
> >   2 files changed, 27 insertions(+), 2 deletions(-)
> >   create mode 100644 libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
> >
> > diff --git a/gcc/ginclude/stdalign.h b/gcc/ginclude/stdalign.h
> > index 5f82f2d68f2..af73c322624 100644
> > --- a/gcc/ginclude/stdalign.h
> > +++ b/gcc/ginclude/stdalign.h
> > @@ -26,11 +26,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> >   #ifndef _STDALIGN_H
> >   #define _STDALIGN_H
> >
> > -#if (!defined __cplusplus                                            \
> > -     && !(defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L))
> > +#if !(defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L)
> >
> > +#ifndef __cplusplus
> >   #define alignas _Alignas
> >   #define alignof _Alignof
> > +#endif
> >
> >   #define __alignas_is_defined 1
> >   #define __alignof_is_defined 1
> > diff --git a/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc b/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
> > new file mode 100644
> > index 00000000000..c50c921cd59
> > --- /dev/null
> > +++ b/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
> > @@ -0,0 +1,24 @@
> > +// { dg-options "-D_GLIBCXX_USE_DEPRECATED=1 -Wno-deprecated" }
> > +// { dg-do preprocess { target c++11 } }
> > +
> > +#include <cstdalign>
>
> Should there also/instead be a test with <stdalign.h>?

We don't usually (or ever?) bother to test the .h versions of headers.
For these ones that are deprecated it probably makes sense to do so,
because <stdalign.h> isn't deprecated, only <cstdalign>. So we should
ensure that the .h forms work fine without diagnostics even in C++17
and later.
I'll add more tests.

> > +
> > +#ifndef __alignas_is_defined
> > +# error "The header <cstdalign> fails to define a macro named  __alignas_is_defined"
> > +#elif __alignas_is_defined != 1
> > +# error "__alignas_is_defined is not defined to 1 in <cstdalign>"
> > +#endif
> > +
> > +#ifndef __alignof_is_defined
> > +# error "The header <cstdalign> fails to define a macro named __alignof_is_defined"
> > +#elif __alignof_is_defined != 1
> > +# error "__alignof_is_defined is not defined to 1 in <cstdalign>"
> > +#endif
> > +
> > +#ifdef alignas
> > +# error "The header <cstdalign> defines a macro named alignas"
> > +#endif
> > +
> > +#ifdef alignof
> > +# error "The header <cstdalign> defines a macro named alignof"
> > +#endif
>
Jonathan Wakely Oct. 23, 2024, 6:44 p.m. UTC | #3 
On Wed, 23 Oct 2024 at 19:41, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Wed, 23 Oct 2024 at 16:02, Jason Merrill <jason@redhat.com> wrote:
> >
> > Should there also/instead be a test with <stdalign.h>?
>
> We don't usually (or ever?) bother to test the .h versions of headers.
> For these ones that are deprecated it probably makes sense to do so,
> because <stdalign.h> isn't deprecated, only <cstdalign>. So we should
> ensure that the .h forms work fine without diagnostics even in C++17
> and later.
> I'll add more tests.

But I think it makes sense to do that as part of a follow-up commit
where I deprecate <cstdalign>.
Jonathan Wakely Oct. 23, 2024, 6:52 p.m. UTC | #4 
On Wed, 23 Oct 2024 at 19:44, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Wed, 23 Oct 2024 at 19:41, Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > On Wed, 23 Oct 2024 at 16:02, Jason Merrill <jason@redhat.com> wrote:
> > >
> > > Should there also/instead be a test with <stdalign.h>?
> >
> > We don't usually (or ever?) bother to test the .h versions of headers.
> > For these ones that are deprecated it probably makes sense to do so,
> > because <stdalign.h> isn't deprecated, only <cstdalign>. So we should
> > ensure that the .h forms work fine without diagnostics even in C++17
> > and later.
> > I'll add more tests.
>
> But I think it makes sense to do that as part of a follow-up commit
> where I deprecate <cstdalign>.

Here's a preview of that follow-up, without the new tests I need to add:
https://forge.sourceware.org/gcc/gcc-TEST/pulls/4
Jason Merrill Oct. 23, 2024, 10:09 p.m. UTC | #5 
On 10/23/24 2:41 PM, Jonathan Wakely wrote:
> On Wed, 23 Oct 2024 at 16:02, Jason Merrill <jason@redhat.com> wrote:
>>
>> On 10/23/24 10:39 AM, Jonathan Wakely wrote:
>>> The __alignas_is_defined macro has been required by C++ since C++11, and
>>> C++ Library DR 4036 clarified that __alignof_is_defined should be
>>> defined too.
>>>
>>> The macros alignas and alignof should not be defined, as they're
>>> keywords in C++.
>>>
>>> Technically it's implementation-defined whether __STDC_VERSION__ is
>>> defined by a C++ compiler, but G++ does not define it. Adjusting the
>>> first #if this way works as intended: A C23 compiler will not enter the
>>> outer if-group and so will not define any of the macros, a C17 compiler
>>> will enter both if-groups and so define all the macros, and a C++
>>> compiler will enter the outer if-group but not the inner if-group.
>>>
>>> gcc/ChangeLog:
>>>
>>>        * ginclude/stdalign.h (__alignas_is_defined): Define for C++.
>>>        (__alignof_is_defined): Likewise.
>>
>> Do we want to note somehow that these macros are deprecated since C++17?
> 
> The <cstdalign> header is deprecated in C++17, but the macros aren't
> if you get them via <stdalign.h>. They were deprecated for C++23 by my
> NB comment https://cplusplus.github.io/LWG/issue3827
> 
> Would you like it to be noted in the commit message, or in the header
> itself, something like this?
> 
> --- a/gcc/ginclude/stdalign.h
> +++ b/gcc/ginclude/stdalign.h
> @@ -33,6 +33,7 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively.  If not, see
> #define alignof _Alignof
> #endif
> 
> +/* These are defined for C++, but deprecated in C++23.  */
> #define __alignas_is_defined 1
> #define __alignof_is_defined 1

Sure, OK with that comment.

Jason
diff mbox series

Patch

diff --git a/gcc/ginclude/stdalign.h b/gcc/ginclude/stdalign.h
index 5f82f2d68f2..af73c322624 100644
--- a/gcc/ginclude/stdalign.h
+++ b/gcc/ginclude/stdalign.h
@@ -26,11 +26,12 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #ifndef _STDALIGN_H
 #define _STDALIGN_H
 
-#if (!defined __cplusplus						\
-     && !(defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L))
+#if !(defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L)
 
+#ifndef __cplusplus
 #define alignas _Alignas
 #define alignof _Alignof
+#endif
 
 #define __alignas_is_defined 1
 #define __alignof_is_defined 1
diff --git a/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc b/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
new file mode 100644
index 00000000000..c50c921cd59
--- /dev/null
+++ b/libstdc++-v3/testsuite/18_support/headers/cstdalign/macros.cc
@@ -0,0 +1,24 @@ 
+// { dg-options "-D_GLIBCXX_USE_DEPRECATED=1 -Wno-deprecated" }
+// { dg-do preprocess { target c++11 } }
+
+#include <cstdalign>
+
+#ifndef __alignas_is_defined
+# error "The header <cstdalign> fails to define a macro named  __alignas_is_defined"
+#elif __alignas_is_defined != 1
+# error "__alignas_is_defined is not defined to 1 in <cstdalign>"
+#endif
+
+#ifndef __alignof_is_defined
+# error "The header <cstdalign> fails to define a macro named __alignof_is_defined"
+#elif __alignof_is_defined != 1
+# error "__alignof_is_defined is not defined to 1 in <cstdalign>"
+#endif
+
+#ifdef alignas
+# error "The header <cstdalign> defines a macro named alignas"
+#endif
+
+#ifdef alignof
+# error "The header <cstdalign> defines a macro named alignof"
+#endif