public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: DR2237, cdtor and template-id tweaks [PR107126]
@ 2024-02-03 15:24 Marek Polacek
  2024-02-05 15:14 ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2024-02-03 15:24 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

I'm not certain OPT_Wc__20_extensions is the best thing for something
from [diff.cpp17]; would you prefer something else?

-- >8 --
Since my r11-532 changes to implement DR2237, for this test:

  template<typename T>
  struct S {
    S<T>();
  };

in C++20 we emit the ugly:

q.C:3:8: error: expected unqualified-id before ')' token
    3 |   S<T>();

which doesn't explain what the problem is.  This patch improves that
diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
diagnostic.  We now say:

q.C:3:7: warning: template-id not allowed for constructor [-Wc++20-extensions]
    3 |   S<T>();

This patch does *not* fix
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
where the C++20 diagnostic is missing altogether.  Something for the
next stage1 I reckon.

-Wc++20-compat triggered in libitm/; I sent a patch for that.

	DR 2237
	PR c++/107126
	PR c++/97202

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
	a pedwarn.  Emit a -Wc++20-compat message.
	(cp_parser_constructor_declarator_p): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/DRs/dr2237.C: Adjust dg-error.
	* g++.dg/parse/constructor2.C: Likewise.
	* g++.dg/template/error34.C: Likewise.
	* g++.old-deja/g++.pt/ctor2.C: Likewise.
	* g++.dg/DRs/dr2237-2.C: New test.
	* g++.dg/DRs/dr2237-3.C: New test.
	* g++.dg/DRs/dr2237-4.C: New test.
	* g++.dg/diagnostic/cdtor-template1.C: New test.
---
 gcc/cp/parser.cc                              | 33 ++++++++++++++-----
 gcc/testsuite/g++.dg/DRs/dr2237-2.C           |  9 +++++
 gcc/testsuite/g++.dg/DRs/dr2237-3.C           | 16 +++++++++
 gcc/testsuite/g++.dg/DRs/dr2237-4.C           | 11 +++++++
 gcc/testsuite/g++.dg/DRs/dr2237.C             |  2 +-
 .../g++.dg/diagnostic/cdtor-template1.C       |  9 +++++
 gcc/testsuite/g++.dg/parse/constructor2.C     | 16 ++++-----
 gcc/testsuite/g++.dg/template/error34.C       | 10 +++---
 gcc/testsuite/g++.old-deja/g++.pt/ctor2.C     |  2 +-
 9 files changed, 85 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-2.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-3.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-4.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/cdtor-template1.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 3748ccd49ff..4f7d4edbad9 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -6717,12 +6717,17 @@ cp_parser_unqualified_id (cp_parser* parser,
 
 	/* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
 	   declarator-id of a constructor or destructor.  */
-	if (token->type == CPP_TEMPLATE_ID && declarator_p
-	    && cxx_dialect >= cxx20)
+	if (token->type == CPP_TEMPLATE_ID && declarator_p)
 	  {
-	    if (!cp_parser_simulate_error (parser))
-	      error_at (tilde_loc, "template-id not allowed for destructor");
-	    return error_mark_node;
+	    if (cxx_dialect >= cxx20)
+	      {
+		if (!cp_parser_simulate_error (parser))
+		  pedwarn (tilde_loc, OPT_Wc__20_extensions,
+			   "template-id not allowed for destructor");
+		return error_mark_node;
+	      }
+	    warning_at (tilde_loc, OPT_Wc__20_compat,
+			"template-id not allowed for destructor in C++20");
 	  }
 
 	/* If there was an explicit qualification (S::~T), first look
@@ -32329,11 +32334,11 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   if (next_token->type != CPP_NAME
       && next_token->type != CPP_SCOPE
       && next_token->type != CPP_NESTED_NAME_SPECIFIER
-      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
-	 declarator-id of a constructor or destructor.  */
-      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
+      && next_token->type != CPP_TEMPLATE_ID)
     return false;
 
+  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);
+
   /* Parse tentatively; we are going to roll back all of the tokens
      consumed here.  */
   cp_parser_parse_tentatively (parser);
@@ -32550,6 +32555,18 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   /* We did not really want to consume any tokens.  */
   cp_parser_abort_tentative_parse (parser);
 
+  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
+     declarator-id of a constructor or destructor.  */
+  if (constructor_p && saw_template_id)
+    {
+      if (cxx_dialect >= cxx20)
+	pedwarn (input_location, OPT_Wc__20_extensions,
+		 "template-id not allowed for constructor");
+      else
+	warning (OPT_Wc__20_compat,
+		 "template-id not allowed for constructor in C++20");
+    }
+
   return constructor_p;
 }
 
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-2.C b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
new file mode 100644
index 00000000000..1d99347229c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
@@ -0,0 +1,9 @@
+// DR 2237 - Can a template-id name a constructor?
+// { dg-options "" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+  X(int); // OK, injected-class-name used
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+};
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-3.C b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
new file mode 100644
index 00000000000..c8ad6852389
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
@@ -0,0 +1,16 @@
+// PR c++/107126
+// { dg-options "" }
+
+template<typename T>
+struct C
+{
+    ~C();
+};
+template<typename T>
+C<T>::~C<T>()	      // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+{
+}
+int main()
+{
+    C<int> c;;
+}
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-4.C b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
new file mode 100644
index 00000000000..a358dd521b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
@@ -0,0 +1,11 @@
+// PR c++/97202
+// { dg-options "" }
+
+template<typename T>
+struct F
+{
+  F<T>();  // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+};
+
+template<typename T>
+inline F<T>::F() { }
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237.C b/gcc/testsuite/g++.dg/DRs/dr2237.C
index f3d6d11e61e..830c8f5a2a6 100644
--- a/gcc/testsuite/g++.dg/DRs/dr2237.C
+++ b/gcc/testsuite/g++.dg/DRs/dr2237.C
@@ -2,7 +2,7 @@
 
 template<class T>
 struct X {
-  X<T>(); // { dg-error "expected" "" { target c++20 } }
+  X<T>(); // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
   X(int); // OK, injected-class-name used
   ~X<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++20 } }
 };
diff --git a/gcc/testsuite/g++.dg/diagnostic/cdtor-template1.C b/gcc/testsuite/g++.dg/diagnostic/cdtor-template1.C
new file mode 100644
index 00000000000..429490708bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/cdtor-template1.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wc++20-compat" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/parse/constructor2.C b/gcc/testsuite/g++.dg/parse/constructor2.C
index d620f41e027..e4b2ad3f2fe 100644
--- a/gcc/testsuite/g++.dg/parse/constructor2.C
+++ b/gcc/testsuite/g++.dg/parse/constructor2.C
@@ -1,11 +1,11 @@
 // PR c++/14260
 
-template <class TClass> 
-class T 
-{ 
-public: 
-  T(short,short f=0) {} 
-  T<TClass>(int f) {} // { dg-error "expected" "" { target c++20 } }
-  T<TClass>(int f=0,const char* b=0) {} // { dg-error "expected" "" { target c++20 } }
-}; 
+template <class TClass>
+class T
+{
+public:
+  T(short,short f=0) {}
+  T<TClass>(int f) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+  T<TClass>(int f=0,const char* b=0) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+};
 
diff --git a/gcc/testsuite/g++.dg/template/error34.C b/gcc/testsuite/g++.dg/template/error34.C
index ab688d9ba8c..921cb8fb729 100644
--- a/gcc/testsuite/g++.dg/template/error34.C
+++ b/gcc/testsuite/g++.dg/template/error34.C
@@ -3,27 +3,27 @@
 
 template<typename T> struct A
 {
-  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|expected" }
+  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|template-id" }
 };
 
 template<typename T> struct B
 {
-  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|expected" }
+  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|template-id" }
 };
 
 template<typename T> struct C
 {
-  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|expected" }
+  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|template-id" }
 };
 
 template<typename T> struct D
 {
-  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|expected" }
+  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|template-id" }
 };
 
 struct E { int x; };
 
 template<typename T> struct F
 {
-  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|expected" }
+  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|template-id" }
 };
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
index bf418ba48c7..56b4232d04b 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
@@ -4,7 +4,7 @@
 
 template <class T>
 struct A {
-  A<T>(); // { dg-error "expected" "" { target c++20 } }
+  A<T>(); // { dg-error "template-id" "" { target c++20 } }
 };
 
 template <class T>

base-commit: 78005c648921899a674d1e561b49b05ccabedfe0
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] c++: DR2237, cdtor and template-id tweaks [PR107126]
  2024-02-03 15:24 [PATCH] c++: DR2237, cdtor and template-id tweaks [PR107126] Marek Polacek
@ 2024-02-05 15:14 ` Jason Merrill
  2024-02-06  3:11   ` [PATCH v2] " Marek Polacek
  0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2024-02-05 15:14 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches

On 2/3/24 10:24, Marek Polacek wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> I'm not certain OPT_Wc__20_extensions is the best thing for something
> from [diff.cpp17]; would you prefer something else?

I think it wants its own flag, that is enabled in C++20 or by 
-Wc++20-compat.

> +	    if (cxx_dialect >= cxx20)
> +	      {
> +		if (!cp_parser_simulate_error (parser))
> +		  pedwarn (tilde_loc, OPT_Wc__20_extensions,
> +			   "template-id not allowed for destructor");
> +		return error_mark_node;
> +	      }
> +	    warning_at (tilde_loc, OPT_Wc__20_compat,
> +			"template-id not allowed for destructor in C++20");

After a pedwarn we should accept the code, not return error_mark_node.

I'm also concerned about pedwarn/warnings not guarded by 
!cp_parser_uncommited_to_tentative_parse; that often leads to warning 
about a tentative parse as a declaration that is eventually abandoned in 
favor of a perfectly fine parse as an expression.

It would be good for cp_parser_context to add a vec of warnings to emit 
at cp_parser_parse_definitely time, and then 
cp_parser_pedwarn/cp_parser_warning to fill it...

Jason


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] c++: DR2237, cdtor and template-id tweaks [PR107126]
  2024-02-05 15:14 ` Jason Merrill
@ 2024-02-06  3:11   ` Marek Polacek
  2024-02-07 22:19     ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2024-02-06  3:11 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Mon, Feb 05, 2024 at 10:14:34AM -0500, Jason Merrill wrote:
