Re: [PATCH] c++: DR2237, cdtor and template-id tweaks [PR107126]

2024-02-05 Thread Jason Merrill 

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

[PATCH] c++: DR2237, cdtor and template-id tweaks [PR107126]

2024-02-03 Thread Marek Polacek 
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
  struct S {
S();
  };

in C++20 we emit the ugly:

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

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();

This patch does *not* fix

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