Re: [PATCH v2] c++: DR2237, cdtor and template-id tweaks [PR107126]
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
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 in C++20
[-Wdeprecated-template-id-cdtor]
3 | S();
q.C:3:7: note: remove the '< >'
This patch does *not* fix
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
[PATCH v2] c++: DR2237, cdtor and template-id tweaks [PR107126]
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
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 in C++20
[-Wdeprecated-template-id-cdtor]
3 | S();
q.C:3:7: note: remove the '< >'
This patch does *not* fix
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
+++