> On 2/3/24 10:24, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > I'm not certain OPT_Wc__20_extensions is the best thing for something
> > from [diff.cpp17]; would you prefer something else?
> 
> I think it wants its own flag, that is enabled in C++20 or by
> -Wc++20-compat.

That seems best.  I called it -Wdeprecated-template-id-cdtor.
 
> > +	    if (cxx_dialect >= cxx20)
> > +	      {
> > +		if (!cp_parser_simulate_error (parser))
> > +		  pedwarn (tilde_loc, OPT_Wc__20_extensions,
> > +			   "template-id not allowed for destructor");
> > +		return error_mark_node;
> > +	      }
> > +	    warning_at (tilde_loc, OPT_Wc__20_compat,
> > +			"template-id not allowed for destructor in C++20");
> 
> After a pedwarn we should accept the code, not return error_mark_node.

/facepalm, yes.
 
> I'm also concerned about pedwarn/warnings not guarded by
> !cp_parser_uncommited_to_tentative_parse; that often leads to warning about
> a tentative parse as a declaration that is eventually abandoned in favor of
> a perfectly fine parse as an expression.

Done.
 
> It would be good for cp_parser_context to add a vec of warnings to emit at
> cp_parser_parse_definitely time, and then
> cp_parser_pedwarn/cp_parser_warning to fill it...

That would be nice; I don't think we can fix bugs like PR61259 otherwise.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Since my r11-532 changes to implement DR2237, for this test:

  template<typename T>
  struct S {
    S<T>();
  };

in C++20 we emit the ugly:

q.C:3:8: error: expected unqualified-id before ')' token
    3 |   S<T>();

which doesn't explain what the problem is.  This patch improves that
diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
diagnostic.  We now say:

q.C:3:7: warning: template-id not allowed for constructor in C++20 [-Wdeprecated-template-id-cdtor]
    3 |   S<T>();
q.C:3:7: note: remove the '< >'

This patch does *not* fix
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
where the C++20 diagnostic is missing altogether.

-Wc++20-compat triggered in libitm/; I sent a patch for that.

	DR 2237
	PR c++/107126
	PR c++/97202

gcc/c-family/ChangeLog:

	* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
	turn on -Wdeprecated-template-id-cdtor.
	* c.opt (Wdeprecated-template-id-cdtor): New.

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
	a pedwarn.
	(cp_parser_constructor_declarator_p): Likewise.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.

gcc/testsuite/ChangeLog:

	* g++.dg/DRs/dr2237.C: Adjust dg-error.
	* g++.dg/parse/constructor2.C: Likewise.
	* g++.dg/template/error34.C: Likewise.
	* g++.old-deja/g++.pt/ctor2.C: Likewise.
	* g++.dg/DRs/dr2237-2.C: New test.
	* g++.dg/DRs/dr2237-3.C: New test.
	* g++.dg/DRs/dr2237-4.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-1.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-2.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-3.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-4.C: New test.
---
 gcc/c-family/c-opts.cc                        |  5 +++
 gcc/c-family/c.opt                            |  4 +++
 gcc/cp/parser.cc                              | 34 ++++++++++++++-----
 gcc/doc/invoke.texi                           | 18 ++++++++++
 gcc/testsuite/g++.dg/DRs/dr2237-2.C           |  9 +++++
 gcc/testsuite/g++.dg/DRs/dr2237-3.C           | 16 +++++++++
 gcc/testsuite/g++.dg/DRs/dr2237-4.C           | 11 ++++++
 gcc/testsuite/g++.dg/DRs/dr2237.C             |  2 +-
 gcc/testsuite/g++.dg/parse/constructor2.C     | 16 ++++-----
 gcc/testsuite/g++.dg/template/error34.C       | 10 +++---
 .../warn/Wdeprecated-template-id-cdtor-1.C    |  9 +++++
 .../warn/Wdeprecated-template-id-cdtor-2.C    |  9 +++++
 .../warn/Wdeprecated-template-id-cdtor-3.C    |  9 +++++
 .../warn/Wdeprecated-template-id-cdtor-4.C    |  9 +++++
 gcc/testsuite/g++.old-deja/g++.pt/ctor2.C     |  2 +-
 15 files changed, 140 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-2.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-3.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-4.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C

diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index b845aff2226..d2358e2009a 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -998,6 +998,11 @@ c_common_post_options (const char **pfilename)
 		       warn_deprecated_enum_float_conv,
 		       cxx_dialect >= cxx20 && warn_deprecated);
 
+  /* -Wdeprecated-template-id-cdtor is enabled by default in C++20.  */
+  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
+		       warn_deprecated_template_id_cdtor,
+		       cxx_dialect >= cxx20 || warn_cxx20_compat);
+
   /* Declone C++ 'structors if -Os.  */
   if (flag_declone_ctor_dtor == -1)
     flag_declone_ctor_dtor = optimize_size;
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9c0a28092fc..2e1d07b66de 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -642,6 +642,10 @@ C++ ObjC++ Var(warn_deprecated_enum_float_conv) Warning
 Warn about deprecated arithmetic conversions on operands where one is of enumeration
 type and the other is of a floating-point type.
 
+Wdeprecated-template-id-cdtor
+C++ ObjC++ Var(warn_deprecated_template_id_cdtor) Warning
+Warn about deprecated simple-template-id in a constructor or destructor.
+
 Wdesignated-init
 C ObjC Var(warn_designated_init) Init(1) Warning
 Warn about positional initialization of structs requiring designated initializers.
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 3748ccd49ff..473eaf4f1f7 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -6717,12 +6717,17 @@ cp_parser_unqualified_id (cp_parser* parser,
 
 	/* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
 	   declarator-id of a constructor or destructor.  */
-	if (token->type == CPP_TEMPLATE_ID && declarator_p
-	    && cxx_dialect >= cxx20)
+	if (token->type == CPP_TEMPLATE_ID
+	    && declarator_p
+	    && !cp_parser_uncommitted_to_tentative_parse_p (parser)
+	    && !cp_parser_simulate_error (parser))
 	  {
-	    if (!cp_parser_simulate_error (parser))
-	      error_at (tilde_loc, "template-id not allowed for destructor");
-	    return error_mark_node;
+	    auto_diagnostic_group d;
+	    if (emit_diagnostic (cxx_dialect >= cxx20 ? DK_PEDWARN : DK_WARNING,
+				 tilde_loc, OPT_Wdeprecated_template_id_cdtor,
+				 "template-id not allowed for destructor in "
+				 "C++20"))
+	      inform (tilde_loc, "remove the %qs", "< >");
 	  }
 
 	/* If there was an explicit qualification (S::~T), first look
@@ -32329,11 +32334,11 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   if (next_token->type != CPP_NAME
       && next_token->type != CPP_SCOPE
       && next_token->type != CPP_NESTED_NAME_SPECIFIER
-      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
-	 declarator-id of a constructor or destructor.  */
-      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
+      && next_token->type != CPP_TEMPLATE_ID)
     return false;
 
+  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);
+
   /* Parse tentatively; we are going to roll back all of the tokens
      consumed here.  */
   cp_parser_parse_tentatively (parser);
@@ -32550,6 +32555,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   /* We did not really want to consume any tokens.  */
   cp_parser_abort_tentative_parse (parser);
 
+  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
+     declarator-id of a constructor or destructor.  */
+  if (constructor_p
+      && saw_template_id
+      && !cp_parser_uncommitted_to_tentative_parse_p (parser))
+    {
+      auto_diagnostic_group d;
+      if (emit_diagnostic (cxx_dialect >= cxx20 ? DK_PEDWARN : DK_WARNING,
+			   input_location, OPT_Wdeprecated_template_id_cdtor,
+			   "template-id not allowed for constructor in C++20"))
+	inform (input_location, "remove the %qs", "< >");
+    }
+
   return constructor_p;
 }
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 71339b8b30f..5bd65bd02be 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -255,6 +255,7 @@ in the following sections.
 -Wdelete-non-virtual-dtor  -Wno-deprecated-array-compare
 -Wdeprecated-copy -Wdeprecated-copy-dtor
 -Wno-deprecated-enum-enum-conversion -Wno-deprecated-enum-float-conversion
+-Wno-deprecated-template-id-cdtor
 -Weffc++ -Wno-elaborated-enum-base
 -Wno-exceptions -Wextra-semi  -Wno-inaccessible-base
 -Wno-inherited-variadic-ctor  -Wno-init-list-lifetime
@@ -3989,6 +3990,23 @@ bool b = e <= 3.7;
 @option{-std=c++20}.  In pre-C++20 dialects, this warning can be enabled
 by @option{-Wenum-conversion}.
 
+@opindex Wdeprecated-template-id-cdtor
+@opindex Wno-deprecated-template-id-cdtor
+@item -Wno-deprecated-template-id-cdtor @r{(C++ and Objective-C++ only)}
+Disable the warning about the deprecated use of simple-template-id as the
+declarator-id of a constructor or destructor, which became invalid in C++20
+via DR 2237.  For example:
+
+@smallexample
+template<typename T> struct S @{
+  S<T>(); // should be S();
+  ~S<T>();  // should be ~S();
+@};
+@end smallexample
+
+@option{-Wdeprecated-template-id-cdtor} is enabled by default with
+@option{-std=c++20}; it is also enabled by @option{-Wc++20-compat}.
+
 @opindex Welaborated-enum-base
 @opindex Wno-elaborated-enum-base
 @item -Wno-elaborated-enum-base
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-2.C b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
new file mode 100644
index 00000000000..1d99347229c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
@@ -0,0 +1,9 @@
+// DR 2237 - Can a template-id name a constructor?
+// { dg-options "" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+  X(int); // OK, injected-class-name used
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+};
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-3.C b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
new file mode 100644
index 00000000000..c8ad6852389
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
@@ -0,0 +1,16 @@
+// PR c++/107126
+// { dg-options "" }
+
+template<typename T>
+struct C
+{
+    ~C();
+};
+template<typename T>
+C<T>::~C<T>()	      // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+{
+}
+int main()
+{
+    C<int> c;;
+}
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-4.C b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
new file mode 100644
index 00000000000..a358dd521b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
@@ -0,0 +1,11 @@
+// PR c++/97202
+// { dg-options "" }
+
+template<typename T>
+struct F
+{
+  F<T>();  // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+};
+
+template<typename T>
+inline F<T>::F() { }
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237.C b/gcc/testsuite/g++.dg/DRs/dr2237.C
index f3d6d11e61e..830c8f5a2a6 100644
--- a/gcc/testsuite/g++.dg/DRs/dr2237.C
+++ b/gcc/testsuite/g++.dg/DRs/dr2237.C
@@ -2,7 +2,7 @@
 
 template<class T>
 struct X {
-  X<T>(); // { dg-error "expected" "" { target c++20 } }
+  X<T>(); // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
   X(int); // OK, injected-class-name used
   ~X<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++20 } }
 };
