[v2] c++: DR2237, cdtor and template-id tweaks [PR107126]
Checks
Commit Message
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
base-commit: c5d34912ad576be1ef19be92f7eabde54b9089eb
Comments
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
@@ -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;
@@ -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.
@@ -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;
}
@@ -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
new file mode 100644
@@ -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 } }
+};
new file mode 100644
@@ -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;;
+}
new file mode 100644
@@ -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() { }
@@ -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 } }
};
@@ -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 } }
+};
@@ -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" }
};
new file mode 100644
@@ -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" }
+};
new file mode 100644
@@ -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" }
+};
new file mode 100644
@@ -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" }
+};
new file mode 100644
@@ -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" }
+};
@@ -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>