diff mbox series

[RFC] c++: Accept elaborated-enum-base in system headers

Message ID ZIG2PaKQvzGAfbd2@arm.com
State New
Headers
Series [RFC] c++: Accept elaborated-enum-base in system headers |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed

Commit Message

Alex Coplan June 8, 2023, 11:06 a.m. UTC 
  Hi,

macOS SDK headers using the CF_ENUM macro can expand to invalid C++ code
of the form:

typedef enum T : BaseType T;

i.e. an elaborated-type-specifier with an additional enum-base.
Upstream LLVM can be made to accept the above construct with
-Wno-error=elaborated-enum-base.

This macro expansion occurs in the case that the compiler declares
support for enums with underlying type using __has_feature, see
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618450.html

GCC rejecting this construct outright means that GCC fails to bootstrap
on Darwin in the case that it (correctly) implements __has_feature and
declares support for C++ enums with underlying type.

This patch attempts to accept this construct in the C++ parser but only
if it appears in system headers. With this patch, GCC can bootstrap on
Darwin in combination with the (WIP) __has_feature patch posted at:
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/617878.html

We also attempt to improve the diagnostic for this case, using a
similar diagnostic to that given by LLVM.

If it is more palatable I can look into restricting the change to accept
this code to only take effect on Darwin, but it's not clear that that's
any better or worse.

Other possible approaches here include trying to fixincludes the SDK
framework headers, but as Iain pointed out in the review of the
has_feature RFC, the necessary infrastructure doesn't exist at the
moment. Even if this support did exist, I believe the headers would
require quite extensive non-trivial "fixing".

Adjusting the parser to accept this construct in system headers seemed
more pragmatic and cleaner on balance.

Bootstrapped/regtested on aarch64-linux-gnu and x86_64-apple-darwin.

Any thoughts?

Thanks,
Alex

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_enum_specifier): Accept
	elaborated-type-specifier with enum-base if in system headers.
	Improve diagnostic when rejecting such a construct.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/enum40.C: Adjust expected diagnostics.
	* g++.dg/cpp0x/forw_enum6.C: Likewise.
	* g++.dg/ext/elab-enum-header.C: New test.
	* g++.dg/ext/elab-enum-invalid.C: New test.

Comments

Jason Merrill June 8, 2023, 6:06 p.m. UTC | #1 
On 6/8/23 07:06, Alex Coplan wrote:
> Hi,
> 
> macOS SDK headers using the CF_ENUM macro can expand to invalid C++ code
> of the form:
> 
> typedef enum T : BaseType T;
> 
> i.e. an elaborated-type-specifier with an additional enum-base.
> Upstream LLVM can be made to accept the above construct with
> -Wno-error=elaborated-enum-base.

I guess we might as well follow that example, and so instead of this check:

> +	   || (underlying_type && !in_system_header_at (colon_loc)))

Make the below an on-by-default pedwarn using OPT_Welaborated_enum_base, 
and don't return error_mark_node.

> +	      cp_parser_commit_to_tentative_parse (parser);
> +	      error_at (colon_loc,
> +			"declaration of enumeration with "
> +			"fixed underlying type and no enumerator list is "
> +			"only permitted as a standalone declaration");
Iain Sandoe June 8, 2023, 6:47 p.m. UTC | #2 
> Begin forwarded message:
> 
> From: Jason Merrill <jason@redhat.com>
> Subject: Re: [PATCH][RFC] c++: Accept elaborated-enum-base in system headers
> Date: 8 June 2023 at 19:06:36 BST
> To: Alex Coplan <alex.coplan@arm.com>, gcc-patches@gcc.gnu.org
> Cc: Nathan Sidwell <nathan@acm.org>, Iain Sandoe <iain@sandoe.co.uk>
> 
> On 6/8/23 07:06, Alex Coplan wrote:
>> Hi,
>> macOS SDK headers using the CF_ENUM macro can expand to invalid C++ code
>> of the form:
>> typedef enum T : BaseType T;
>> i.e. an elaborated-type-specifier with an additional enum-base.
>> Upstream LLVM can be made to accept the above construct with
>> -Wno-error=elaborated-enum-base.
> 
> I guess we might as well follow that example, and so instead of this check:
> 
>> +	   || (underlying_type && !in_system_header_at (colon_loc)))
> 
> Make the below an on-by-default pedwarn using OPT_Welaborated_enum_base, and don't return error_mark_node.

I was also wondering about (for this and other reasons) a -fclang-compat to put some of these things behind (since std=clang++NN is not really going to work to describe other non-standard extensions etc. since most are not synchronised to std revisions.)

Iain