diff --git a/gcc/testsuite/g++.dg/parse/constructor2.C b/gcc/testsuite/g++.dg/parse/constructor2.C
index d620f41e027..e4b2ad3f2fe 100644
--- a/gcc/testsuite/g++.dg/parse/constructor2.C
+++ b/gcc/testsuite/g++.dg/parse/constructor2.C
@@ -1,11 +1,11 @@
 // PR c++/14260
 
-template <class TClass> 
-class T 
-{ 
-public: 
-  T(short,short f=0) {} 
-  T<TClass>(int f) {} // { dg-error "expected" "" { target c++20 } }
-  T<TClass>(int f=0,const char* b=0) {} // { dg-error "expected" "" { target c++20 } }
-}; 
+template <class TClass>
+class T
+{
+public:
+  T(short,short f=0) {}
+  T<TClass>(int f) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+  T<TClass>(int f=0,const char* b=0) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+};
 
diff --git a/gcc/testsuite/g++.dg/template/error34.C b/gcc/testsuite/g++.dg/template/error34.C
index ab688d9ba8c..921cb8fb729 100644
--- a/gcc/testsuite/g++.dg/template/error34.C
+++ b/gcc/testsuite/g++.dg/template/error34.C
@@ -3,27 +3,27 @@
 
 template<typename T> struct A
 {
-  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|expected" }
+  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|template-id" }
 };
 
 template<typename T> struct B
 {
-  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|expected" }
+  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|template-id" }
 };
 
 template<typename T> struct C
 {
-  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|expected" }
+  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|template-id" }
 };
 
 template<typename T> struct D
 {
-  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|expected" }
+  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|template-id" }
 };
 
 struct E { int x; };
 
 template<typename T> struct F
 {
-  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|expected" }
+  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|template-id" }
 };
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C
new file mode 100644
index 00000000000..429490708bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wc++20-compat" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C
new file mode 100644
index 00000000000..d76b647c615
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wdeprecated-template-id-cdtor" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C
new file mode 100644
index 00000000000..80fc2ff7739
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wc++20-compat -Wno-deprecated-template-id-cdtor" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-bogus "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-bogus "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C
new file mode 100644
index 00000000000..6f1747701da
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wdeprecated-template-id-cdtor -Wno-c++20-compat" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
index bf418ba48c7..56b4232d04b 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
@@ -4,7 +4,7 @@
 
 template <class T>
 struct A {
-  A<T>(); // { dg-error "expected" "" { target c++20 } }
+  A<T>(); // { dg-error "template-id" "" { target c++20 } }
 };
 
 template <class T>

base-commit: c5d34912ad576be1ef19be92f7eabde54b9089eb
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] c++: DR2237, cdtor and template-id tweaks [PR107126]
  2024-02-06  3:11   ` [PATCH v2] " Marek Polacek
@ 2024-02-07 22:19     ` Jason Merrill
  2024-02-08 21:26       ` [PATCH v3] " Marek Polacek
  0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2024-02-07 22:19 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 2/5/24 22:11, Marek Polacek wrote:
> On Mon, Feb 05, 2024 at 10:14:34AM -0500, Jason Merrill wrote:
>> On 2/3/24 10:24, Marek Polacek wrote:
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> I'm not certain OPT_Wc__20_extensions is the best thing for something
>>> from [diff.cpp17]; would you prefer something else?
>>
>> I think it wants its own flag, that is enabled in C++20 or by
>> -Wc++20-compat.
> 
> That seems best.  I called it -Wdeprecated-template-id-cdtor.
>   
>>> +	    if (cxx_dialect >= cxx20)
>>> +	      {
>>> +		if (!cp_parser_simulate_error (parser))
>>> +		  pedwarn (tilde_loc, OPT_Wc__20_extensions,
>>> +			   "template-id not allowed for destructor");
>>> +		return error_mark_node;
>>> +	      }
>>> +	    warning_at (tilde_loc, OPT_Wc__20_compat,
>>> +			"template-id not allowed for destructor in C++20");
>>
>> After a pedwarn we should accept the code, not return error_mark_node.
> 
> /facepalm, yes.
>   
>> I'm also concerned about pedwarn/warnings not guarded by
>> !cp_parser_uncommited_to_tentative_parse; that often leads to warning about
>> a tentative parse as a declaration that is eventually abandoned in favor of
>> a perfectly fine parse as an expression.
> 
> Done.
>   
>> It would be good for cp_parser_context to add a vec of warnings to emit at
>> cp_parser_parse_definitely time, and then
>> cp_parser_pedwarn/cp_parser_warning to fill it...
> 
> That would be nice; I don't think we can fix bugs like PR61259 otherwise.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> Since my r11-532 changes to implement DR2237, for this test:
> 
>    template<typename T>
>    struct S {
>      S<T>();
>    };
> 
> in C++20 we emit the ugly:
> 
> q.C:3:8: error: expected unqualified-id before ')' token
>      3 |   S<T>();
> 
> which doesn't explain what the problem is.  This patch improves that
> diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
> diagnostic.  We now say:
> 
> q.C:3:7: warning: template-id not allowed for constructor in C++20 [-Wdeprecated-template-id-cdtor]
>      3 |   S<T>();
> q.C:3:7: note: remove the '< >'
> 
> This patch does *not* fix
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
> where the C++20 diagnostic is missing altogether.
> 
> -Wc++20-compat triggered in libitm/; I sent a patch for that.
> 
> 	DR 2237
> 	PR c++/107126
> 	PR c++/97202
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
> 	turn on -Wdeprecated-template-id-cdtor.
> 	* c.opt (Wdeprecated-template-id-cdtor): New.
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
> 	a pedwarn.
> 	(cp_parser_constructor_declarator_p): Likewise.
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 3748ccd49ff..473eaf4f1f7 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -6717,12 +6717,17 @@ cp_parser_unqualified_id (cp_parser* parser,
>   
>   	/* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
>   	   declarator-id of a constructor or destructor.  */
> -	if (token->type == CPP_TEMPLATE_ID && declarator_p
> -	    && cxx_dialect >= cxx20)
> +	if (token->type == CPP_TEMPLATE_ID
> +	    && declarator_p
> +	    && !cp_parser_uncommitted_to_tentative_parse_p (parser)
> +	    && !cp_parser_simulate_error (parser))

Calling both _uncommitted_ and _simulate_ here is redundant since 
_simulate_ checks _uncommitted_; since you check _uncommitted_ first, 
_simulate_ will never do anything.

And checking either of them before cxx_dialect seems wrong; we don't 
want _simulate_ in pre-C++20 mode where it isn't an error, but we do 
want _simulate_ in C++20 mode when uncommitted.

> @@ -32550,6 +32555,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
>     /* We did not really want to consume any tokens.  */
>     cp_parser_abort_tentative_parse (parser);
>   
> +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> +     declarator-id of a constructor or destructor.  */
> +  if (constructor_p
> +      && saw_template_id
> +      && !cp_parser_uncommitted_to_tentative_parse_p (parser))

As above, checking _uncommitted_ before cxx_dialect seems wrong for C++20.

If testing succeeded with this patch, maybe we can't get here while 
parsing tentatively, and we just want a checking_assert (!_uncommitted_) 
in both places?  Or would that break on 97202?

Jason


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] c++: DR2237, cdtor and template-id tweaks [PR107126]
  2024-02-07 22:19     ` Jason Merrill
