Re: [PATCH] c++: C++20 DR 2237, disallow simple-template-id in cdtor.

[Marek Polacek <polacek@redhat.com>]

Ping.

On Mon, May 11, 2020 at 04:23:17PM -0400, Marek Polacek via Gcc-patches wrote:
> On Sun, Apr 05, 2020 at 09:46:09PM -0400, Jason Merrill wrote:
> > On 4/4/20 7:30 PM, Marek Polacek wrote:
> > > This patch implements DR 2237 which says that a simple-template-id is
> > > no longer valid as the declarator-id of a constructor or destructor;
> > > see <https://eel.is/c++draft/diff.cpp17.class#2>.  It is not explicitly
> > > stated but out-of-line destructors with a simple-template-id are also
> > > meant to be ill-formed now.  (Out-of-line constructors like that are
> > > invalid since DR1435 I think.)  This change only applies to C++20; it
> > > is not a DR against C++17.
> > > 
> > > I'm not crazy about the diagnostic in constructors but ISTM that
> > > cp_parser_constructor_declarator_p shouldn't print errors.
> > > 
> > > Does it seem reasonable to apply this now or should I defer to GCC 11?
> > 
> > A new error should wait for GCC 11.
> 
> Coming back to this now that we're again in stage 1.
> 
> > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > 
> > > 2020-04-04  Marek Polacek  <polacek@redhat.com>
> > > 
> > > 	DR 2237
> > > 	* parser.c (cp_parser_unqualified_id): Reject simple-template-id as
> > > 	the declarator-id of a destructor.
> > > 	(cp_parser_constructor_declarator_p): Reject simple-template-id as
> > > 	the declarator-id of a constructor.
> > > 
> > > 	* g++.dg/DRs/dr2237.C: New test.
> > > 	* g++.dg/parse/constructor2.C: Add dg-error for C++20.
> > > 	* g++.dg/parse/dtor12.C: Likewise.
> > > 	* g++.dg/parse/dtor4.C: Likewise.
> > > 	* g++.dg/template/dtor4.C: Adjust dg-error.
> > > 	* g++.dg/template/error34.C: Likewise.
> > > 	* g++.old-deja/g++.other/inline15.C: Only run for C++17 and lesses.
> > > 	* g++.old-deja/g++.pt/ctor2.C: Add dg-error for C++20.
> > > ---
> > >   gcc/cp/parser.c                                | 16 ++++++++++++++++
> > >   gcc/testsuite/g++.dg/DRs/dr2237.C              | 18 ++++++++++++++++++
> > >   gcc/testsuite/g++.dg/parse/constructor2.C      |  4 ++--
> > >   gcc/testsuite/g++.dg/parse/dtor12.C            |  2 +-
> > >   gcc/testsuite/g++.dg/parse/dtor4.C             |  2 +-
> > >   gcc/testsuite/g++.dg/template/dtor4.C          |  2 +-
> > >   gcc/testsuite/g++.dg/template/error34.C        | 10 +++++-----
> > >   .../g++.old-deja/g++.other/inline15.C          |  2 +-
> > >   gcc/testsuite/g++.old-deja/g++.pt/ctor2.C      |  2 +-
> > >   9 files changed, 46 insertions(+), 12 deletions(-)
> > >   create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237.C
> > > 
> > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > > index 7e5921e039f..810edfa87a9 100644
> > > --- a/gcc/cp/parser.c
> > > +++ b/gcc/cp/parser.c
> > > @@ -6114,6 +6114,16 @@ cp_parser_unqualified_id (cp_parser* parser,
> > >   	    return build_min_nt_loc (loc, BIT_NOT_EXPR, make_auto ());
> > >   	  }
> > > +	/* 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 && cxx_dialect >= cxx2a)
> > > +	  {
> > > +	    if (!cp_parser_uncommitted_to_tentative_parse_p (parser))
> > > +	      error_at (tilde_loc, "template-id not allowed for destructor");
> > > +	    cp_parser_simulate_error (parser);
> > 
> > The usual pattern is
> > 
> > if (!cp_parser_simulate_error (parser))
> >   error...
> 
> Fixed.
> 
> > > +	    return error_mark_node;
> > > +	  }
> > > +
> > >   	/* If there was an explicit qualification (S::~T), first look
> > >   	   in the scope given by the qualification (i.e., S).
> > > @@ -28675,6 +28685,12 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
> > >         if (!constructor_name_p (id, nested_name_specifier))
> > >   	constructor_p = false;
> > >       }
> > > +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> > > +     declarator-id of a constructor or destructor.  */
> > > +  else if (constructor_p
> > > +	   && cxx_dialect >= cxx2a
> > > +	   && cp_lexer_next_token_is (parser->lexer, CPP_TEMPLATE_ID))
> > > +    constructor_p = false;
> > >     /* If we still think that this might be a constructor-declarator,
> > >        look for a class-name.  */
> > >     else if (constructor_p)
> > 
> > Do you also want to exclude CPP_TEMPLATE_ID from the test at the top of the
> > function for C++20?
> 
> In fact we can get away with only excluding CPP_TEMPLATE_ID in C++20 there,
> because for e.g. X<T>::X<T>(); we'll give the "names the constructor, not the
> type" error, so we won't even get here.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> This patch implements DR 2237 which says that a simple-template-id is
> no longer valid as the declarator-id of a constructor or destructor;
> see [diff.cpp17.class]#2.  It is not explicitly stated but out-of-line
> destructors with a simple-template-id are also meant to be ill-formed
> now.  (Out-of-line constructors like that are invalid since DR1435 I
> think.)  This change only applies to C++20; it is not a DR against C++17.
> 
> I'm not crazy about the diagnostic in constructors but ISTM that
> cp_parser_constructor_declarator_p shouldn't print errors.
> 
> 	DR 2237
> 	* parser.c (cp_parser_unqualified_id): Reject simple-template-id as
> 	the declarator-id of a destructor.
> 	(cp_parser_constructor_declarator_p): Reject simple-template-id as
> 	the declarator-id of a constructor.
> 
> 	* g++.dg/DRs/dr2237.C: New test.
> 	* g++.dg/parse/constructor2.C: Add dg-error for C++20.
> 	* g++.dg/parse/dtor12.C: Likewise.
> 	* g++.dg/parse/dtor4.C: Likewise.
> 	* g++.dg/template/dtor4.C: Adjust dg-error.
> 	* g++.dg/template/error34.C: Likewise.
> 	* g++.old-deja/g++.other/inline15.C: Only run for C++17 and lesses.
> 	* g++.old-deja/g++.pt/ctor2.C: Add dg-error for C++20.
> ---
>  gcc/cp/parser.c                                | 13 ++++++++++++-
>  gcc/testsuite/g++.dg/DRs/dr2237.C              | 18 ++++++++++++++++++
>  gcc/testsuite/g++.dg/parse/constructor2.C      |  4 ++--
>  gcc/testsuite/g++.dg/parse/dtor12.C            |  2 +-
>  gcc/testsuite/g++.dg/parse/dtor4.C             |  2 +-
>  gcc/testsuite/g++.dg/template/dtor4.C          |  2 +-
>  gcc/testsuite/g++.dg/template/error34.C        | 10 +++++-----
>  .../g++.old-deja/g++.other/inline15.C          |  2 +-
>  gcc/testsuite/g++.old-deja/g++.pt/ctor2.C      |  2 +-
>  9 files changed, 42 insertions(+), 13 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/DRs/dr2237.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 591f44f4934..593c45b5888 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -6114,6 +6114,15 @@ cp_parser_unqualified_id (cp_parser* parser,
>  	    return build_min_nt_loc (loc, BIT_NOT_EXPR, make_auto ());
>  	  }
>  
> +	/* 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 && cxx_dialect >= cxx2a)
> +	  {
> +	    if (!cp_parser_simulate_error (parser))
> +	      error_at (tilde_loc, "template-id not allowed for destructor");
> +	    return error_mark_node;
> +	  }
> +
>  	/* If there was an explicit qualification (S::~T), first look
>  	   in the scope given by the qualification (i.e., S).
>  
> @@ -28636,7 +28645,9 @@ 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
> -      && next_token->type != CPP_TEMPLATE_ID)
> +      /* 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 >= cxx2a))
>      return false;
>  
>    /* Parse tentatively; we are going to roll back all of the tokens
> diff --git a/gcc/testsuite/g++.dg/DRs/dr2237.C b/gcc/testsuite/g++.dg/DRs/dr2237.C
> new file mode 100644
> index 00000000000..17abdf179c1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/DRs/dr2237.C
> @@ -0,0 +1,18 @@
> +// DR 2237 - Can a template-id name a constructor?
> +
> +template<class T>
> +struct X {
> +  X<T>(); // { dg-error "expected" "" { target c++2a } }
> +  X(int); // OK, injected-class-name used
> +  ~X<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++2a } }
> +};
> +
> +// ill-formed since DR1435
> +template<typename T> X<T>::X<T>() {} // { dg-error "names the constructor|as no template constructors" }
> +template<typename T> X<T>::~X<T>() {} // { dg-error "template-id not allowed for destructor" "" { target c++2a } }
> +
> +struct Q {
> +  // ill-formed since DR1435
> +  template<typename T> friend X<T>::X<T>(); // { dg-error "names the constructor|as no template constructors" }
> +  template<typename T> friend X<T>::~X<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++2a } }
> +};
> diff --git a/gcc/testsuite/g++.dg/parse/constructor2.C b/gcc/testsuite/g++.dg/parse/constructor2.C
> index e514e9397e9..87007e9c715 100644
> --- a/gcc/testsuite/g++.dg/parse/constructor2.C
> +++ b/gcc/testsuite/g++.dg/parse/constructor2.C
> @@ -5,7 +5,7 @@ class T
>  { 
>  public: 
>    T(short,short f=0) {} 
> -  T<TClass>(int f) {} 
> -  T<TClass>(int f=0,const char* b=0) {} 
> +  T<TClass>(int f) {} // { dg-error "expected" "" { target c++2a } }
> +  T<TClass>(int f=0,const char* b=0) {} // { dg-error "expected" "" { target c++2a } }
>  }; 
>  
> diff --git a/gcc/testsuite/g++.dg/parse/dtor12.C b/gcc/testsuite/g++.dg/parse/dtor12.C
> index 1acdfa36b69..d758d3e861d 100644
> --- a/gcc/testsuite/g++.dg/parse/dtor12.C
> +++ b/gcc/testsuite/g++.dg/parse/dtor12.C
> @@ -2,5 +2,5 @@
>  
>  template <class T> class a
>  {
> -  ~a<T>();
> +  ~a<T>(); // { dg-error "template-id not allowed for destructor" "" { target c++2a } }
>  };
> diff --git a/gcc/testsuite/g++.dg/parse/dtor4.C b/gcc/testsuite/g++.dg/parse/dtor4.C
> index 729ee2fa130..1059c189eff 100644
> --- a/gcc/testsuite/g++.dg/parse/dtor4.C
> +++ b/gcc/testsuite/g++.dg/parse/dtor4.C
> @@ -7,4 +7,4 @@ template <int N> struct X {
>  };
>  
>  template <int N>
> -X<N>::~X<N>(){}
> +X<N>::~X<N>(){} // { dg-error "template-id not allowed for destructor" "" { target c++2a } }
> diff --git a/gcc/testsuite/g++.dg/template/dtor4.C b/gcc/testsuite/g++.dg/template/dtor4.C
> index 4f277b248bd..917681fc13e 100644
> --- a/gcc/testsuite/g++.dg/template/dtor4.C
> +++ b/gcc/testsuite/g++.dg/template/dtor4.C
> @@ -5,5 +5,5 @@
>  
>  template<int> struct A
>  {
> -  ~A<0>(); // { dg-error "parse error|declaration" }
> +  ~A<0>(); // { dg-error "parse error|declaration|template-id" }
>  };
> diff --git a/gcc/testsuite/g++.dg/template/error34.C b/gcc/testsuite/g++.dg/template/error34.C
> index d0cbcaef28e..ab688d9ba8c 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\\)" }
> +  A<__builtin_offsetof(T, x)>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\)|expected" }
>  };
>  
>  template<typename T> struct B
>  {
> -  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)" }
> +  B<__builtin_offsetof(T, x.y)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\)|expected" }
>  };
>  
>  template<typename T> struct C
>  {
> -  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)" }
> +  C<__builtin_offsetof(T, x[6])>();	// { dg-error "type/value mismatch|offsetof\\(T, x\\\[6\\\]\\)|expected" }
>  };
>  
>  template<typename T> struct D
>  {
> -  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)" }
> +  D<__builtin_offsetof(T, x.y[6].z)>();	// { dg-error "type/value mismatch|offsetof\\(T, x.y\\\[6\\\].z\\)|expected" }
>  };
>  
>  struct E { int x; };
>  
>  template<typename T> struct F
>  {
> -  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)" }
> +  F<__builtin_offsetof(E, x)>();	// { dg-error "type/value mismatch|offsetof\\(E, x\\)|expected" }
>  };
> diff --git a/gcc/testsuite/g++.old-deja/g++.other/inline15.C b/gcc/testsuite/g++.old-deja/g++.other/inline15.C
> index 43f7f5e3025..a31c60960a5 100644
> --- a/gcc/testsuite/g++.old-deja/g++.other/inline15.C
> +++ b/gcc/testsuite/g++.old-deja/g++.other/inline15.C
> @@ -1,4 +1,4 @@
> -// { dg-do assemble  }
> +// { dg-do assemble { target c++17_down } }
>  // { dg-options "-O1" }
>  // Origin: Jakub Jelinek <jakub@redhat.com>
>  
> diff --git a/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C b/gcc/testsuite/g++.old-deja/g++.pt/ctor2.C
> index a8be91ddbb9..4159c84ff56 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>();
> +  A<T>(); // { dg-error "expected" "" { target c++2a } }
>  };
>  
>  template <class T>
> 
> base-commit: 2b2d298ff845ab7a07ffbd51da79473736da3324
> -- 
> Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
> 

Marek