> 
>> +	      cp_parser_commit_to_tentative_parse (parser);
>> +	      error_at (colon_loc,
>> +			"declaration of enumeration with "
>> +			"fixed underlying type and no enumerator list is "
>> +			"only permitted as a standalone declaration");
> 
>
diff mbox series

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index d77fbd20e56..e13133a6cfb 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -21024,11 +21024,13 @@  cp_parser_enum_specifier (cp_parser* parser)
 
   /* Check for the `:' that denotes a specified underlying type in C++0x.
      Note that a ':' could also indicate a bitfield width, however.  */
+  location_t colon_loc = UNKNOWN_LOCATION;
   if (cp_lexer_next_token_is (parser->lexer, CPP_COLON))
     {
       cp_decl_specifier_seq type_specifiers;
 
       /* Consume the `:'.  */
+      colon_loc = cp_lexer_peek_token (parser->lexer)->location;
       cp_lexer_consume_token (parser->lexer);
 
       auto tdf
@@ -21073,12 +21075,20 @@  cp_parser_enum_specifier (cp_parser* parser)
 	    return error_mark_node;
 	}
       /* An opaque-enum-specifier must have a ';' here.  */
-      if ((scoped_enum_p || underlying_type)
+      if ((scoped_enum_p
+	   || (underlying_type && !in_system_header_at (colon_loc)))
 	  && cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
 	{
 	  if (has_underlying_type)
-	    cp_parser_commit_to_tentative_parse (parser);
-	  cp_parser_error (parser, "expected %<;%> or %<{%>");
+	    {
+	      cp_parser_commit_to_tentative_parse (parser);
+	      error_at (colon_loc,
+			"declaration of enumeration with "
+			"fixed underlying type and no enumerator list is "
+			"only permitted as a standalone declaration");
+	    }
+	  else
+	    cp_parser_error (parser, "expected %<;%> or %<{%>");
 	  if (has_underlying_type)
 	    return error_mark_node;
 	}
diff --git a/gcc/testsuite/g++.dg/cpp0x/enum40.C b/gcc/testsuite/g++.dg/cpp0x/enum40.C
index cfdf2a4a18a..d3ffeb62d70 100644
--- a/gcc/testsuite/g++.dg/cpp0x/enum40.C
+++ b/gcc/testsuite/g++.dg/cpp0x/enum40.C
@@ -4,23 +4,25 @@ 
 void
 foo ()
 {
-  enum : int a alignas;		// { dg-error "expected" }
+  enum : int a alignas;		// { dg-error "declaration of enum" }
+  // { dg-error {expected '\(' before ';'} "" { target *-*-* } .-1 }
 }
 
 void
 bar ()
 {
-  enum : int a;			// { dg-error "expected" }
+  enum : int a;			// { dg-error "declaration of enum" }
 }
 
 void
 baz ()
 {
-  enum class a : int b alignas;	// { dg-error "expected" }
+  enum class a : int b alignas;	// { dg-error "declaration of enum" }
+  // { dg-error {expected '\(' before ';'} "" { target *-*-* } .-1 }
 }
 
 void
 qux ()
 {
-  enum class a : int b;		// { dg-error "expected" }
+  enum class a : int b;		// { dg-error "declaration of enum" }
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/forw_enum6.C b/gcc/testsuite/g++.dg/cpp0x/forw_enum6.C
index 01bf563bcdd..8ad3f733292 100644
--- a/gcc/testsuite/g++.dg/cpp0x/forw_enum6.C
+++ b/gcc/testsuite/g++.dg/cpp0x/forw_enum6.C
@@ -23,7 +23,7 @@  enum class E7 : int; //ok
 
 enum class E3 e3; // { dg-error "scoped enum must not use" }
 enum struct E3 e4; // { dg-error "scoped enum must not use" }
-enum E5 : int e5; // { dg-error "expected|invalid type" }
+enum E5 : int e5; // { dg-error "declaration of enumeration with fixed underlying type|invalid type" }
 
 enum E6 : int { a, b, c }; // { dg-message "previous definition" }
 enum E6 : int { a, b, c }; // { dg-error "multiple definition" }
diff --git a/gcc/testsuite/g++.dg/ext/elab-enum-header.C b/gcc/testsuite/g++.dg/ext/elab-enum-header.C
new file mode 100644
index 00000000000..d8c03faf443
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/elab-enum-header.C
@@ -0,0 +1,5 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fpreprocessed" }
+# 1 "" 3
+typedef long CFIndex;
+typedef enum CFComparisonResult : CFIndex CFComparisonResult;
diff --git a/gcc/testsuite/g++.dg/ext/elab-enum-invalid.C b/gcc/testsuite/g++.dg/ext/elab-enum-invalid.C
new file mode 100644
index 00000000000..db3957d7367
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/elab-enum-invalid.C
@@ -0,0 +1,4 @@ 
+// { dg-do compile { target c++11 } }
+typedef long CFIndex;
+typedef enum CFComparisonResult : CFIndex CFComparisonResult;
+// { dg-error "declaration of enumeration with fixed underlying type" "" {target *-*-*} .-1 }