@ 2024-02-08 21:26       ` Marek Polacek
  2024-02-09 15:20         ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2024-02-08 21:26 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Wed, Feb 07, 2024 at 05:19:56PM -0500, Jason Merrill wrote:
> On 2/5/24 22:11, Marek Polacek wrote:
> > On Mon, Feb 05, 2024 at 10:14:34AM -0500, Jason Merrill wrote:
> > > On 2/3/24 10:24, Marek Polacek wrote:
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > 
> > > > I'm not certain OPT_Wc__20_extensions is the best thing for something
> > > > from [diff.cpp17]; would you prefer something else?
> > > 
> > > I think it wants its own flag, that is enabled in C++20 or by
> > > -Wc++20-compat.
> > 
> > That seems best.  I called it -Wdeprecated-template-id-cdtor.
> > > > +	    if (cxx_dialect >= cxx20)
> > > > +	      {
> > > > +		if (!cp_parser_simulate_error (parser))
> > > > +		  pedwarn (tilde_loc, OPT_Wc__20_extensions,
> > > > +			   "template-id not allowed for destructor");
> > > > +		return error_mark_node;
> > > > +	      }
> > > > +	    warning_at (tilde_loc, OPT_Wc__20_compat,
> > > > +			"template-id not allowed for destructor in C++20");
> > > 
> > > After a pedwarn we should accept the code, not return error_mark_node.
> > 
> > /facepalm, yes.
> > > I'm also concerned about pedwarn/warnings not guarded by
> > > !cp_parser_uncommited_to_tentative_parse; that often leads to warning about
> > > a tentative parse as a declaration that is eventually abandoned in favor of
> > > a perfectly fine parse as an expression.
> > 
> > Done.
> > > It would be good for cp_parser_context to add a vec of warnings to emit at
> > > cp_parser_parse_definitely time, and then
> > > cp_parser_pedwarn/cp_parser_warning to fill it...
> > 
> > That would be nice; I don't think we can fix bugs like PR61259 otherwise.
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > Since my r11-532 changes to implement DR2237, for this test:
> > 
> >    template<typename T>
> >    struct S {
> >      S<T>();
> >    };
> > 
> > in C++20 we emit the ugly:
> > 
> > q.C:3:8: error: expected unqualified-id before ')' token
> >      3 |   S<T>();
> > 
> > which doesn't explain what the problem is.  This patch improves that
> > diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
> > diagnostic.  We now say:
> > 
> > q.C:3:7: warning: template-id not allowed for constructor in C++20 [-Wdeprecated-template-id-cdtor]
> >      3 |   S<T>();
> > q.C:3:7: note: remove the '< >'
> > 
> > This patch does *not* fix
> > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
> > where the C++20 diagnostic is missing altogether.
> > 
> > -Wc++20-compat triggered in libitm/; I sent a patch for that.
> > 
> > 	DR 2237
> > 	PR c++/107126
> > 	PR c++/97202
> > 
> > gcc/c-family/ChangeLog:
> > 
> > 	* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
> > 	turn on -Wdeprecated-template-id-cdtor.
> > 	* c.opt (Wdeprecated-template-id-cdtor): New.
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
> > 	a pedwarn.
> > 	(cp_parser_constructor_declarator_p): Likewise.
> > 
> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > index 3748ccd49ff..473eaf4f1f7 100644
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > @@ -6717,12 +6717,17 @@ cp_parser_unqualified_id (cp_parser* parser,
> >   	/* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> >   	   declarator-id of a constructor or destructor.  */
> > -	if (token->type == CPP_TEMPLATE_ID && declarator_p
> > -	    && cxx_dialect >= cxx20)
> > +	if (token->type == CPP_TEMPLATE_ID
> > +	    && declarator_p
> > +	    && !cp_parser_uncommitted_to_tentative_parse_p (parser)
> > +	    && !cp_parser_simulate_error (parser))
> 
> Calling both _uncommitted_ and _simulate_ here is redundant since _simulate_
> checks _uncommitted_; since you check _uncommitted_ first, _simulate_ will
> never do anything.

Oops, pardon me.
 
> And checking either of them before cxx_dialect seems wrong; we don't want
> _simulate_ in pre-C++20 mode where it isn't an error, but we do want
> _simulate_ in C++20 mode when uncommitted.

I see.

> > @@ -32550,6 +32555,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
> >     /* We did not really want to consume any tokens.  */
> >     cp_parser_abort_tentative_parse (parser);
> > +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> > +     declarator-id of a constructor or destructor.  */
> > +  if (constructor_p
> > +      && saw_template_id
> > +      && !cp_parser_uncommitted_to_tentative_parse_p (parser))
> 
> As above, checking _uncommitted_ before cxx_dialect seems wrong for C++20.
> 
> If testing succeeded with this patch, maybe we can't get here while parsing
> tentatively, and we just want a checking_assert (!_uncommitted_) in both
> places?  Or would that break on 97202?

I can add the assert to the second spot, but adding it to the first one
causes crashes.  So maybe like this, then?

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Since my r11-532 changes to implement DR2237, for this test:

  template<typename T>
  struct S {
    S<T>();
  };

in C++20 we emit the ugly:

q.C:3:8: error: expected unqualified-id before ')' token
    3 |   S<T>();

which doesn't explain what the problem is.  This patch improves that
diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
diagnostic.  We now say:

q.C:3:7: warning: template-id not allowed for constructor in C++20 [-Wdeprecated-template-id-cdtor]
    3 |   S<T>();
q.C:3:7: note: remove the '< >'

This patch does *not* fix
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
where the C++20 diagnostic is missing altogether.

-Wc++20-compat triggered in libitm/; I sent a patch for that.

	DR 2237
	PR c++/107126
	PR c++/97202

gcc/c-family/ChangeLog:

	* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
	turn on -Wdeprecated-template-id-cdtor.
	* c.opt (Wdeprecated-template-id-cdtor): New.

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
	a pedwarn.
	(cp_parser_constructor_declarator_p): Likewise.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.

gcc/testsuite/ChangeLog:

	* g++.dg/DRs/dr2237.C: Adjust dg-error.
	* g++.dg/parse/constructor2.C: Likewise.
	* g++.dg/template/error34.C: Likewise.
	* g++.old-deja/g++.pt/ctor2.C: Likewise.
	* g++.dg/DRs/dr2237-2.C: New test.
	* g++.dg/DRs/dr2237-3.C: New test.
	* g++.dg/DRs/dr2237-4.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-1.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-2.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-3.C: New test.
	* g++.dg/warn/Wdeprecated-template-id-cdtor-4.C: New test.
---
 gcc/c-family/c-opts.cc                        |  5 +++
 gcc/c-family/c.opt                            |  4 +++
 gcc/cp/parser.cc                              | 36 ++++++++++++++-----
 gcc/doc/invoke.texi                           | 18 ++++++++++
 gcc/testsuite/g++.dg/DRs/dr2237-2.C           |  9 +++++
 gcc/testsuite/g++.dg/DRs/dr2237-3.C           | 16 +++++++++
 gcc/testsuite/g++.dg/DRs/dr2237-4.C           | 11 ++++++
 gcc/testsuite/g++.dg/DRs/dr2237.C             |  2 +-
 gcc/testsuite/g++.dg/parse/constructor2.C     | 16 ++++-----
 gcc/testsuite/g++.dg/template/error34.C       | 10 +++---
 .../warn/Wdeprecated-template-id-cdtor-1.C    |  9 +++++
 .../warn/Wdeprecated-template-id-cdtor-2.C    |  9 +++++
 .../warn/Wdeprecated-template-id-cdtor-3.C    |  9 +++++
 .../warn/Wdeprecated-template-id-cdtor-4.C    |  9 +++++
 gcc/testsuite/g++.old-deja/g++.pt/ctor2.C     |  2 +-
 15 files changed, 142 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-2.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-3.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-4.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C

diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index b845aff2226..d2358e2009a 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -998,6 +998,11 @@ c_common_post_options (const char **pfilename)
 		       warn_deprecated_enum_float_conv,
 		       cxx_dialect >= cxx20 && warn_deprecated);
 
+  /* -Wdeprecated-template-id-cdtor is enabled by default in C++20.  */
+  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
+		       warn_deprecated_template_id_cdtor,
+		       cxx_dialect >= cxx20 || warn_cxx20_compat);
+
   /* Declone C++ 'structors if -Os.  */
   if (flag_declone_ctor_dtor == -1)
     flag_declone_ctor_dtor = optimize_size;
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9c0a28092fc..2e1d07b66de 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -642,6 +642,10 @@ C++ ObjC++ Var(warn_deprecated_enum_float_conv) Warning
 Warn about deprecated arithmetic conversions on operands where one is of enumeration
 type and the other is of a floating-point type.
 
+Wdeprecated-template-id-cdtor
+C++ ObjC++ Var(warn_deprecated_template_id_cdtor) Warning
+Warn about deprecated simple-template-id in a constructor or destructor.
+
 Wdesignated-init
 C ObjC Var(warn_designated_init) Init(1) Warning
 Warn about positional initialization of structs requiring designated initializers.
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index c4292c49ce2..759ba7e54c4 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -6719,12 +6719,19 @@ cp_parser_unqualified_id (cp_parser* parser,
 
 	/* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
 	   declarator-id of a constructor or destructor.  */
-	if (token->type == CPP_TEMPLATE_ID && declarator_p
-	    && cxx_dialect >= cxx20)
+	if (token->type == CPP_TEMPLATE_ID && declarator_p)
 	  {
-	    if (!cp_parser_simulate_error (parser))
-	      error_at (tilde_loc, "template-id not allowed for destructor");
-	    return error_mark_node;
+	    auto_diagnostic_group d;
+	    bool w = false;
+	    if (cxx_dialect >= cxx20 && !cp_parser_simulate_error (parser))
+	      w = pedwarn (tilde_loc, OPT_Wdeprecated_template_id_cdtor,
+			   "template-id not allowed for destructor in C++20");
+	    else if (cxx_dialect < cxx20
+		     && !cp_parser_uncommitted_to_tentative_parse_p (parser))
+	      w = warning_at (tilde_loc, OPT_Wdeprecated_template_id_cdtor,
+			      "template-id not allowed for destructor in C++20");
+	    if (w)
+	      inform (tilde_loc, "remove the %qs", "< >");
 	  }
 
 	/* If there was an explicit qualification (S::~T), first look
@@ -32331,11 +32338,11 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   if (next_token->type != CPP_NAME
       && next_token->type != CPP_SCOPE
       && next_token->type != CPP_NESTED_NAME_SPECIFIER
-      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
-	 declarator-id of a constructor or destructor.  */
-      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
+      && next_token->type != CPP_TEMPLATE_ID)
     return false;
 
+  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);
+
   /* Parse tentatively; we are going to roll back all of the tokens
      consumed here.  */
   cp_parser_parse_tentatively (parser);
@@ -32552,6 +32559,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   /* We did not really want to consume any tokens.  */
   cp_parser_abort_tentative_parse (parser);
 
+  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
+     declarator-id of a constructor or destructor.  */
+  if (constructor_p && saw_template_id)
+    {
+      gcc_checking_assert
+	(!cp_parser_uncommitted_to_tentative_parse_p (parser));
+      auto_diagnostic_group d;
+      if (emit_diagnostic (cxx_dialect >= cxx20 ? DK_PEDWARN : DK_WARNING,
+			   input_location, OPT_Wdeprecated_template_id_cdtor,
+			   "template-id not allowed for constructor in C++20"))
+	inform (input_location, "remove the %qs", "< >");
+    }
+
   return constructor_p;
 }
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 71339b8b30f..5bd65bd02be 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -255,6 +255,7 @@ in the following sections.
 -Wdelete-non-virtual-dtor  -Wno-deprecated-array-compare
 -Wdeprecated-copy -Wdeprecated-copy-dtor
 -Wno-deprecated-enum-enum-conversion -Wno-deprecated-enum-float-conversion
+-Wno-deprecated-template-id-cdtor
 -Weffc++ -Wno-elaborated-enum-base
 -Wno-exceptions -Wextra-semi  -Wno-inaccessible-base
 -Wno-inherited-variadic-ctor  -Wno-init-list-lifetime
@@ -3989,6 +3990,23 @@ bool b = e <= 3.7;
 @option{-std=c++20}.  In pre-C++20 dialects, this warning can be enabled
 by @option{-Wenum-conversion}.
 
+@opindex Wdeprecated-template-id-cdtor
+@opindex Wno-deprecated-template-id-cdtor
+@item -Wno-deprecated-template-id-cdtor @r{(C++ and Objective-C++ only)}
+Disable the warning about the deprecated use of simple-template-id as the
+declarator-id of a constructor or destructor, which became invalid in C++20
+via DR 2237.  For example:
+
+@smallexample
+template<typename T> struct S @{
+  S<T>(); // should be S();
+  ~S<T>();  // should be ~S();
+@};
+@end smallexample
+
+@option{-Wdeprecated-template-id-cdtor} is enabled by default with
+@option{-std=c++20}; it is also enabled by @option{-Wc++20-compat}.
+
 @opindex Welaborated-enum-base
 @opindex Wno-elaborated-enum-base
 @item -Wno-elaborated-enum-base
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-2.C b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
new file mode 100644
index 00000000000..1d99347229c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
@@ -0,0 +1,9 @@
+// DR 2237 - Can a template-id name a constructor?
+// { dg-options "" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+  X(int); // OK, injected-class-name used
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+};
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-3.C b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
new file mode 100644
index 00000000000..c8ad6852389
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
@@ -0,0 +1,16 @@
+// PR c++/107126
+// { dg-options "" }
+
+template<typename T>
+struct C
+{
+    ~C();
+};
+template<typename T>
+C<T>::~C<T>()	      // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+{
+}
+int main()
+{
+    C<int> c;;
+}
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-4.C b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
new file mode 100644
index 00000000000..a358dd521b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
@@ -0,0 +1,11 @@
+// PR c++/97202
+// { dg-options "" }
+
+template<typename T>
+struct F
+{
+  F<T>();  // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+};
+
+template<typename T>
+inline F<T>::F() { }
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237.C b/gcc/testsuite/g++.dg/DRs/dr2237.C
index f3d6d11e61e..830c8f5a2a6 100644
--- a/gcc/testsuite/g++.dg/DRs/dr2237.C
+++ b/gcc/testsuite/g++.dg/DRs/dr2237.C
@@ -2,7 +2,7 @@
 
 template<class T>
 struct X {
-  X<T>(); // { dg-error "expected" "" { target c++20 } }
+  X<T>(); // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
   X(int); // OK, injected-class-name used
   ~X<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++20 } }
 };
diff --git a/gcc/testsuite/g++.dg/parse/constructor2.C b/gcc/testsuite/g++.dg/parse/constructor2.C
index d620f41e027..e4b2ad3f2fe 100644
--- a/gcc/testsuite/g++.dg/parse/constructor2.C
+++ b/gcc/testsuite/g++.dg/parse/constructor2.C
@@ -1,11 +1,11 @@
 // PR c++/14260
 
-template <class TClass> 
-class T 
-{ 
-public: 
-  T(short,short f=0) {} 
-  T<TClass>(int f) {} // { dg-error "expected" "" { target c++20 } }
-  T<TClass>(int f=0,const char* b=0) {} // { dg-error "expected" "" { target c++20 } }
-}; 
+template <class TClass>
+class T
+{
+public:
+  T(short,short f=0) {}
+  T<TClass>(int f) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+  T<TClass>(int f=0,const char* b=0) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+};
 
diff --git a/gcc/testsuite/g++.dg/template/error34.C b/gcc/testsuite/g++.dg/template/error34.C
index ab688d9ba8c..921cb8fb729 100644
--- a/gcc/testsuite/g++.dg/template/error34.C
+++ b/gcc/testsuite/g++.dg/template/error34.C
@@ -3,27 +3,27 @@
 
 template<typename T> struct A
 {
-  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|expected" }
+  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|template-id" }
 };
 
 template<typename T> struct B
 {
-  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|expected" }
+  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|template-id" }
 };
 
 template<typename T> struct C
 {
-  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|expected" }
+  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|template-id" }
 };
 
 template<typename T> struct D
 {
-  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|expected" }
+  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|template-id" }
 };
 
 struct E { int x; };
 
 template<typename T> struct F
 {
-  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|expected" }
+  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|template-id" }
 };
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C
new file mode 100644
index 00000000000..429490708bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-1.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wc++20-compat" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C
new file mode 100644
index 00000000000..d76b647c615
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-2.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wdeprecated-template-id-cdtor" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C
new file mode 100644
index 00000000000..80fc2ff7739
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-3.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wc++20-compat -Wno-deprecated-template-id-cdtor" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-bogus "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-bogus "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C
new file mode 100644
index 00000000000..6f1747701da
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wdeprecated-template-id-cdtor-4.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wdeprecated-template-id-cdtor -Wno-c++20-compat" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
index bf418ba48c7..56b4232d04b 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
@@ -4,7 +4,7 @@
 
 template <class T>
 struct A {
-  A<T>(); // { dg-error "expected" "" { target c++20 } }
+  A<T>(); // { dg-error "template-id" "" { target c++20 } }
 };
 
 template <class T>

base-commit: 232ffb61ceb63c4a86a8bc67f1db989c2124db5f
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] c++: DR2237, cdtor and template-id tweaks [PR107126]
  2024-02-08 21:26       ` [PATCH v3] " Marek Polacek
@ 2024-02-09 15:20         ` Jason Merrill
  2024-02-09 23:46           ` [PATCH v4] " Marek Polacek
  0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2024-02-09 15:20 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 2/8/24 16:26, Marek Polacek wrote:
> This patch does *not* fix
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
> where the C++20 diagnostic is missing altogether.

What would it take to fix that as well?

> 	* doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.

Was it ever deprecated?  I'm not seeing that in

https://timsong-cpp.github.io/cppwp/n4659/#depr (C++17)

Let's drop the word "deprecated" from the option name and documentation.

> @@ -32331,11 +32338,11 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
>     if (next_token->type != CPP_NAME
>         && next_token->type != CPP_SCOPE
>         && next_token->type != CPP_NESTED_NAME_SPECIFIER
> -      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> -	 declarator-id of a constructor or destructor.  */
> -      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
> +      && next_token->type != CPP_TEMPLATE_ID)
>       return false;
>   
> +  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);

Please incorporate your "further DR2237 fix" patch into this one.

> @@ -32552,6 +32559,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
>     /* We did not really want to consume any tokens.  */
>     cp_parser_abort_tentative_parse (parser);
>   
> +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> +     declarator-id of a constructor or destructor.  */
> +  if (constructor_p && saw_template_id)
> +    {
> +      gcc_checking_assert
> +	(!cp_parser_uncommitted_to_tentative_parse_p (parser));

Now I see the abort_ just above, so this seems unnecessary after all.

Thanks,
Jason


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4] c++: DR2237, cdtor and template-id tweaks [PR107126]
  2024-02-09 15:20         ` Jason Merrill
@ 2024-02-09 23:46           ` Marek Polacek
  2024-02-09 23:59             ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2024-02-09 23:46 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Fri, Feb 09, 2024 at 10:20:04AM -0500, Jason Merrill wrote:
> On 2/8/24 16:26, Marek Polacek wrote:
> > This patch does *not* fix
> > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
> > where the C++20 diagnostic is missing altogether.
> 
> What would it take to fix that as well?

It's the "further DR2237 fix" below which is basically a one liner.
I'd thought it would be more involved than that.
 
> > 	* doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.
> 
> Was it ever deprecated?  I'm not seeing that in
> 
> https://timsong-cpp.github.io/cppwp/n4659/#depr (C++17)

Aha, [diff] != [depr]...
 
> Let's drop the word "deprecated" from the option name and documentation.

Done throughout.
 
> > @@ -32331,11 +32338,11 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
> >     if (next_token->type != CPP_NAME
> >         && next_token->type != CPP_SCOPE
> >         && next_token->type != CPP_NESTED_NAME_SPECIFIER
> > -      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> > -	 declarator-id of a constructor or destructor.  */
> > -      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
> > +      && next_token->type != CPP_TEMPLATE_ID)
> >       return false;
> > +  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);
> 
> Please incorporate your "further DR2237 fix" patch into this one.

Patches squashed.
 
> > @@ -32552,6 +32559,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
> >     /* We did not really want to consume any tokens.  */
> >     cp_parser_abort_tentative_parse (parser);
> > +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> > +     declarator-id of a constructor or destructor.  */
> > +  if (constructor_p && saw_template_id)
> > +    {
> > +      gcc_checking_assert
> > +	(!cp_parser_uncommitted_to_tentative_parse_p (parser));
> 
> Now I see the abort_ just above, so this seems unnecessary after all.

Done, thanks.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Since my r11-532 changes to implement DR2237, for this test:

  template<typename T>
  struct S {
    S<T>();
  };

in C++20 we emit the ugly:

q.C:3:8: error: expected unqualified-id before ')' token
    3 |   S<T>();

which doesn't explain what the problem is.  This patch improves that
diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
diagnostic.  We now say:

q.C:3:7: warning: template-id not allowed for constructor in C++20 [-Wtemplate-id-cdtor]
    3 |   S<T>();
q.C:3:7: note: remove the '< >'

This patch also fixes
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
where the C++20 diagnostic was missing altogether: The problem was that I checked
for CPP_TEMPLATE_ID too early, at a point at which cp_parser_template_id may not
have been called yet.  So let's check for it at the end of the function, after
the tentative parse and rollback.

-Wc++20-compat triggered in libitm/; I sent a patch for that.

	DR 2237
	PR c++/107126
	PR c++/97202

gcc/c-family/ChangeLog:

	* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
	turn on -Wtemplate-id-cdtor.
	* c.opt (Wtemplate-id-cdtor): New.

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
	a pedwarn.
	(cp_parser_constructor_declarator_p): Likewise.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wtemplate-id-cdtor.

gcc/testsuite/ChangeLog:

	* g++.dg/DRs/dr2237.C: Adjust dg-error.
	* g++.dg/parse/constructor2.C: Likewise.
	* g++.dg/template/error34.C: Likewise.
	* g++.old-deja/g++.pt/ctor2.C: Likewise.
	* g++.dg/DRs/dr2237-2.C: New test.
	* g++.dg/DRs/dr2237-3.C: New test.
	* g++.dg/DRs/dr2237-4.C: New test.
	* g++.dg/DRs/dr2237-5.C: New test.
	* g++.dg/warn/Wtemplate-id-cdtor-1.C: New test.
	* g++.dg/warn/Wtemplate-id-cdtor-2.C: New test.
	* g++.dg/warn/Wtemplate-id-cdtor-3.C: New test.
	* g++.dg/warn/Wtemplate-id-cdtor-4.C: New test.
---
 gcc/c-family/c-opts.cc                        |  5 +++
 gcc/c-family/c.opt                            |  4 +++
 gcc/cp/parser.cc                              | 33 ++++++++++++++-----
 gcc/doc/invoke.texi                           | 19 ++++++++++-
 gcc/testsuite/g++.dg/DRs/dr2237-2.C           |  9 +++++
 gcc/testsuite/g++.dg/DRs/dr2237-3.C           | 16 +++++++++
 gcc/testsuite/g++.dg/DRs/dr2237-4.C           | 11 +++++++
 gcc/testsuite/g++.dg/DRs/dr2237-5.C           |  7 ++++
 gcc/testsuite/g++.dg/DRs/dr2237.C             |  2 +-
 gcc/testsuite/g++.dg/parse/constructor2.C     | 16 ++++-----
 gcc/testsuite/g++.dg/template/error34.C       | 10 +++---
 .../g++.dg/warn/Wtemplate-id-cdtor-1.C        |  9 +++++
 .../g++.dg/warn/Wtemplate-id-cdtor-2.C        |  9 +++++
 .../g++.dg/warn/Wtemplate-id-cdtor-3.C        |  9 +++++
 .../g++.dg/warn/Wtemplate-id-cdtor-4.C        |  9 +++++
 gcc/testsuite/g++.old-deja/g++.pt/ctor2.C     |  2 +-
 16 files changed, 146 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-2.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-3.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-4.C
 create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-5.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C

diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index b845aff2226..be3058dca63 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -998,6 +998,11 @@ c_common_post_options (const char **pfilename)
 		       warn_deprecated_enum_float_conv,
 		       cxx_dialect >= cxx20 && warn_deprecated);
 
+  /* -Wtemplate-id-cdtor is enabled by default in C++20.  */
+  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
+		       warn_template_id_cdtor,
+		       cxx_dialect >= cxx20 || warn_cxx20_compat);
+
   /* Declone C++ 'structors if -Os.  */
   if (flag_declone_ctor_dtor == -1)
     flag_declone_ctor_dtor = optimize_size;
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9c0a28092fc..b7a4a1a68e3 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1408,6 +1408,10 @@ Wtautological-compare
 C ObjC C++ ObjC++ Var(warn_tautological_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn if a comparison always evaluates to true or false.
 
+Wtemplate-id-cdtor
+C++ ObjC++ Var(warn_template_id_cdtor) Warning
+Warn about simple-template-id in a constructor or destructor.
+
 Wterminate
 C++ ObjC++ Warning Var(warn_terminate) Init(1)
 Warn if a throw expression will always result in a call to terminate().
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 09ecfa23b5d..8bba2fdef96 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -6719,12 +6719,19 @@ cp_parser_unqualified_id (cp_parser* parser,
 
 	/* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
 	   declarator-id of a constructor or destructor.  */
-	if (token->type == CPP_TEMPLATE_ID && declarator_p
-	    && cxx_dialect >= cxx20)
+	if (token->type == CPP_TEMPLATE_ID && declarator_p)
 	  {
-	    if (!cp_parser_simulate_error (parser))
-	      error_at (tilde_loc, "template-id not allowed for destructor");
-	    return error_mark_node;
+	    auto_diagnostic_group d;
+	    bool w = false;
+	    if (cxx_dialect >= cxx20 && !cp_parser_simulate_error (parser))
+	      w = pedwarn (tilde_loc, OPT_Wtemplate_id_cdtor,
+			   "template-id not allowed for destructor in C++20");
+	    else if (cxx_dialect < cxx20
+		     && !cp_parser_uncommitted_to_tentative_parse_p (parser))
+	      w = warning_at (tilde_loc, OPT_Wtemplate_id_cdtor,
+			      "template-id not allowed for destructor in C++20");
+	    if (w)
+	      inform (tilde_loc, "remove the %qs", "< >");
 	  }
 
 	/* If there was an explicit qualification (S::~T), first look
@@ -32331,9 +32338,7 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   if (next_token->type != CPP_NAME
       && next_token->type != CPP_SCOPE
       && next_token->type != CPP_NESTED_NAME_SPECIFIER
-      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
-	 declarator-id of a constructor or destructor.  */
-      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
+      && next_token->type != CPP_TEMPLATE_ID)
     return false;
 
   /* Parse tentatively; we are going to roll back all of the tokens
@@ -32552,6 +32557,18 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
   /* We did not really want to consume any tokens.  */
   cp_parser_abort_tentative_parse (parser);
 
+  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
+     declarator-id of a constructor or destructor.  */
+  if (constructor_p
+      && cp_lexer_peek_token (parser->lexer)->type == CPP_TEMPLATE_ID)
+    {
+      auto_diagnostic_group d;
+      if (emit_diagnostic (cxx_dialect >= cxx20 ? DK_PEDWARN : DK_WARNING,
+			   input_location, OPT_Wtemplate_id_cdtor,
+			   "template-id not allowed for constructor in C++20"))
+	inform (input_location, "remove the %qs", "< >");
+    }
+
   return constructor_p;
 }
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 71339b8b30f..0de184f6241 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -270,7 +270,7 @@ in the following sections.
 -Wno-non-template-friend  -Wold-style-cast
 -Woverloaded-virtual  -Wno-pmf-conversions -Wself-move -Wsign-promo
 -Wsized-deallocation  -Wsuggest-final-methods
--Wsuggest-final-types  -Wsuggest-override
+-Wsuggest-final-types  -Wsuggest-override  -Wno-template-id-cdtor
 -Wno-terminate  -Wno-vexing-parse  -Wvirtual-inheritance
 -Wno-virtual-move-assign  -Wvolatile  -Wzero-as-null-pointer-constant}
 
@@ -4604,6 +4604,23 @@ namespaces, and this may be used to enforce that rule.  The warning is
 inactive inside a system header file, such as the STL, so one can still
 use the STL.  One may also use using directives and qualified names.
 
+@opindex Wtemplate-id-cdtor
+@opindex Wno-template-id-cdtor
+@item -Wno-template-id-cdtor @r{(C++ and Objective-C++ only)}
+Disable the warning about the use of simple-template-id as the declarator-id
+of a constructor or destructor, which became invalid in C++20 via DR 2237.
+For example:
+
+@smallexample
+template<typename T> struct S @{
+  S<T>(); // should be S();
+  ~S<T>();  // should be ~S();
+@};
+@end smallexample
+
+@option{-Wtemplate-id-cdtor} is enabled by default with
+@option{-std=c++20}; it is also enabled by @option{-Wc++20-compat}.
+
 @opindex Wterminate
 @opindex Wno-terminate
 @item -Wno-terminate @r{(C++ and Objective-C++ only)}
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-2.C b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
new file mode 100644
index 00000000000..1d99347229c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
@@ -0,0 +1,9 @@
+// DR 2237 - Can a template-id name a constructor?
+// { dg-options "" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+  X(int); // OK, injected-class-name used
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+};
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-3.C b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
new file mode 100644
index 00000000000..c8ad6852389
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
@@ -0,0 +1,16 @@
+// PR c++/107126
+// { dg-options "" }
+
+template<typename T>
+struct C
+{
+    ~C();
+};
+template<typename T>
+C<T>::~C<T>()	      // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+{
+}
+int main()
+{
+    C<int> c;;
+}
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-4.C b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
new file mode 100644
index 00000000000..a358dd521b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
@@ -0,0 +1,11 @@
+// PR c++/97202
+// { dg-options "" }
+
+template<typename T>
+struct F
+{
+  F<T>();  // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+};
+
+template<typename T>
+inline F<T>::F() { }
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-5.C b/gcc/testsuite/g++.dg/DRs/dr2237-5.C
new file mode 100644
index 00000000000..fd51968f7e1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2237-5.C
@@ -0,0 +1,7 @@
+// PR c++/97202
+// { dg-options "" }
+
+template<typename Base> struct S : Base {
+  inline S<Base>() {} // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
+  inline ~S<Base>() {} // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
+};
diff --git a/gcc/testsuite/g++.dg/DRs/dr2237.C b/gcc/testsuite/g++.dg/DRs/dr2237.C
index f3d6d11e61e..830c8f5a2a6 100644
--- a/gcc/testsuite/g++.dg/DRs/dr2237.C
+++ b/gcc/testsuite/g++.dg/DRs/dr2237.C
@@ -2,7 +2,7 @@
 
 template<class T>
 struct X {
-  X<T>(); // { dg-error "expected" "" { target c++20 } }
+  X<T>(); // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
   X(int); // OK, injected-class-name used
   ~X<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++20 } }
 };
diff --git a/gcc/testsuite/g++.dg/parse/constructor2.C b/gcc/testsuite/g++.dg/parse/constructor2.C
index d620f41e027..e4b2ad3f2fe 100644
--- a/gcc/testsuite/g++.dg/parse/constructor2.C
+++ b/gcc/testsuite/g++.dg/parse/constructor2.C
@@ -1,11 +1,11 @@
 // PR c++/14260
 
-template <class TClass> 
-class T 
-{ 
-public: 
-  T(short,short f=0) {} 
-  T<TClass>(int f) {} // { dg-error "expected" "" { target c++20 } }
-  T<TClass>(int f=0,const char* b=0) {} // { dg-error "expected" "" { target c++20 } }
-}; 
+template <class TClass>
+class T
+{
+public:
+  T(short,short f=0) {}
+  T<TClass>(int f) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+  T<TClass>(int f=0,const char* b=0) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
+};
 
diff --git a/gcc/testsuite/g++.dg/template/error34.C b/gcc/testsuite/g++.dg/template/error34.C
index ab688d9ba8c..921cb8fb729 100644
--- a/gcc/testsuite/g++.dg/template/error34.C
+++ b/gcc/testsuite/g++.dg/template/error34.C
@@ -3,27 +3,27 @@
 
 template<typename T> struct A
 {
-  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|expected" }
+  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|template-id" }
 };
 
 template<typename T> struct B
 {
-  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|expected" }
+  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|template-id" }
 };
 
 template<typename T> struct C
 {
-  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|expected" }
+  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|template-id" }
 };
 
 template<typename T> struct D
 {
-  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|expected" }
+  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|template-id" }
 };
 
 struct E { int x; };
 
 template<typename T> struct F
 {
-  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|expected" }
+  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|template-id" }
 };
diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C
new file mode 100644
index 00000000000..429490708bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wc++20-compat" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C
new file mode 100644
index 00000000000..2b1c4ea8222
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wtemplate-id-cdtor" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C
new file mode 100644
index 00000000000..bed96e84bd2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wc++20-compat -Wno-template-id-cdtor" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-bogus "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-bogus "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C
new file mode 100644
index 00000000000..706e5746e60
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C
@@ -0,0 +1,9 @@
+// PR c++/107126
+// { dg-do compile }
+// { dg-options "-Wtemplate-id-cdtor -Wno-c++20-compat" }
+
+template<class T>
+struct X {
+  X<T>(); // { dg-warning "template-id not allowed for constructor" }
+  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
+};
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
index bf418ba48c7..56b4232d04b 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
@@ -4,7 +4,7 @@
 
 template <class T>
 struct A {
-  A<T>(); // { dg-error "expected" "" { target c++20 } }
+  A<T>(); // { dg-error "template-id" "" { target c++20 } }
 };
 
 template <class T>

base-commit: f29f7f86935e29786bf9f976ec99d7639b381b14
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] c++: DR2237, cdtor and template-id tweaks [PR107126]
  2024-02-09 23:46           ` [PATCH v4] " Marek Polacek
@ 2024-02-09 23:59             ` Jason Merrill
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Merrill @ 2024-02-09 23:59 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 2/9/24 18:46, Marek Polacek wrote:
> On Fri, Feb 09, 2024 at 10:20:04AM -0500, Jason Merrill wrote:
>> On 2/8/24 16:26, Marek Polacek wrote:
>>> This patch does *not* fix
>>> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
>>> where the C++20 diagnostic is missing altogether.
>>
>> What would it take to fix that as well?
> 
> It's the "further DR2237 fix" below which is basically a one liner.
> I'd thought it would be more involved than that.
>   
>>> 	* doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.
>>
>> Was it ever deprecated?  I'm not seeing that in
>>
>> https://timsong-cpp.github.io/cppwp/n4659/#depr (C++17)
> 
> Aha, [diff] != [depr]...
>   
>> Let's drop the word "deprecated" from the option name and documentation.
> 
> Done throughout.
>   
>>> @@ -32331,11 +32338,11 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
>>>      if (next_token->type != CPP_NAME
>>>          && next_token->type != CPP_SCOPE
>>>          && next_token->type != CPP_NESTED_NAME_SPECIFIER
>>> -      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
>>> -	 declarator-id of a constructor or destructor.  */
>>> -      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
>>> +      && next_token->type != CPP_TEMPLATE_ID)
>>>        return false;
>>> +  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);
>>
>> Please incorporate your "further DR2237 fix" patch into this one.
> 
> Patches squashed.
>   
>>> @@ -32552,6 +32559,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
>>>      /* We did not really want to consume any tokens.  */
>>>      cp_parser_abort_tentative_parse (parser);
>>> +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
>>> +     declarator-id of a constructor or destructor.  */
>>> +  if (constructor_p && saw_template_id)
>>> +    {
>>> +      gcc_checking_assert
>>> +	(!cp_parser_uncommitted_to_tentative_parse_p (parser));
>>
>> Now I see the abort_ just above, so this seems unnecessary after all.
> 
> Done, thanks.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK, thanks.

> -- >8 --
> Since my r11-532 changes to implement DR2237, for this test:
> 
>    template<typename T>
>    struct S {
>      S<T>();
>    };
> 
> in C++20 we emit the ugly:
> 
> q.C:3:8: error: expected unqualified-id before ')' token
>      3 |   S<T>();
> 
> which doesn't explain what the problem is.  This patch improves that
> diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
> diagnostic.  We now say:
> 
> q.C:3:7: warning: template-id not allowed for constructor in C++20 [-Wtemplate-id-cdtor]
>      3 |   S<T>();
> q.C:3:7: note: remove the '< >'
> 
> This patch also fixes
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
> where the C++20 diagnostic was missing altogether: The problem was that I checked
> for CPP_TEMPLATE_ID too early, at a point at which cp_parser_template_id may not
> have been called yet.  So let's check for it at the end of the function, after
> the tentative parse and rollback.
> 
> -Wc++20-compat triggered in libitm/; I sent a patch for that.
> 
> 	DR 2237
> 	PR c++/107126
> 	PR c++/97202
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
> 	turn on -Wtemplate-id-cdtor.
> 	* c.opt (Wtemplate-id-cdtor): New.
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
> 	a pedwarn.
> 	(cp_parser_constructor_declarator_p): Likewise.
> 
> gcc/ChangeLog:
> 
> 	* doc/invoke.texi: Document -Wtemplate-id-cdtor.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/DRs/dr2237.C: Adjust dg-error.
> 	* g++.dg/parse/constructor2.C: Likewise.
> 	* g++.dg/template/error34.C: Likewise.
> 	* g++.old-deja/g++.pt/ctor2.C: Likewise.
> 	* g++.dg/DRs/dr2237-2.C: New test.
> 	* g++.dg/DRs/dr2237-3.C: New test.
> 	* g++.dg/DRs/dr2237-4.C: New test.
> 	* g++.dg/DRs/dr2237-5.C: New test.
> 	* g++.dg/warn/Wtemplate-id-cdtor-1.C: New test.
> 	* g++.dg/warn/Wtemplate-id-cdtor-2.C: New test.
> 	* g++.dg/warn/Wtemplate-id-cdtor-3.C: New test.
> 	* g++.dg/warn/Wtemplate-id-cdtor-4.C: New test.
> ---
>   gcc/c-family/c-opts.cc                        |  5 +++
>   gcc/c-family/c.opt                            |  4 +++
>   gcc/cp/parser.cc                              | 33 ++++++++++++++-----
>   gcc/doc/invoke.texi                           | 19 ++++++++++-
>   gcc/testsuite/g++.dg/DRs/dr2237-2.C           |  9 +++++
>   gcc/testsuite/g++.dg/DRs/dr2237-3.C           | 16 +++++++++
>   gcc/testsuite/g++.dg/DRs/dr2237-4.C           | 11 +++++++
>   gcc/testsuite/g++.dg/DRs/dr2237-5.C           |  7 ++++
>   gcc/testsuite/g++.dg/DRs/dr2237.C             |  2 +-
>   gcc/testsuite/g++.dg/parse/constructor2.C     | 16 ++++-----
>   gcc/testsuite/g++.dg/template/error34.C       | 10 +++---
>   .../g++.dg/warn/Wtemplate-id-cdtor-1.C        |  9 +++++
>   .../g++.dg/warn/Wtemplate-id-cdtor-2.C        |  9 +++++
>   .../g++.dg/warn/Wtemplate-id-cdtor-3.C        |  9 +++++
>   .../g++.dg/warn/Wtemplate-id-cdtor-4.C        |  9 +++++
>   gcc/testsuite/g++.old-deja/g++.pt/ctor2.C     |  2 +-
>   16 files changed, 146 insertions(+), 24 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-2.C
>   create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-3.C
>   create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-4.C
>   create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237-5.C
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C
> 
> diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
> index b845aff2226..be3058dca63 100644
> --- a/gcc/c-family/c-opts.cc
> +++ b/gcc/c-family/c-opts.cc
> @@ -998,6 +998,11 @@ c_common_post_options (const char **pfilename)
>   		       warn_deprecated_enum_float_conv,
>   		       cxx_dialect >= cxx20 && warn_deprecated);
>   
> +  /* -Wtemplate-id-cdtor is enabled by default in C++20.  */
> +  SET_OPTION_IF_UNSET (&global_options, &global_options_set,
> +		       warn_template_id_cdtor,
> +		       cxx_dialect >= cxx20 || warn_cxx20_compat);
> +
>     /* Declone C++ 'structors if -Os.  */
>     if (flag_declone_ctor_dtor == -1)
>       flag_declone_ctor_dtor = optimize_size;
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 9c0a28092fc..b7a4a1a68e3 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1408,6 +1408,10 @@ Wtautological-compare
>   C ObjC C++ ObjC++ Var(warn_tautological_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
>   Warn if a comparison always evaluates to true or false.
>   
> +Wtemplate-id-cdtor
> +C++ ObjC++ Var(warn_template_id_cdtor) Warning
> +Warn about simple-template-id in a constructor or destructor.
> +
>   Wterminate
>   C++ ObjC++ Warning Var(warn_terminate) Init(1)
>   Warn if a throw expression will always result in a call to terminate().
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 09ecfa23b5d..8bba2fdef96 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -6719,12 +6719,19 @@ cp_parser_unqualified_id (cp_parser* parser,
>   
>   	/* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
>   	   declarator-id of a constructor or destructor.  */
> -	if (token->type == CPP_TEMPLATE_ID && declarator_p
> -	    && cxx_dialect >= cxx20)
> +	if (token->type == CPP_TEMPLATE_ID && declarator_p)
>   	  {
> -	    if (!cp_parser_simulate_error (parser))
> -	      error_at (tilde_loc, "template-id not allowed for destructor");
> -	    return error_mark_node;
> +	    auto_diagnostic_group d;
> +	    bool w = false;
> +	    if (cxx_dialect >= cxx20 && !cp_parser_simulate_error (parser))
> +	      w = pedwarn (tilde_loc, OPT_Wtemplate_id_cdtor,
> +			   "template-id not allowed for destructor in C++20");
> +	    else if (cxx_dialect < cxx20
> +		     && !cp_parser_uncommitted_to_tentative_parse_p (parser))
> +	      w = warning_at (tilde_loc, OPT_Wtemplate_id_cdtor,
> +			      "template-id not allowed for destructor in C++20");
> +	    if (w)
> +	      inform (tilde_loc, "remove the %qs", "< >");
>   	  }
>   
>   	/* If there was an explicit qualification (S::~T), first look
> @@ -32331,9 +32338,7 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
>     if (next_token->type != CPP_NAME
>         && next_token->type != CPP_SCOPE
>         && next_token->type != CPP_NESTED_NAME_SPECIFIER
> -      /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> -	 declarator-id of a constructor or destructor.  */
> -      && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
> +      && next_token->type != CPP_TEMPLATE_ID)
>       return false;
>   
>     /* Parse tentatively; we are going to roll back all of the tokens
> @@ -32552,6 +32557,18 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
>     /* We did not really want to consume any tokens.  */
>     cp_parser_abort_tentative_parse (parser);
>   
> +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> +     declarator-id of a constructor or destructor.  */
> +  if (constructor_p
> +      && cp_lexer_peek_token (parser->lexer)->type == CPP_TEMPLATE_ID)
> +    {
> +      auto_diagnostic_group d;
> +      if (emit_diagnostic (cxx_dialect >= cxx20 ? DK_PEDWARN : DK_WARNING,
> +			   input_location, OPT_Wtemplate_id_cdtor,
> +			   "template-id not allowed for constructor in C++20"))
> +	inform (input_location, "remove the %qs", "< >");
> +    }
> +
>     return constructor_p;
>   }
>   
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 71339b8b30f..0de184f6241 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -270,7 +270,7 @@ in the following sections.
>   -Wno-non-template-friend  -Wold-style-cast
>   -Woverloaded-virtual  -Wno-pmf-conversions -Wself-move -Wsign-promo
>   -Wsized-deallocation  -Wsuggest-final-methods
> --Wsuggest-final-types  -Wsuggest-override
> +-Wsuggest-final-types  -Wsuggest-override  -Wno-template-id-cdtor
>   -Wno-terminate  -Wno-vexing-parse  -Wvirtual-inheritance
>   -Wno-virtual-move-assign  -Wvolatile  -Wzero-as-null-pointer-constant}
>   
> @@ -4604,6 +4604,23 @@ namespaces, and this may be used to enforce that rule.  The warning is
>   inactive inside a system header file, such as the STL, so one can still
>   use the STL.  One may also use using directives and qualified names.
>   
> +@opindex Wtemplate-id-cdtor
> +@opindex Wno-template-id-cdtor
> +@item -Wno-template-id-cdtor @r{(C++ and Objective-C++ only)}
> +Disable the warning about the use of simple-template-id as the declarator-id
> +of a constructor or destructor, which became invalid in C++20 via DR 2237.
> +For example:
> +
> +@smallexample
> +template<typename T> struct S @{
> +  S<T>(); // should be S();
> +  ~S<T>();  // should be ~S();
> +@};
> +@end smallexample
> +
> +@option{-Wtemplate-id-cdtor} is enabled by default with
> +@option{-std=c++20}; it is also enabled by @option{-Wc++20-compat}.
> +
>   @opindex Wterminate
>   @opindex Wno-terminate
>   @item -Wno-terminate @r{(C++ and Objective-C++ only)}
> diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-2.C b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
> new file mode 100644
> index 00000000000..1d99347229c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/DRs/dr2237-2.C
> @@ -0,0 +1,9 @@
> +// DR 2237 - Can a template-id name a constructor?
> +// { dg-options "" }
> +
> +template<class T>
> +struct X {
> +  X<T>(); // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
> +  X(int); // OK, injected-class-name used
> +  ~X<T>(); // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
> +};
> diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-3.C b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
> new file mode 100644
> index 00000000000..c8ad6852389
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/DRs/dr2237-3.C
> @@ -0,0 +1,16 @@
> +// PR c++/107126
> +// { dg-options "" }
> +
> +template<typename T>
> +struct C
> +{
> +    ~C();
> +};
> +template<typename T>
> +C<T>::~C<T>()	      // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
> +{
> +}
> +int main()
> +{
> +    C<int> c;;
> +}
> diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-4.C b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
> new file mode 100644
> index 00000000000..a358dd521b5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/DRs/dr2237-4.C
> @@ -0,0 +1,11 @@
> +// PR c++/97202
> +// { dg-options "" }
> +
> +template<typename T>
> +struct F
> +{
> +  F<T>();  // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
> +};
> +
> +template<typename T>
> +inline F<T>::F() { }
> diff --git a/gcc/testsuite/g++.dg/DRs/dr2237-5.C b/gcc/testsuite/g++.dg/DRs/dr2237-5.C
> new file mode 100644
> index 00000000000..fd51968f7e1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/DRs/dr2237-5.C
> @@ -0,0 +1,7 @@
> +// PR c++/97202
> +// { dg-options "" }
> +
> +template<typename Base> struct S : Base {
> +  inline S<Base>() {} // { dg-warning "template-id not allowed for constructor" "" { target c++20 } }
> +  inline ~S<Base>() {} // { dg-warning "template-id not allowed for destructor" "" { target c++20 } }
> +};
> diff --git a/gcc/testsuite/g++.dg/DRs/dr2237.C b/gcc/testsuite/g++.dg/DRs/dr2237.C
> index f3d6d11e61e..830c8f5a2a6 100644
> --- a/gcc/testsuite/g++.dg/DRs/dr2237.C
> +++ b/gcc/testsuite/g++.dg/DRs/dr2237.C
> @@ -2,7 +2,7 @@
>   
>   template<class T>
>   struct X {
> -  X<T>(); // { dg-error "expected" "" { target c++20 } }
> +  X<T>(); // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
>     X(int); // OK, injected-class-name used
>     ~X<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++20 } }
>   };
> diff --git a/gcc/testsuite/g++.dg/parse/constructor2.C b/gcc/testsuite/g++.dg/parse/constructor2.C
> index d620f41e027..e4b2ad3f2fe 100644
> --- a/gcc/testsuite/g++.dg/parse/constructor2.C
> +++ b/gcc/testsuite/g++.dg/parse/constructor2.C
> @@ -1,11 +1,11 @@
>   // PR c++/14260
>   
> -template <class TClass>
> -class T
> -{
> -public:
> -  T(short,short f=0) {}
> -  T<TClass>(int f) {} // { dg-error "expected" "" { target c++20 } }
> -  T<TClass>(int f=0,const char* b=0) {} // { dg-error "expected" "" { target c++20 } }
> -};
> +template <class TClass>
> +class T
> +{
> +public:
> +  T(short,short f=0) {}
> +  T<TClass>(int f) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
> +  T<TClass>(int f=0,const char* b=0) {} // { dg-error "template-id not allowed for constructor" "" { target c++20 } }
> +};
>   
> diff --git a/gcc/testsuite/g++.dg/template/error34.C b/gcc/testsuite/g++.dg/template/error34.C
> index ab688d9ba8c..921cb8fb729 100644
> --- a/gcc/testsuite/g++.dg/template/error34.C
> +++ b/gcc/testsuite/g++.dg/template/error34.C
> @@ -3,27 +3,27 @@
>   
>   template<typename T> struct A
>   {
> -  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|expected" }
> +  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|template-id" }
>   };
>   
>   template<typename T> struct B
>   {
> -  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|expected" }
> +  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|template-id" }
>   };
>   
>   template<typename T> struct C
>   {
> -  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|expected" }
> +  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|template-id" }
>   };
>   
>   template<typename T> struct D
>   {
> -  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|expected" }
> +  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|template-id" }
>   };
>   
>   struct E { int x; };
>   
>   template<typename T> struct F
>   {
> -  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|expected" }
> +  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|template-id" }
>   };
> diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C
> new file mode 100644
> index 00000000000..429490708bc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-1.C
> @@ -0,0 +1,9 @@
> +// PR c++/107126
> +// { dg-do compile }
> +// { dg-options "-Wc++20-compat" }
> +
> +template<class T>
> +struct X {
> +  X<T>(); // { dg-warning "template-id not allowed for constructor" }
> +  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
> +};
> diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C
> new file mode 100644
> index 00000000000..2b1c4ea8222
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-2.C
> @@ -0,0 +1,9 @@
> +// PR c++/107126
> +// { dg-do compile }
> +// { dg-options "-Wtemplate-id-cdtor" }
> +
> +template<class T>
> +struct X {
> +  X<T>(); // { dg-warning "template-id not allowed for constructor" }
> +  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
> +};
> diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C
> new file mode 100644
> index 00000000000..bed96e84bd2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-3.C
> @@ -0,0 +1,9 @@
> +// PR c++/107126
> +// { dg-do compile }
> +// { dg-options "-Wc++20-compat -Wno-template-id-cdtor" }
> +
> +template<class T>
> +struct X {
> +  X<T>(); // { dg-bogus "template-id not allowed for constructor" }
> +  ~X<T>(); // { dg-bogus "template-id not allowed for destructor" }
> +};
> diff --git a/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C
> new file mode 100644
> index 00000000000..706e5746e60
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wtemplate-id-cdtor-4.C
> @@ -0,0 +1,9 @@
> +// PR c++/107126
> +// { dg-do compile }
> +// { dg-options "-Wtemplate-id-cdtor -Wno-c++20-compat" }
> +
> +template<class T>
> +struct X {
> +  X<T>(); // { dg-warning "template-id not allowed for constructor" }
> +  ~X<T>(); // { dg-warning "template-id not allowed for destructor" }
> +};
> diff --git a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
> index bf418ba48c7..56b4232d04b 100644
> --- a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
> +++ b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
> @@ -4,7 +4,7 @@
>   
>   template <class T>
>   struct A {
> -  A<T>(); // { dg-error "expected" "" { target c++20 } }
> +  A<T>(); // { dg-error "template-id" "" { target c++20 } }
>   };
>   
>   template <class T>
> 
> base-commit: f29f7f86935e29786bf9f976ec99d7639b381b14


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-02-09 23:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-03 15:24 [PATCH] c++: DR2237, cdtor and template-id tweaks [PR107126] Marek Polacek
2024-02-05 15:14 ` Jason Merrill
2024-02-06  3:11   ` [PATCH v2] " Marek Polacek
2024-02-07 22:19     ` Jason Merrill
2024-02-08 21:26       ` [PATCH v3] " Marek Polacek
2024-02-09 15:20         ` Jason Merrill
2024-02-09 23:46           ` [PATCH v4] " Marek Polacek
2024-02-09 23:59             ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).