[PATCH] c++: Suppress this injection for static member functions [PR97399]

[PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
the expression tmp::integral<T> (because it's type-dependent, and also
current_class_ptr is set) within the trailing return type, and later
during substitution we can't resolve the 'this' since
tsubst_function_type does inject_this_parm only for non-static member
functions which tmp::func is not.

It seems the root of the problem is that we set current_class_ptr when
parsing the signature of a static member function.  Since explicit uses
of 'this' are already not allowed in this context, we probably shouldn't
be doing inject_this_parm either.

Bootstrapped and regtested on x64_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

	PR c++/97399
	* parser.c (cp_parser_init_declarator): If the storage class
	specifier is sc_static, pass true for static_p to
	cp_parser_declarator.
	(cp_parser_direct_declarator): Don't do inject_this_parm when
	the member function is static.

gcc/testsuite/ChangeLog:

	PR c++/88548
	PR c++/97399
	* g++.dg/cpp0x/this2.C: New test.
	* g++.dg/template/pr97399a.C: New test.
	* g++.dg/template/pr97399b.C: New test.
---
 gcc/cp/parser.c                          |  5 +++--
 gcc/testsuite/g++.dg/cpp0x/this2.C       |  8 ++++++++
 gcc/testsuite/g++.dg/template/pr97399a.C | 11 +++++++++++
 gcc/testsuite/g++.dg/template/pr97399b.C | 11 +++++++++++
 4 files changed, 33 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/this2.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 48437f23175..18cf9888632 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -21413,6 +21413,7 @@ cp_parser_init_declarator (cp_parser* parser,
   bool is_non_constant_init;
   int ctor_dtor_or_conv_p;
   bool friend_p = cp_parser_friend_p (decl_specifiers);
+  bool static_p = decl_specifiers->storage_class == sc_static;
   tree pushed_scope = NULL_TREE;
   bool range_for_decl_p = false;
   bool saved_default_arg_ok_p = parser->default_arg_ok_p;
@@ -21446,7 +21447,7 @@ cp_parser_init_declarator (cp_parser* parser,
     = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
 			    flags, &ctor_dtor_or_conv_p,
 			    /*parenthesized_p=*/NULL,
-			    member_p, friend_p, /*static_p=*/false);
+			    member_p, friend_p, static_p);
   /* Gather up the deferred checks.  */
   stop_deferring_access_checks ();
 
@@ -22122,7 +22123,7 @@ cp_parser_direct_declarator (cp_parser* parser,
 
 		  tree save_ccp = current_class_ptr;
 		  tree save_ccr = current_class_ref;
-		  if (memfn)
+		  if (memfn && !static_p)
 		    /* DR 1207: 'this' is in scope after the cv-quals.  */
 		    inject_this_parameter (current_class_type, cv_quals);
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/this2.C b/gcc/testsuite/g++.dg/cpp0x/this2.C
new file mode 100644
index 00000000000..3781bc5efec
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/this2.C
@@ -0,0 +1,8 @@
+// PR c++/88548
+// { dg-do compile { target c++11 } }
+
+struct S {
+  int a;
+  template <class> static auto m1 ()
+    -> decltype(this->a) { return 0; }; // { dg-error "'this'" }
+};
diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
new file mode 100644
index 00000000000..3713dbde6e0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr97399a.C
@@ -0,0 +1,11 @@
+// PR c++/97399
+// { dg-do compile { target c++11 } }
+
+template <bool> struct enable_if_t {};
+struct tmp {
+  template <class T> static constexpr bool is_integral();
+  template <class T> static auto func()
+    -> enable_if_t<tmp::is_integral<T>()>;
+};
+template <class> constexpr bool tmp::is_integral() { return true; }
+int main() { tmp::func<int>(); }
diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
new file mode 100644
index 00000000000..9196c985834
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr97399b.C
@@ -0,0 +1,11 @@
+// PR c++/97399
+// { dg-do compile { target c++11 } }
+
+template <bool> struct enable_if_t {};
+struct tmp {
+  template <class T> constexpr bool is_integral(); // non-static
+  template <class T> static auto func()
+    -> enable_if_t<tmp::is_integral<T>()>; // { dg-error "without object" }
+};
+template <class> constexpr bool tmp::is_integral() { return true; }
+int main() { tmp::func<int>(); } // { dg-error "no match" }
-- 
2.30.0.155.g66e871b664

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

On Thu, 21 Jan 2021, Patrick Palka wrote:

> Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
> the expression tmp::integral<T> (because it's type-dependent, and also
> current_class_ptr is set) within the trailing return type, and later
> during substitution we can't resolve the 'this' since
> tsubst_function_type does inject_this_parm only for non-static member
> functions which tmp::func is not.
> 
> It seems the root of the problem is that we set current_class_ptr when
> parsing the signature of a static member function.  Since explicit uses
> of 'this' are already not allowed in this context, we probably shouldn't
> be doing inject_this_parm either.
> 
> Bootstrapped and regtested on x64_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/97399
> 	* parser.c (cp_parser_init_declarator): If the storage class
> 	specifier is sc_static, pass true for static_p to
> 	cp_parser_declarator.
> 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> 	the member function is static.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/88548
> 	PR c++/97399
> 	* g++.dg/cpp0x/this2.C: New test.
> 	* g++.dg/template/pr97399a.C: New test.
> 	* g++.dg/template/pr97399b.C: New test.
> ---
>  gcc/cp/parser.c                          |  5 +++--
>  gcc/testsuite/g++.dg/cpp0x/this2.C       |  8 ++++++++
>  gcc/testsuite/g++.dg/template/pr97399a.C | 11 +++++++++++
>  gcc/testsuite/g++.dg/template/pr97399b.C | 11 +++++++++++
>  4 files changed, 33 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/this2.C
>  create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
>  create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 48437f23175..18cf9888632 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -21413,6 +21413,7 @@ cp_parser_init_declarator (cp_parser* parser,
>    bool is_non_constant_init;
>    int ctor_dtor_or_conv_p;
>    bool friend_p = cp_parser_friend_p (decl_specifiers);
> +  bool static_p = decl_specifiers->storage_class == sc_static;
>    tree pushed_scope = NULL_TREE;
>    bool range_for_decl_p = false;
>    bool saved_default_arg_ok_p = parser->default_arg_ok_p;
> @@ -21446,7 +21447,7 @@ cp_parser_init_declarator (cp_parser* parser,
>      = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
>  			    flags, &ctor_dtor_or_conv_p,
>  			    /*parenthesized_p=*/NULL,
> -			    member_p, friend_p, /*static_p=*/false);
> +			    member_p, friend_p, static_p);
>    /* Gather up the deferred checks.  */
>    stop_deferring_access_checks ();

I should note that the above parser change is needed so that we properly
communicate static-ness to cp_parser_direct_declarator when parsing a
member function template.

>  
> @@ -22122,7 +22123,7 @@ cp_parser_direct_declarator (cp_parser* parser,
>  
>  		  tree save_ccp = current_class_ptr;
>  		  tree save_ccr = current_class_ref;
> -		  if (memfn)
> +		  if (memfn && !static_p)
>  		    /* DR 1207: 'this' is in scope after the cv-quals.  */
>  		    inject_this_parameter (current_class_type, cv_quals);
>  
> diff --git a/gcc/testsuite/g++.dg/cpp0x/this2.C b/gcc/testsuite/g++.dg/cpp0x/this2.C
> new file mode 100644
> index 00000000000..3781bc5efec
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/this2.C
> @@ -0,0 +1,8 @@
> +// PR c++/88548
> +// { dg-do compile { target c++11 } }
> +
> +struct S {
> +  int a;
> +  template <class> static auto m1 ()
> +    -> decltype(this->a) { return 0; }; // { dg-error "'this'" }
> +};
> diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
> new file mode 100644
> index 00000000000..3713dbde6e0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
> @@ -0,0 +1,11 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +struct tmp {
> +  template <class T> static constexpr bool is_integral();
> +  template <class T> static auto func()
> +    -> enable_if_t<tmp::is_integral<T>()>;
> +};
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +int main() { tmp::func<int>(); }
> diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
> new file mode 100644
> index 00000000000..9196c985834
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
> @@ -0,0 +1,11 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +struct tmp {
> +  template <class T> constexpr bool is_integral(); // non-static
> +  template <class T> static auto func()
> +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-error "without object" }
> +};
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +int main() { tmp::func<int>(); } // { dg-error "no match" }
> -- 
> 2.30.0.155.g66e871b664
> 
> 

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Marek Polacek]

On Thu, Jan 21, 2021 at 11:22:24AM -0500, Patrick Palka via Gcc-patches wrote:
> Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
> the expression tmp::integral<T> (because it's type-dependent, and also
> current_class_ptr is set) within the trailing return type, and later
> during substitution we can't resolve the 'this' since
> tsubst_function_type does inject_this_parm only for non-static member
> functions which tmp::func is not.
> 
> It seems the root of the problem is that we set current_class_ptr when
> parsing the signature of a static member function.  Since explicit uses
> of 'this' are already not allowed in this context, we probably shouldn't
> be doing inject_this_parm either.
> 
> Bootstrapped and regtested on x64_64-pc-linux-gnu, does this look OK for
> trunk?

This looks fine to me.

> gcc/cp/ChangeLog:
> 
> 	PR c++/97399
> 	* parser.c (cp_parser_init_declarator): If the storage class
> 	specifier is sc_static, pass true for static_p to
> 	cp_parser_declarator.
> 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> 	the member function is static.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/88548
> 	PR c++/97399
> 	* g++.dg/cpp0x/this2.C: New test.
> 	* g++.dg/template/pr97399a.C: New test.
> 	* g++.dg/template/pr97399b.C: New test.
> ---
>  gcc/cp/parser.c                          |  5 +++--
>  gcc/testsuite/g++.dg/cpp0x/this2.C       |  8 ++++++++
>  gcc/testsuite/g++.dg/template/pr97399a.C | 11 +++++++++++
>  gcc/testsuite/g++.dg/template/pr97399b.C | 11 +++++++++++
>  4 files changed, 33 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/this2.C
>  create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
>  create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 48437f23175..18cf9888632 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -21413,6 +21413,7 @@ cp_parser_init_declarator (cp_parser* parser,
>    bool is_non_constant_init;
>    int ctor_dtor_or_conv_p;
>    bool friend_p = cp_parser_friend_p (decl_specifiers);
> +  bool static_p = decl_specifiers->storage_class == sc_static;
>    tree pushed_scope = NULL_TREE;
>    bool range_for_decl_p = false;
>    bool saved_default_arg_ok_p = parser->default_arg_ok_p;
> @@ -21446,7 +21447,7 @@ cp_parser_init_declarator (cp_parser* parser,
>      = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
>  			    flags, &ctor_dtor_or_conv_p,
>  			    /*parenthesized_p=*/NULL,
> -			    member_p, friend_p, /*static_p=*/false);
> +			    member_p, friend_p, static_p);
>    /* Gather up the deferred checks.  */
>    stop_deferring_access_checks ();
>  
> @@ -22122,7 +22123,7 @@ cp_parser_direct_declarator (cp_parser* parser,
>  
>  		  tree save_ccp = current_class_ptr;
>  		  tree save_ccr = current_class_ref;
> -		  if (memfn)
> +		  if (memfn && !static_p)
>  		    /* DR 1207: 'this' is in scope after the cv-quals.  */
>  		    inject_this_parameter (current_class_type, cv_quals);
>  
> diff --git a/gcc/testsuite/g++.dg/cpp0x/this2.C b/gcc/testsuite/g++.dg/cpp0x/this2.C
> new file mode 100644
> index 00000000000..3781bc5efec
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/this2.C
> @@ -0,0 +1,8 @@
> +// PR c++/88548
> +// { dg-do compile { target c++11 } }
> +
> +struct S {
> +  int a;
> +  template <class> static auto m1 ()
> +    -> decltype(this->a) { return 0; }; // { dg-error "'this'" }
> +};
> diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
> new file mode 100644
> index 00000000000..3713dbde6e0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
> @@ -0,0 +1,11 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +struct tmp {
> +  template <class T> static constexpr bool is_integral();
> +  template <class T> static auto func()
> +    -> enable_if_t<tmp::is_integral<T>()>;
> +};
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +int main() { tmp::func<int>(); }
> diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
> new file mode 100644
> index 00000000000..9196c985834
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
> @@ -0,0 +1,11 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +struct tmp {
> +  template <class T> constexpr bool is_integral(); // non-static
> +  template <class T> static auto func()
> +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-error "without object" }
> +};
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +int main() { tmp::func<int>(); } // { dg-error "no match" }
> -- 
> 2.30.0.155.g66e871b664
> 

Marek

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

On Thu, 21 Jan 2021, Marek Polacek wrote:

> On Thu, Jan 21, 2021 at 11:22:24AM -0500, Patrick Palka via Gcc-patches wrote:
> > Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
> > the expression tmp::integral<T> (because it's type-dependent, and also
> > current_class_ptr is set) within the trailing return type, and later
> > during substitution we can't resolve the 'this' since
> > tsubst_function_type does inject_this_parm only for non-static member
> > functions which tmp::func is not.
> > 
> > It seems the root of the problem is that we set current_class_ptr when
> > parsing the signature of a static member function.  Since explicit uses
> > of 'this' are already not allowed in this context, we probably shouldn't
> > be doing inject_this_parm either.
> > 
> > Bootstrapped and regtested on x64_64-pc-linux-gnu, does this look OK for
> > trunk?
> 
> This looks fine to me.

Thanks.  It occurred to me that we should similarly avoid doing
inject_this_parm for friend functions, too.  Here's a patch to that end,
and which also addresses a minor diagnostic regression in a gomp
testcase that I didn't notice earlier:

-- >8 --

Subject: [PATCH] c++: Suppress 'this' injection for static member functions
 [PR97399]

In the testcase pr97399a.C below, finish_qualified_id_expr at parse time
adds an implicit 'this->' to the expression tmp::integral<T> (because
it's type-dependent, and also current_class_ptr is set at this point)
within the trailing return type.  Later when substituting into the
trailing return type we ICE due to the unresolved 'this', since
tsubst_function_type does inject_this_parm only for non-static member
functions, which tmp::func is not.

It seems the root of the problem is that we set current_class_ptr when
parsing the signature of a static member function.  Since explicit uses
of 'this' are already forbidden in this context, we probably shouldn't
be doing inject_this_parm either.  Likewise for friend functions, with
which we can exhibit the same ICE.

Testing revealed a diagnostic regression in the gomp testcase this-1.C
due to current_class_ptr now being unset when parsing a use of 'this'
within the pragma for a static member function.  The below changes to
cp_parser_omp_var_list_no_open and finish_this_expr try to salvage the
quality of the affected error message (or else the error messages in
question would degenerate to "expected qualified-id before 'this'").

The change to cp_parser_init_declarator below is needed so that we
properly communicate static-ness to cp_parser_direct_declarator when
parsing a member function template.

Bootstrap and regtest re-running on x86_64-pc-linux-gnu, does this look
OK for trunk if successful?

gcc/cp/ChangeLog:

	PR c++/97399
	* parser.c (cp_parser_init_declarator): If the storage class
	specifier is sc_static, pass true for static_p to
	cp_parser_declarator.
	(cp_parser_direct_declarator): Don't do inject_this_parm when
	the function is specified as static or friend.
	(cp_parser_omp_var_list_no_open): Call finish_this_expr even
	when current_class_ptr is not set for better diagnostics.
	* semantics.c (finish_this_expr): Emit a more specific diagnostic
	when at class scope.

gcc/testsuite/ChangeLog:

	PR c++/88548
	PR c++/97399
	* g++.dg/cpp0x/this2.C: New test.
	* g++.dg/gomp/this-1.C: Adjust expected error for use of 'this'
	in signature of static member function.
	* g++.dg/template/pr97399a.C: New test.
	* g++.dg/template/pr97399b.C: New test.
---
 gcc/cp/parser.c                          |  6 +++---
 gcc/cp/semantics.c                       |  2 ++
 gcc/testsuite/g++.dg/cpp0x/this2.C       |  9 +++++++++
 gcc/testsuite/g++.dg/gomp/this-1.C       |  4 ++--
 gcc/testsuite/g++.dg/template/pr97399a.C | 20 ++++++++++++++++++++
 gcc/testsuite/g++.dg/template/pr97399b.C | 20 ++++++++++++++++++++
 6 files changed, 56 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/this2.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 48437f23175..6d6bd1e60fd 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -21413,6 +21413,7 @@ cp_parser_init_declarator (cp_parser* parser,
   bool is_non_constant_init;
   int ctor_dtor_or_conv_p;
   bool friend_p = cp_parser_friend_p (decl_specifiers);
+  bool static_p = decl_specifiers->storage_class == sc_static;
   tree pushed_scope = NULL_TREE;
   bool range_for_decl_p = false;
   bool saved_default_arg_ok_p = parser->default_arg_ok_p;
@@ -21446,7 +21447,7 @@ cp_parser_init_declarator (cp_parser* parser,
     = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
 			    flags, &ctor_dtor_or_conv_p,
 			    /*parenthesized_p=*/NULL,
-			    member_p, friend_p, /*static_p=*/false);
+			    member_p, friend_p, static_p);
   /* Gather up the deferred checks.  */
   stop_deferring_access_checks ();
 
@@ -22122,7 +22123,7 @@ cp_parser_direct_declarator (cp_parser* parser,
 
 		  tree save_ccp = current_class_ptr;
 		  tree save_ccr = current_class_ref;
-		  if (memfn)
+		  if (memfn && !static_p && !friend_p)
 		    /* DR 1207: 'this' is in scope after the cv-quals.  */
 		    inject_this_parameter (current_class_type, cv_quals);
 
@@ -35184,7 +35185,6 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
 	cp_parser_parse_tentatively (parser);
       token = cp_lexer_peek_token (parser->lexer);
       if (kind != 0
-	  && current_class_ptr
 	  && cp_parser_is_keyword (token, RID_THIS))
 	{
 	  decl = finish_this_expr ();
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 067095276af..359a6df1d32 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2801,6 +2801,8 @@ finish_this_expr (void)
     error ("%<this%> is unavailable for static member functions");
   else if (fn)
     error ("invalid use of %<this%> in non-member function");
+  else if (CLASS_TYPE_P (current_nonlambda_scope ()))
+    error ("invalid use of %<this%> at class scope");
   else
     error ("invalid use of %<this%> at top level");
   return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/cpp0x/this2.C b/gcc/testsuite/g++.dg/cpp0x/this2.C
new file mode 100644
index 00000000000..5e19161a70f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/this2.C
@@ -0,0 +1,9 @@
+// PR c++/88548
+// { dg-do compile { target c++11 } }
+
+struct S {
+  int a;
+  template <class> static auto m1() -> decltype(this->a); // { dg-error "'this'" }
+  friend auto m2() -> decltype(this->a); // { dg-error "'this'" }
+  template <class> friend auto m3() -> decltype(this->a); // { dg-error "'this'" }
+};
diff --git a/gcc/testsuite/g++.dg/gomp/this-1.C b/gcc/testsuite/g++.dg/gomp/this-1.C
index 30ab8b3903c..0a83fee9216 100644
--- a/gcc/testsuite/g++.dg/gomp/this-1.C
+++ b/gcc/testsuite/g++.dg/gomp/this-1.C
@@ -3,7 +3,7 @@
 
 struct S
 {
-  #pragma omp declare simd linear(this)		// { dg-error "is not a function argument" }
+  #pragma omp declare simd linear(this)		// { dg-error "invalid use of .this." }
   static void foo ();
   void bar ();
 };
@@ -35,7 +35,7 @@ S::bar ()
 template <int N>
 struct T
 {
-  #pragma omp declare simd linear(this)		// { dg-error "is not a function argument" }
+  #pragma omp declare simd linear(this)		// { dg-error "invalid use of .this." }
   static void foo ();
   void bar ();
 };
diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
new file mode 100644
index 00000000000..fc2fbd79294
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr97399a.C
@@ -0,0 +1,20 @@
+// PR c++/97399
+// { dg-do compile { target c++11 } }
+
+template <bool> struct enable_if_t {};
+
+struct tmp {
+  template <class> static constexpr bool is_integral();
+  template <class T> static auto f()
+    -> enable_if_t<tmp::is_integral<T>()>;
+  template <class T> friend auto g(tmp, T)
+    -> enable_if_t<!tmp::is_integral<T>()>;
+};
+
+template <class> constexpr bool tmp::is_integral() { return true; }
+
+int main()
+{
+  tmp::f<int>();
+  g(tmp{}, 0);
+}
diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
new file mode 100644
index 00000000000..3375be8cfbc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr97399b.C
@@ -0,0 +1,20 @@
+// PR c++/97399
+// { dg-do compile { target c++11 } }
+
+template <bool> struct enable_if_t {};
+
+struct tmp {
+  template <class> constexpr bool is_integral(); // non-static
+  template <class T> static auto f()
+    -> enable_if_t<tmp::is_integral<T>()>; // { dg-error "without object" }
+  template <class T> friend auto g(tmp, T)
+    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without object" }
+};
+
+template <class> constexpr bool tmp::is_integral() { return true; }
+
+int main()
+{
+  tmp::f<int>(); // { dg-error "no match" }
+  g(tmp{}, 0); // { dg-error "no match" }
+}
-- 
2.30.0.155.g66e871b664

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Jason Merrill]

On 1/21/21 11:22 AM, Patrick Palka wrote:
> Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
> the expression tmp::integral<T> (because it's type-dependent, and also
> current_class_ptr is set) within the trailing return type, and later
> during substitution we can't resolve the 'this' since
> tsubst_function_type does inject_this_parm only for non-static member
> functions which tmp::func is not.
> 
> It seems the root of the problem is that we set current_class_ptr when
> parsing the signature of a static member function.  Since explicit uses
> of 'this' are already not allowed in this context, we probably shouldn't
> be doing inject_this_parm either.

Hmm, 'this' is defined in a static member function, it's just ill-formed 
to use it:

7.5.2/2: "... [this] shall not appear within the declaration of a static 
member function (although its type and value category are defined within 
a static member function as they are within a non-static member 
function). [Note: This is because declaration matching does not occur 
until the complete declarator is known. — end note]"

Perhaps maybe_dummy_object needs to be smarter about recognizing static 
context.  Or perhaps there's no way to tell the difference between the 
specified behavior above and the behavior with your patch, but:

The note suggests that we need to test the case of an out-of-class 
definition of a static member function (template); we must 
inject_this_parm when parsing such a declaration, since we don't know 
it's static until we match it to the declaration in the class.  I'm 
guessing that this would lead to the same problem.

> Bootstrapped and regtested on x64_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/97399
> 	* parser.c (cp_parser_init_declarator): If the storage class
> 	specifier is sc_static, pass true for static_p to
> 	cp_parser_declarator.
> 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> 	the member function is static.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/88548
> 	PR c++/97399
> 	* g++.dg/cpp0x/this2.C: New test.
> 	* g++.dg/template/pr97399a.C: New test.
> 	* g++.dg/template/pr97399b.C: New test.
> ---
>   gcc/cp/parser.c                          |  5 +++--
>   gcc/testsuite/g++.dg/cpp0x/this2.C       |  8 ++++++++
>   gcc/testsuite/g++.dg/template/pr97399a.C | 11 +++++++++++
>   gcc/testsuite/g++.dg/template/pr97399b.C | 11 +++++++++++
>   4 files changed, 33 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/this2.C
>   create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
>   create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 48437f23175..18cf9888632 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -21413,6 +21413,7 @@ cp_parser_init_declarator (cp_parser* parser,
>     bool is_non_constant_init;
>     int ctor_dtor_or_conv_p;
>     bool friend_p = cp_parser_friend_p (decl_specifiers);
> +  bool static_p = decl_specifiers->storage_class == sc_static;
>     tree pushed_scope = NULL_TREE;
>     bool range_for_decl_p = false;
>     bool saved_default_arg_ok_p = parser->default_arg_ok_p;
> @@ -21446,7 +21447,7 @@ cp_parser_init_declarator (cp_parser* parser,
>       = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
>   			    flags, &ctor_dtor_or_conv_p,
>   			    /*parenthesized_p=*/NULL,
> -			    member_p, friend_p, /*static_p=*/false);
> +			    member_p, friend_p, static_p);
>     /* Gather up the deferred checks.  */
>     stop_deferring_access_checks ();
>   
> @@ -22122,7 +22123,7 @@ cp_parser_direct_declarator (cp_parser* parser,
>   
>   		  tree save_ccp = current_class_ptr;
>   		  tree save_ccr = current_class_ref;
> -		  if (memfn)
> +		  if (memfn && !static_p)
>   		    /* DR 1207: 'this' is in scope after the cv-quals.  */
>   		    inject_this_parameter (current_class_type, cv_quals);
>   
> diff --git a/gcc/testsuite/g++.dg/cpp0x/this2.C b/gcc/testsuite/g++.dg/cpp0x/this2.C
> new file mode 100644
> index 00000000000..3781bc5efec
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/this2.C
> @@ -0,0 +1,8 @@
> +// PR c++/88548
> +// { dg-do compile { target c++11 } }
> +
> +struct S {
> +  int a;
> +  template <class> static auto m1 ()
> +    -> decltype(this->a) { return 0; }; // { dg-error "'this'" }
> +};
> diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
> new file mode 100644
> index 00000000000..3713dbde6e0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
> @@ -0,0 +1,11 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +struct tmp {
> +  template <class T> static constexpr bool is_integral();
> +  template <class T> static auto func()
> +    -> enable_if_t<tmp::is_integral<T>()>;
> +};
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +int main() { tmp::func<int>(); }
> diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
> new file mode 100644
> index 00000000000..9196c985834
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
> @@ -0,0 +1,11 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +struct tmp {
> +  template <class T> constexpr bool is_integral(); // non-static
> +  template <class T> static auto func()
> +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-error "without object" }
> +};
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +int main() { tmp::func<int>(); } // { dg-error "no match" }
> 

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

On Thu, 21 Jan 2021, Jason Merrill wrote:

> On 1/21/21 11:22 AM, Patrick Palka wrote:
> > Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
> > the expression tmp::integral<T> (because it's type-dependent, and also
> > current_class_ptr is set) within the trailing return type, and later
> > during substitution we can't resolve the 'this' since
> > tsubst_function_type does inject_this_parm only for non-static member
> > functions which tmp::func is not.
> > 
> > It seems the root of the problem is that we set current_class_ptr when
> > parsing the signature of a static member function.  Since explicit uses
> > of 'this' are already not allowed in this context, we probably shouldn't
> > be doing inject_this_parm either.
> 
> Hmm, 'this' is defined in a static member function, it's just ill-formed to
> use it:
> 
> 7.5.2/2: "... [this] shall not appear within the declaration of a static
> member function (although its type and value category are defined within a
> static member function as they are within a non-static member function).
> [Note: This is because declaration matching does not occur until the complete
> declarator is known. — end note]"

Ah, I see.

> 
> Perhaps maybe_dummy_object needs to be smarter about recognizing static
> context.  Or perhaps there's no way to tell the difference between the
> specified behavior above and the behavior with your patch, but:
> 
> The note suggests that we need to test the case of an out-of-class definition
> of a static member function (template); we must inject_this_parm when parsing
> such a declaration, since we don't know it's static until we match it to the
> declaration in the class.  I'm guessing that this would lead to the same
> problem.

Good point, we do get a signature mismatch error in this case, due to
'this->' appearing in the trailing return type out-of-class declaration
but not in that of the in-class declaration, so the trailing return
types don't match up.  In light of this case, it doesn't seem possible
for maybe_dummy_object to distinguish between a static and non-static
context when parsing the trailing return type of the declaration.

So perhaps we should mirror at substitution time what we do at parse
time, and inject 'this' also when substituting into the function type of
a static member function?  That way, we'll resolve the use of 'this->'
and then discard it the RHS of -> is a static member function, or
complain that 'this' is not a constant expression if the RHS is a
non-static member function.

The following patch performs that, and seems to pass light testing.
Full testing in progress.  Revised commit message is still a WIP.
How does this approach look?

-- >8 --

Subject: [PATCH] c++: 'this' injection and static member functions [PR97399]

gcc/cp/ChangeLog:

	PR c++/97399
	* parser.c (cp_parser_init_declarator): If the storage class
	specifier is sc_static, pass true for static_p to
	cp_parser_declarator.
	(cp_parser_direct_declarator): Don't do inject_this_parm when
	the function is specified as a friend.
	* pt.c (tsubst_function_type): Also inject 'this' when
	substituting into the function type of a static member
	function.

gcc/testsuite/ChangeLog:

	PR c++/88548
	PR c++/97399
	* g++.dg/cpp0x/this2.C: New test.
	* g++.dg/gomp/this-1.C: Adjust expected error for use of 'this'
	in signature of static member function.
	* g++.dg/template/pr97399a.C: New test.
	* g++.dg/template/pr97399b.C: New test.
---
 gcc/cp/parser.c                          |  2 +-
 gcc/cp/pt.c                              | 13 +++++++++++--
 gcc/testsuite/g++.dg/template/pr97399a.C | 23 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/template/pr97399b.C | 23 +++++++++++++++++++++++
 4 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 48437f23175..a0efa55d21e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -22122,7 +22122,7 @@ cp_parser_direct_declarator (cp_parser* parser,
 
 		  tree save_ccp = current_class_ptr;
 		  tree save_ccr = current_class_ref;
-		  if (memfn)
+		  if (memfn && !friend_p)
 		    /* DR 1207: 'this' is in scope after the cv-quals.  */
 		    inject_this_parameter (current_class_type, cv_quals);
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ad855dbde72..d4763325e25 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -15072,8 +15072,17 @@ tsubst_function_type (tree t,
 
       tree save_ccp = current_class_ptr;
       tree save_ccr = current_class_ref;
-      tree this_type = (TREE_CODE (t) == METHOD_TYPE
-			? TREE_TYPE (TREE_VALUE (arg_types)) : NULL_TREE);
+      tree this_type = NULL_TREE;
+      if (TREE_CODE (t) == METHOD_TYPE)
+	this_type = TREE_TYPE (TREE_VALUE (arg_types));
+      else if (in_decl
+	       && DECL_FUNCTION_MEMBER_P (in_decl)
+	       && t == TREE_TYPE (in_decl))
+	/* Also inject 'this' when substituting into the function type
+	   of a static member function, as we may have conservatively
+	   added 'this->' to a dependent member function call in its
+	   trailing return type which we might need to resolve.  */
+	this_type = DECL_CONTEXT (in_decl);
       bool do_inject = this_type && CLASS_TYPE_P (this_type);
       if (do_inject)
 	{
diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
new file mode 100644
index 00000000000..4bb818908fd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr97399a.C
@@ -0,0 +1,23 @@
+// PR c++/97399
+// { dg-do compile { target c++11 } }
+
+template <bool> struct enable_if_t {};
+
+struct tmp {
+  template <class> static constexpr bool is_integral();
+  template <class T> static auto f()
+    -> enable_if_t<tmp::is_integral<T>()>;
+  template <class T> friend auto g(tmp, T)
+    -> enable_if_t<!tmp::is_integral<T>()>;
+};
+
+template <class> constexpr bool tmp::is_integral() { return true; }
+
+template <class T> auto tmp::f()
+  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
+
+int main()
+{
+  tmp::f<int>();
+  g(tmp{}, 0);
+}
diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
new file mode 100644
index 00000000000..673ba6624e3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr97399b.C
@@ -0,0 +1,23 @@
+// PR c++/97399
+// { dg-do compile { target c++11 } }
+
+template <bool> struct enable_if_t {};
+
+struct tmp {
+  template <class> constexpr bool is_integral(); // non-static
+  template <class T> static auto f()
+    -> enable_if_t<tmp::is_integral<T>()>; // { dg-message "in template argument" }
+  template <class T> friend auto g(tmp, T)
+    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without object" }
+};
+
+template <class> constexpr bool tmp::is_integral() { return true; }
+
+template <class T> auto tmp::f() // { dg-error "'this' is not a constant" }
+  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
+
+int main()
+{
+  tmp::f<int>(); // { dg-error "no match" }
+  g(tmp{}, 0); // { dg-error "no match" }
+}
-- 
2.30.0.155.g66e871b664

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

On Fri, 22 Jan 2021, Patrick Palka wrote:

> On Thu, 21 Jan 2021, Jason Merrill wrote:
> 
> > On 1/21/21 11:22 AM, Patrick Palka wrote:
> > > Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
> > > the expression tmp::integral<T> (because it's type-dependent, and also
> > > current_class_ptr is set) within the trailing return type, and later
> > > during substitution we can't resolve the 'this' since
> > > tsubst_function_type does inject_this_parm only for non-static member
> > > functions which tmp::func is not.
> > > 
> > > It seems the root of the problem is that we set current_class_ptr when
> > > parsing the signature of a static member function.  Since explicit uses
> > > of 'this' are already not allowed in this context, we probably shouldn't
> > > be doing inject_this_parm either.
> > 
> > Hmm, 'this' is defined in a static member function, it's just ill-formed to
> > use it:
> > 
> > 7.5.2/2: "... [this] shall not appear within the declaration of a static
> > member function (although its type and value category are defined within a
> > static member function as they are within a non-static member function).
> > [Note: This is because declaration matching does not occur until the complete
> > declarator is known. — end note]"
> 
> Ah, I see.
> 
> > 
> > Perhaps maybe_dummy_object needs to be smarter about recognizing static
> > context.  Or perhaps there's no way to tell the difference between the
> > specified behavior above and the behavior with your patch, but:
> > 
> > The note suggests that we need to test the case of an out-of-class definition
> > of a static member function (template); we must inject_this_parm when parsing
> > such a declaration, since we don't know it's static until we match it to the
> > declaration in the class.  I'm guessing that this would lead to the same
> > problem.
> 
> Good point, we do get a signature mismatch error in this case, due to
> 'this->' appearing in the trailing return type out-of-class declaration
> but not in that of the in-class declaration, so the trailing return
> types don't match up.  In light of this case, it doesn't seem possible
> for maybe_dummy_object to distinguish between a static and non-static
> context when parsing the trailing return type of the declaration.
> 
> So perhaps we should mirror at substitution time what we do at parse
> time, and inject 'this' also when substituting into the function type of
> a static member function?  That way, we'll resolve the use of 'this->'
> and then discard it the RHS of -> is a static member function, or
> complain that 'this' is not a constant expression if the RHS is a
> non-static member function.
> 
> The following patch performs that, and seems to pass light testing.
> Full testing in progress.  Revised commit message is still a WIP.
> How does this approach look?
> 
> -- >8 --
> 
> Subject: [PATCH] c++: 'this' injection and static member functions [PR97399]
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/97399
> 	* parser.c (cp_parser_init_declarator): If the storage class
> 	specifier is sc_static, pass true for static_p to
> 	cp_parser_declarator.
> 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> 	the function is specified as a friend.
> 	* pt.c (tsubst_function_type): Also inject 'this' when
> 	substituting into the function type of a static member
> 	function.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/88548
> 	PR c++/97399
> 	* g++.dg/cpp0x/this2.C: New test.
> 	* g++.dg/gomp/this-1.C: Adjust expected error for use of 'this'
> 	in signature of static member function.
> 	* g++.dg/template/pr97399a.C: New test.
> 	* g++.dg/template/pr97399b.C: New test.
> ---
>  gcc/cp/parser.c                          |  2 +-
>  gcc/cp/pt.c                              | 13 +++++++++++--
>  gcc/testsuite/g++.dg/template/pr97399a.C | 23 +++++++++++++++++++++++
>  gcc/testsuite/g++.dg/template/pr97399b.C | 23 +++++++++++++++++++++++
>  4 files changed, 58 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
>  create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 48437f23175..a0efa55d21e 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -22122,7 +22122,7 @@ cp_parser_direct_declarator (cp_parser* parser,
>  
>  		  tree save_ccp = current_class_ptr;
>  		  tree save_ccr = current_class_ref;
> -		  if (memfn)
> +		  if (memfn && !friend_p)
>  		    /* DR 1207: 'this' is in scope after the cv-quals.  */
>  		    inject_this_parameter (current_class_type, cv_quals);
>  
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index ad855dbde72..d4763325e25 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -15072,8 +15072,17 @@ tsubst_function_type (tree t,
>  
>        tree save_ccp = current_class_ptr;
>        tree save_ccr = current_class_ref;
> -      tree this_type = (TREE_CODE (t) == METHOD_TYPE
> -			? TREE_TYPE (TREE_VALUE (arg_types)) : NULL_TREE);
> +      tree this_type = NULL_TREE;
> +      if (TREE_CODE (t) == METHOD_TYPE)
> +	this_type = TREE_TYPE (TREE_VALUE (arg_types));
> +      else if (in_decl
> +	       && DECL_FUNCTION_MEMBER_P (in_decl)

Oops, this line should instead be checking DECL_STATIC_FUNCTION_P.
Consider it changed.

> +	       && t == TREE_TYPE (in_decl))
> +	/* Also inject 'this' when substituting into the function type
> +	   of a static member function, as we may have conservatively
> +	   added 'this->' to a dependent member function call in its
> +	   trailing return type which we might need to resolve.  */
> +	this_type = DECL_CONTEXT (in_decl);
>        bool do_inject = this_type && CLASS_TYPE_P (this_type);
>        if (do_inject)
>  	{
> diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
> new file mode 100644
> index 00000000000..4bb818908fd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
> @@ -0,0 +1,23 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +
> +struct tmp {
> +  template <class> static constexpr bool is_integral();
> +  template <class T> static auto f()
> +    -> enable_if_t<tmp::is_integral<T>()>;
> +  template <class T> friend auto g(tmp, T)
> +    -> enable_if_t<!tmp::is_integral<T>()>;
> +};
> +
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +
> +template <class T> auto tmp::f()
> +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> +
> +int main()
> +{
> +  tmp::f<int>();
> +  g(tmp{}, 0);
> +}
> diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
> new file mode 100644
> index 00000000000..673ba6624e3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
> @@ -0,0 +1,23 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +
> +struct tmp {
> +  template <class> constexpr bool is_integral(); // non-static
> +  template <class T> static auto f()
> +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-message "in template argument" }
> +  template <class T> friend auto g(tmp, T)
> +    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without object" }
> +};
> +
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +
> +template <class T> auto tmp::f() // { dg-error "'this' is not a constant" }
> +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> +
> +int main()
> +{
> +  tmp::f<int>(); // { dg-error "no match" }
> +  g(tmp{}, 0); // { dg-error "no match" }
> +}
> -- 
> 2.30.0.155.g66e871b664
> 

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

On Fri, 22 Jan 2021, Patrick Palka wrote:

> On Fri, 22 Jan 2021, Patrick Palka wrote:
> 
> > On Thu, 21 Jan 2021, Jason Merrill wrote:
> > 
> > > On 1/21/21 11:22 AM, Patrick Palka wrote:
> > > > Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
> > > > the expression tmp::integral<T> (because it's type-dependent, and also
> > > > current_class_ptr is set) within the trailing return type, and later
> > > > during substitution we can't resolve the 'this' since
> > > > tsubst_function_type does inject_this_parm only for non-static member
> > > > functions which tmp::func is not.
> > > > 
> > > > It seems the root of the problem is that we set current_class_ptr when
> > > > parsing the signature of a static member function.  Since explicit uses
> > > > of 'this' are already not allowed in this context, we probably shouldn't
> > > > be doing inject_this_parm either.
> > > 
> > > Hmm, 'this' is defined in a static member function, it's just ill-formed to
> > > use it:
> > > 
> > > 7.5.2/2: "... [this] shall not appear within the declaration of a static
> > > member function (although its type and value category are defined within a
> > > static member function as they are within a non-static member function).
> > > [Note: This is because declaration matching does not occur until the complete
> > > declarator is known. — end note]"
> > 
> > Ah, I see.
> > 
> > > 
> > > Perhaps maybe_dummy_object needs to be smarter about recognizing static
> > > context.  Or perhaps there's no way to tell the difference between the
> > > specified behavior above and the behavior with your patch, but:
> > > 
> > > The note suggests that we need to test the case of an out-of-class definition
> > > of a static member function (template); we must inject_this_parm when parsing
> > > such a declaration, since we don't know it's static until we match it to the
> > > declaration in the class.  I'm guessing that this would lead to the same
> > > problem.
> > 
> > Good point, we do get a signature mismatch error in this case, due to
> > 'this->' appearing in the trailing return type out-of-class declaration
> > but not in that of the in-class declaration, so the trailing return
> > types don't match up.  In light of this case, it doesn't seem possible
> > for maybe_dummy_object to distinguish between a static and non-static
> > context when parsing the trailing return type of the declaration.
> > 
> > So perhaps we should mirror at substitution time what we do at parse
> > time, and inject 'this' also when substituting into the function type of
> > a static member function?  That way, we'll resolve the use of 'this->'
> > and then discard it the RHS of -> is a static member function, or
> > complain that 'this' is not a constant expression if the RHS is a
> > non-static member function.
> > 
> > The following patch performs that, and seems to pass light testing.
> > Full testing in progress.  Revised commit message is still a WIP.
> > How does this approach look?

Sorry for the spam, but I'd also like to propose a more conservative and
targeted approach that basically undoes part of the r9-5972 patch which
caused the regression.

According to the email thread at
https://gcc.gnu.org/pipermail/gcc-patches/2019-February/516421.html
the hunk from r9-5972

--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class,
     {
       /* See if any of the functions are non-static members.  */
       /* If so, the expression may be relative to 'this'.  */
-      if (!shared_member_p (expr)
+      if ((type_dependent_expression_p (expr)
+          || !shared_member_p (expr))
          && current_class_ptr
          && DERIVED_FROM_P (qualifying_class,
                             current_nonlambda_class_type ()))

was added to avoid calling shared_member_p here when the overload set
contains a dependent USING_DECL.  But according to
https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513237.html if
an overload set contains a dependent USING_DECL, then it'll be the first
in the overload set.  So we could partially revert the r9-5972 patch and
do:

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index c6b4c70dc0f..6b0d68ae4bf 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2214,7 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
     {
       /* See if any of the functions are non-static members.  */
       /* If so, the expression may be relative to 'this'.  */
-      if ((type_dependent_expression_p (expr)
+      if ((TREE_CODE (get_first_fn (expr)) == USING_DECL
           || !shared_member_p (expr))
          && current_class_ptr
          && DERIVED_FROM_P (qualifying_class,

The above pases 'make check-c++', and allows us to compile pr97399a.C
successfully, but we'd still ICE on the invalid pr97399b.C.

> > 
> > -- >8 --
> > 
> > Subject: [PATCH] c++: 'this' injection and static member functions [PR97399]
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	PR c++/97399
> > 	* parser.c (cp_parser_init_declarator): If the storage class
> > 	specifier is sc_static, pass true for static_p to
> > 	cp_parser_declarator.
> > 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> > 	the function is specified as a friend.
> > 	* pt.c (tsubst_function_type): Also inject 'this' when
> > 	substituting into the function type of a static member
> > 	function.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	PR c++/88548
> > 	PR c++/97399
> > 	* g++.dg/cpp0x/this2.C: New test.
> > 	* g++.dg/gomp/this-1.C: Adjust expected error for use of 'this'
> > 	in signature of static member function.
> > 	* g++.dg/template/pr97399a.C: New test.
> > 	* g++.dg/template/pr97399b.C: New test.
> > ---
> >  gcc/cp/parser.c                          |  2 +-
> >  gcc/cp/pt.c                              | 13 +++++++++++--
> >  gcc/testsuite/g++.dg/template/pr97399a.C | 23 +++++++++++++++++++++++
> >  gcc/testsuite/g++.dg/template/pr97399b.C | 23 +++++++++++++++++++++++
> >  4 files changed, 58 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
> >  create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
> > 
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 48437f23175..a0efa55d21e 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -22122,7 +22122,7 @@ cp_parser_direct_declarator (cp_parser* parser,
> >  
> >  		  tree save_ccp = current_class_ptr;
> >  		  tree save_ccr = current_class_ref;
> > -		  if (memfn)
> > +		  if (memfn && !friend_p)
> >  		    /* DR 1207: 'this' is in scope after the cv-quals.  */
> >  		    inject_this_parameter (current_class_type, cv_quals);
> >  
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index ad855dbde72..d4763325e25 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -15072,8 +15072,17 @@ tsubst_function_type (tree t,
> >  
> >        tree save_ccp = current_class_ptr;
> >        tree save_ccr = current_class_ref;
> > -      tree this_type = (TREE_CODE (t) == METHOD_TYPE
> > -			? TREE_TYPE (TREE_VALUE (arg_types)) : NULL_TREE);
> > +      tree this_type = NULL_TREE;
> > +      if (TREE_CODE (t) == METHOD_TYPE)
> > +	this_type = TREE_TYPE (TREE_VALUE (arg_types));
> > +      else if (in_decl
> > +	       && DECL_FUNCTION_MEMBER_P (in_decl)
> 
> Oops, this line should instead be checking DECL_STATIC_FUNCTION_P.
> Consider it changed.
> 
> > +	       && t == TREE_TYPE (in_decl))
> > +	/* Also inject 'this' when substituting into the function type
> > +	   of a static member function, as we may have conservatively
> > +	   added 'this->' to a dependent member function call in its
> > +	   trailing return type which we might need to resolve.  */
> > +	this_type = DECL_CONTEXT (in_decl);
> >        bool do_inject = this_type && CLASS_TYPE_P (this_type);
> >        if (do_inject)
> >  	{
> > diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
> > new file mode 100644
> > index 00000000000..4bb818908fd
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
> > @@ -0,0 +1,23 @@
> > +// PR c++/97399
> > +// { dg-do compile { target c++11 } }
> > +
> > +template <bool> struct enable_if_t {};
> > +
> > +struct tmp {
> > +  template <class> static constexpr bool is_integral();
> > +  template <class T> static auto f()
> > +    -> enable_if_t<tmp::is_integral<T>()>;
> > +  template <class T> friend auto g(tmp, T)
> > +    -> enable_if_t<!tmp::is_integral<T>()>;
> > +};
> > +
> > +template <class> constexpr bool tmp::is_integral() { return true; }
> > +
> > +template <class T> auto tmp::f()
> > +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> > +
> > +int main()
> > +{
> > +  tmp::f<int>();
> > +  g(tmp{}, 0);
> > +}
> > diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
> > new file mode 100644
> > index 00000000000..673ba6624e3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
> > @@ -0,0 +1,23 @@
> > +// PR c++/97399
> > +// { dg-do compile { target c++11 } }
> > +
> > +template <bool> struct enable_if_t {};
> > +
> > +struct tmp {
> > +  template <class> constexpr bool is_integral(); // non-static
> > +  template <class T> static auto f()
> > +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-message "in template argument" }
> > +  template <class T> friend auto g(tmp, T)
> > +    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without object" }
> > +};
> > +
> > +template <class> constexpr bool tmp::is_integral() { return true; }
> > +
> > +template <class T> auto tmp::f() // { dg-error "'this' is not a constant" }
> > +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> > +
> > +int main()
> > +{
> > +  tmp::f<int>(); // { dg-error "no match" }
> > +  g(tmp{}, 0); // { dg-error "no match" }
> > +}
> > -- 
> > 2.30.0.155.g66e871b664
> > 

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Jason Merrill]

On 1/22/21 12:59 PM, Patrick Palka wrote:
> On Fri, 22 Jan 2021, Patrick Palka wrote:
> 
>> On Fri, 22 Jan 2021, Patrick Palka wrote:
>>
>>> On Thu, 21 Jan 2021, Jason Merrill wrote:
>>>
>>>> On 1/21/21 11:22 AM, Patrick Palka wrote:
>>>>> Here at parse time finish_qualified_id_expr adds an implicit 'this->' to
>>>>> the expression tmp::integral<T> (because it's type-dependent, and also
>>>>> current_class_ptr is set) within the trailing return type, and later
>>>>> during substitution we can't resolve the 'this' since
>>>>> tsubst_function_type does inject_this_parm only for non-static member
>>>>> functions which tmp::func is not.
>>>>>
>>>>> It seems the root of the problem is that we set current_class_ptr when
>>>>> parsing the signature of a static member function.  Since explicit uses
>>>>> of 'this' are already not allowed in this context, we probably shouldn't
>>>>> be doing inject_this_parm either.
>>>>
>>>> Hmm, 'this' is defined in a static member function, it's just ill-formed to
>>>> use it:
>>>>
>>>> 7.5.2/2: "... [this] shall not appear within the declaration of a static
>>>> member function (although its type and value category are defined within a
>>>> static member function as they are within a non-static member function).
>>>> [Note: This is because declaration matching does not occur until the complete
>>>> declarator is known. — end note]"
>>>
>>> Ah, I see.
>>>
>>>>
>>>> Perhaps maybe_dummy_object needs to be smarter about recognizing static
>>>> context.  Or perhaps there's no way to tell the difference between the
>>>> specified behavior above and the behavior with your patch, but:
>>>>
>>>> The note suggests that we need to test the case of an out-of-class definition
>>>> of a static member function (template); we must inject_this_parm when parsing
>>>> such a declaration, since we don't know it's static until we match it to the
>>>> declaration in the class.  I'm guessing that this would lead to the same
>>>> problem.
>>>
>>> Good point, we do get a signature mismatch error in this case, due to
>>> 'this->' appearing in the trailing return type out-of-class declaration
>>> but not in that of the in-class declaration, so the trailing return
>>> types don't match up.  In light of this case, it doesn't seem possible
>>> for maybe_dummy_object to distinguish between a static and non-static
>>> context when parsing the trailing return type of the declaration.
>>>
>>> So perhaps we should mirror at substitution time what we do at parse
>>> time, and inject 'this' also when substituting into the function type of
>>> a static member function?  That way, we'll resolve the use of 'this->'
>>> and then discard it the RHS of -> is a static member function, or
>>> complain that 'this' is not a constant expression if the RHS is a
>>> non-static member function.
>>>
>>> The following patch performs that, and seems to pass light testing.
>>> Full testing in progress.  Revised commit message is still a WIP.
>>> How does this approach look?
> 
> Sorry for the spam, but I'd also like to propose a more conservative and
> targeted approach that basically undoes part of the r9-5972 patch which
> caused the regression.
> 
> According to the email thread at
> https://gcc.gnu.org/pipermail/gcc-patches/2019-February/516421.html
> the hunk from r9-5972
> 
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class,
>       {
>         /* See if any of the functions are non-static members.  */
>         /* If so, the expression may be relative to 'this'.  */
> -      if (!shared_member_p (expr)
> +      if ((type_dependent_expression_p (expr)
> +          || !shared_member_p (expr))
>            && current_class_ptr
>            && DERIVED_FROM_P (qualifying_class,
>                               current_nonlambda_class_type ()))
> 
> was added to avoid calling shared_member_p here when the overload set
> contains a dependent USING_DECL.  But according to
> https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513237.html if
> an overload set contains a dependent USING_DECL, then it'll be the first
> in the overload set.  So we could partially revert the r9-5972 patch and
> do:
> 
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index c6b4c70dc0f..6b0d68ae4bf 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -2214,7 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
>       {
>         /* See if any of the functions are non-static members.  */
>         /* If so, the expression may be relative to 'this'.  */
> -      if ((type_dependent_expression_p (expr)
> +      if ((TREE_CODE (get_first_fn (expr)) == USING_DECL
>             || !shared_member_p (expr))
>            && current_class_ptr
>            && DERIVED_FROM_P (qualifying_class,
> 
> The above pases 'make check-c++', and allows us to compile pr97399a.C
> successfully, but we'd still ICE on the invalid pr97399b.C.

What if we flipped the sense of the type_dependent_expression_p check, 
so we never add this-> if the function operand is dependent?

>>> -- >8 --
>>>
>>> Subject: [PATCH] c++: 'this' injection and static member functions [PR97399]
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	PR c++/97399
>>> 	* parser.c (cp_parser_init_declarator): If the storage class
>>> 	specifier is sc_static, pass true for static_p to
>>> 	cp_parser_declarator.

I don't see this change in the patch; it seems like a good cleanup even 
if it isn't needed for this bug.

>>> 	(cp_parser_direct_declarator): Don't do inject_this_parm when
>>> 	the function is specified as a friend.
>>> 	* pt.c (tsubst_function_type): Also inject 'this' when
>>> 	substituting into the function type of a static member
>>> 	function.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	PR c++/88548
>>> 	PR c++/97399
>>> 	* g++.dg/cpp0x/this2.C: New test.
>>> 	* g++.dg/gomp/this-1.C: Adjust expected error for use of 'this'
>>> 	in signature of static member function.
>>> 	* g++.dg/template/pr97399a.C: New test.
>>> 	* g++.dg/template/pr97399b.C: New test.
>>> ---
>>>   gcc/cp/parser.c                          |  2 +-
>>>   gcc/cp/pt.c                              | 13 +++++++++++--
>>>   gcc/testsuite/g++.dg/template/pr97399a.C | 23 +++++++++++++++++++++++
>>>   gcc/testsuite/g++.dg/template/pr97399b.C | 23 +++++++++++++++++++++++
>>>   4 files changed, 58 insertions(+), 3 deletions(-)
>>>   create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
>>>   create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
>>>
>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>>> index 48437f23175..a0efa55d21e 100644
>>> --- a/gcc/cp/parser.c
>>> +++ b/gcc/cp/parser.c
>>> @@ -22122,7 +22122,7 @@ cp_parser_direct_declarator (cp_parser* parser,
>>>   
>>>   		  tree save_ccp = current_class_ptr;
>>>   		  tree save_ccr = current_class_ref;
>>> -		  if (memfn)
>>> +		  if (memfn && !friend_p)
>>>   		    /* DR 1207: 'this' is in scope after the cv-quals.  */
>>>   		    inject_this_parameter (current_class_type, cv_quals);
>>>   
>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
>>> index ad855dbde72..d4763325e25 100644
>>> --- a/gcc/cp/pt.c
>>> +++ b/gcc/cp/pt.c
>>> @@ -15072,8 +15072,17 @@ tsubst_function_type (tree t,
>>>   
>>>         tree save_ccp = current_class_ptr;
>>>         tree save_ccr = current_class_ref;
>>> -      tree this_type = (TREE_CODE (t) == METHOD_TYPE
>>> -			? TREE_TYPE (TREE_VALUE (arg_types)) : NULL_TREE);
>>> +      tree this_type = NULL_TREE;
>>> +      if (TREE_CODE (t) == METHOD_TYPE)
>>> +	this_type = TREE_TYPE (TREE_VALUE (arg_types));
>>> +      else if (in_decl
>>> +	       && DECL_FUNCTION_MEMBER_P (in_decl)
>>
>> Oops, this line should instead be checking DECL_STATIC_FUNCTION_P.
>> Consider it changed.
>>
>>> +	       && t == TREE_TYPE (in_decl))
>>> +	/* Also inject 'this' when substituting into the function type
>>> +	   of a static member function, as we may have conservatively
>>> +	   added 'this->' to a dependent member function call in its
>>> +	   trailing return type which we might need to resolve.  */
>>> +	this_type = DECL_CONTEXT (in_decl);
>>>         bool do_inject = this_type && CLASS_TYPE_P (this_type);
>>>         if (do_inject)
>>>   	{
>>> diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C b/gcc/testsuite/g++.dg/template/pr97399a.C
>>> new file mode 100644
>>> index 00000000000..4bb818908fd
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
>>> @@ -0,0 +1,23 @@
>>> +// PR c++/97399
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +template <bool> struct enable_if_t {};
>>> +
>>> +struct tmp {
>>> +  template <class> static constexpr bool is_integral();
>>> +  template <class T> static auto f()
>>> +    -> enable_if_t<tmp::is_integral<T>()>;
>>> +  template <class T> friend auto g(tmp, T)
>>> +    -> enable_if_t<!tmp::is_integral<T>()>;
>>> +};
>>> +
>>> +template <class> constexpr bool tmp::is_integral() { return true; }
>>> +
>>> +template <class T> auto tmp::f()
>>> +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
>>> +
>>> +int main()
>>> +{
>>> +  tmp::f<int>();
>>> +  g(tmp{}, 0);
>>> +}
>>> diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C b/gcc/testsuite/g++.dg/template/pr97399b.C
>>> new file mode 100644
>>> index 00000000000..673ba6624e3
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
>>> @@ -0,0 +1,23 @@
>>> +// PR c++/97399
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +template <bool> struct enable_if_t {};
>>> +
>>> +struct tmp {
>>> +  template <class> constexpr bool is_integral(); // non-static
>>> +  template <class T> static auto f()
>>> +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-message "in template argument" }
>>> +  template <class T> friend auto g(tmp, T)
>>> +    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without object" }
>>> +};
>>> +
>>> +template <class> constexpr bool tmp::is_integral() { return true; }
>>> +
>>> +template <class T> auto tmp::f() // { dg-error "'this' is not a constant" }
>>> +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
>>> +
>>> +int main()
>>> +{
>>> +  tmp::f<int>(); // { dg-error "no match" }
>>> +  g(tmp{}, 0); // { dg-error "no match" }
>>> +}
>>> -- 
>>> 2.30.0.155.g66e871b664

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

On Fri, 22 Jan 2021, Jason Merrill wrote:

> On 1/22/21 12:59 PM, Patrick Palka wrote:
> > On Fri, 22 Jan 2021, Patrick Palka wrote:
> > 
> > > On Fri, 22 Jan 2021, Patrick Palka wrote:
> > > 
> > > > On Thu, 21 Jan 2021, Jason Merrill wrote:
> > > > 
> > > > > On 1/21/21 11:22 AM, Patrick Palka wrote:
> > > > > > Here at parse time finish_qualified_id_expr adds an implicit
> > > > > > 'this->' to
> > > > > > the expression tmp::integral<T> (because it's type-dependent, and
> > > > > > also
> > > > > > current_class_ptr is set) within the trailing return type, and later
> > > > > > during substitution we can't resolve the 'this' since
> > > > > > tsubst_function_type does inject_this_parm only for non-static
> > > > > > member
> > > > > > functions which tmp::func is not.
> > > > > > 
> > > > > > It seems the root of the problem is that we set current_class_ptr
> > > > > > when
> > > > > > parsing the signature of a static member function.  Since explicit
> > > > > > uses
> > > > > > of 'this' are already not allowed in this context, we probably
> > > > > > shouldn't
> > > > > > be doing inject_this_parm either.
> > > > > 
> > > > > Hmm, 'this' is defined in a static member function, it's just
> > > > > ill-formed to
> > > > > use it:
> > > > > 
> > > > > 7.5.2/2: "... [this] shall not appear within the declaration of a
> > > > > static
> > > > > member function (although its type and value category are defined
> > > > > within a
> > > > > static member function as they are within a non-static member
> > > > > function).
> > > > > [Note: This is because declaration matching does not occur until the
> > > > > complete
> > > > > declarator is known. — end note]"
> > > > 
> > > > Ah, I see.
> > > > 
> > > > > 
> > > > > Perhaps maybe_dummy_object needs to be smarter about recognizing
> > > > > static
> > > > > context.  Or perhaps there's no way to tell the difference between the
> > > > > specified behavior above and the behavior with your patch, but:
> > > > > 
> > > > > The note suggests that we need to test the case of an out-of-class
> > > > > definition
> > > > > of a static member function (template); we must inject_this_parm when
> > > > > parsing
> > > > > such a declaration, since we don't know it's static until we match it
> > > > > to the
> > > > > declaration in the class.  I'm guessing that this would lead to the
> > > > > same
> > > > > problem.
> > > > 
> > > > Good point, we do get a signature mismatch error in this case, due to
> > > > 'this->' appearing in the trailing return type out-of-class declaration
> > > > but not in that of the in-class declaration, so the trailing return
> > > > types don't match up.  In light of this case, it doesn't seem possible
> > > > for maybe_dummy_object to distinguish between a static and non-static
> > > > context when parsing the trailing return type of the declaration.
> > > > 
> > > > So perhaps we should mirror at substitution time what we do at parse
> > > > time, and inject 'this' also when substituting into the function type of
> > > > a static member function?  That way, we'll resolve the use of 'this->'
> > > > and then discard it the RHS of -> is a static member function, or
> > > > complain that 'this' is not a constant expression if the RHS is a
> > > > non-static member function.
> > > > 
> > > > The following patch performs that, and seems to pass light testing.
> > > > Full testing in progress.  Revised commit message is still a WIP.
> > > > How does this approach look?
> > 
> > Sorry for the spam, but I'd also like to propose a more conservative and
> > targeted approach that basically undoes part of the r9-5972 patch which
> > caused the regression.
> > 
> > According to the email thread at
> > https://gcc.gnu.org/pipermail/gcc-patches/2019-February/516421.html
> > the hunk from r9-5972
> > 
> > --- a/gcc/cp/semantics.c
> > +++ b/gcc/cp/semantics.c
> > @@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class,
> >       {
> >         /* See if any of the functions are non-static members.  */
> >         /* If so, the expression may be relative to 'this'.  */
> > -      if (!shared_member_p (expr)
> > +      if ((type_dependent_expression_p (expr)
> > +          || !shared_member_p (expr))
> >            && current_class_ptr
> >            && DERIVED_FROM_P (qualifying_class,
> >                               current_nonlambda_class_type ()))
> > 
> > was added to avoid calling shared_member_p here when the overload set
> > contains a dependent USING_DECL.  But according to
> > https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513237.html if
> > an overload set contains a dependent USING_DECL, then it'll be the first
> > in the overload set.  So we could partially revert the r9-5972 patch and
> > do:
> > 
> > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> > index c6b4c70dc0f..6b0d68ae4bf 100644
> > --- a/gcc/cp/semantics.c
> > +++ b/gcc/cp/semantics.c
> > @@ -2214,7 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
> >       {
> >         /* See if any of the functions are non-static members.  */
> >         /* If so, the expression may be relative to 'this'.  */
> > -      if ((type_dependent_expression_p (expr)
> > +      if ((TREE_CODE (get_first_fn (expr)) == USING_DECL
> >             || !shared_member_p (expr))
> >            && current_class_ptr
> >            && DERIVED_FROM_P (qualifying_class,
> > 
> > The above pases 'make check-c++', and allows us to compile pr97399a.C
> > successfully, but we'd still ICE on the invalid pr97399b.C.
> 
> What if we flipped the sense of the type_dependent_expression_p check, so we
> never add this-> if the function operand is dependent?

Looks like this doesn't survive 'make check-c++', we crash on the
testcase g++.dg/cpp1y/lambda-generic-this1a.C:

gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C: In instantiation of ‘MyType::crash()::<lambda(auto:1)> [with auto:1 = A]’:
gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:12:10:   required from here
gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:9:28: internal compiler error: trying to capture ‘this’ in instantiation of generic lambda
    9 |       MyType::make_crash<i>(); // Line (1)
      |       ~~~~~~~~~~~~~~~~~~~~~^~
0x9bab23 add_capture(tree_node*, tree_node*, tree_node*, bool, bool)
        /home/patrick/gcc-master/gcc/cp/lambda.c:633
0x9bac75 add_default_capture(tree_node*, tree_node*, tree_node*)
        /home/patrick/gcc-master/gcc/cp/lambda.c:693
0x9bb3a1 lambda_expr_this_capture(tree_node*, int)
        /home/patrick/gcc-master/gcc/cp/lambda.c:811
0x9bb4a0 maybe_resolve_dummy(tree_node*, bool)
        /home/patrick/gcc-master/gcc/cp/lambda.c:894
0x8de510 build_new_method_call_1
        /home/patrick/gcc-master/gcc/cp/call.c:10503
0x8df04f build_new_method_call(tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>**, tree_node*, int, tree_node**, int)

Presumably because we no longer add 'this->' to the dependent call
MyType::make_crash<i> inside the generic lambda, so we don't capture it
when we should have?

To be clear, this is with

--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2214,8 +2214,8 @@ finish_qualified_id_expr (tree qualifying_class,
     {
       /* See if any of the functions are non-static members.  */
       /* If so, the expression may be relative to 'this'.  */
-      if ((type_dependent_expression_p (expr)
-          || !shared_member_p (expr))
+      if (!type_dependent_expression_p (expr)
+         && !shared_member_p (expr)
          && current_class_ptr
          && DERIVED_FROM_P (qualifying_class,
                             current_nonlambda_class_type ()))

> 
> > > > -- >8 --
> > > > 
> > > > Subject: [PATCH] c++: 'this' injection and static member functions
> > > > [PR97399]
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > 	PR c++/97399
> > > > 	* parser.c (cp_parser_init_declarator): If the storage class
> > > > 	specifier is sc_static, pass true for static_p to
> > > > 	cp_parser_declarator.
> 
> I don't see this change in the patch; it seems like a good cleanup even if it
> isn't needed for this bug.

Sounds good.  I'll add it back in (and the testcase cpp0x/this2.C) in the next
iteration of the patch.

> 
> > > > 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> > > > 	the function is specified as a friend.
> > > > 	* pt.c (tsubst_function_type): Also inject 'this' when
> > > > 	substituting into the function type of a static member
> > > > 	function.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 	PR c++/88548
> > > > 	PR c++/97399
> > > > 	* g++.dg/cpp0x/this2.C: New test.
> > > > 	* g++.dg/gomp/this-1.C: Adjust expected error for use of 'this'
> > > > 	in signature of static member function.
> > > > 	* g++.dg/template/pr97399a.C: New test.
> > > > 	* g++.dg/template/pr97399b.C: New test.
> > > > ---
> > > >   gcc/cp/parser.c                          |  2 +-
> > > >   gcc/cp/pt.c                              | 13 +++++++++++--
> > > >   gcc/testsuite/g++.dg/template/pr97399a.C | 23 +++++++++++++++++++++++
> > > >   gcc/testsuite/g++.dg/template/pr97399b.C | 23 +++++++++++++++++++++++
> > > >   4 files changed, 58 insertions(+), 3 deletions(-)
> > > >   create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
> > > >   create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
> > > > 
> > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > > > index 48437f23175..a0efa55d21e 100644
> > > > --- a/gcc/cp/parser.c
> > > > +++ b/gcc/cp/parser.c
> > > > @@ -22122,7 +22122,7 @@ cp_parser_direct_declarator (cp_parser* parser,
> > > >     		  tree save_ccp = current_class_ptr;
> > > >   		  tree save_ccr = current_class_ref;
> > > > -		  if (memfn)
> > > > +		  if (memfn && !friend_p)
> > > >   		    /* DR 1207: 'this' is in scope after the cv-quals.
> > > > */
> > > >   		    inject_this_parameter (current_class_type,
> > > > cv_quals);
> > > >   diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > index ad855dbde72..d4763325e25 100644
> > > > --- a/gcc/cp/pt.c
> > > > +++ b/gcc/cp/pt.c
> > > > @@ -15072,8 +15072,17 @@ tsubst_function_type (tree t,
> > > >           tree save_ccp = current_class_ptr;
> > > >         tree save_ccr = current_class_ref;
> > > > -      tree this_type = (TREE_CODE (t) == METHOD_TYPE
> > > > -			? TREE_TYPE (TREE_VALUE (arg_types)) : NULL_TREE);
> > > > +      tree this_type = NULL_TREE;
> > > > +      if (TREE_CODE (t) == METHOD_TYPE)
> > > > +	this_type = TREE_TYPE (TREE_VALUE (arg_types));
> > > > +      else if (in_decl
> > > > +	       && DECL_FUNCTION_MEMBER_P (in_decl)
> > > 
> > > Oops, this line should instead be checking DECL_STATIC_FUNCTION_P.
> > > Consider it changed.
> > > 
> > > > +	       && t == TREE_TYPE (in_decl))
> > > > +	/* Also inject 'this' when substituting into the function type
> > > > +	   of a static member function, as we may have conservatively
> > > > +	   added 'this->' to a dependent member function call in its
> > > > +	   trailing return type which we might need to resolve.  */
> > > > +	this_type = DECL_CONTEXT (in_decl);
> > > >         bool do_inject = this_type && CLASS_TYPE_P (this_type);
> > > >         if (do_inject)
> > > >   	{
> > > > diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C
> > > > b/gcc/testsuite/g++.dg/template/pr97399a.C
> > > > new file mode 100644
> > > > index 00000000000..4bb818908fd
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
> > > > @@ -0,0 +1,23 @@
> > > > +// PR c++/97399
> > > > +// { dg-do compile { target c++11 } }
> > > > +
> > > > +template <bool> struct enable_if_t {};
> > > > +
> > > > +struct tmp {
> > > > +  template <class> static constexpr bool is_integral();
> > > > +  template <class T> static auto f()
> > > > +    -> enable_if_t<tmp::is_integral<T>()>;
> > > > +  template <class T> friend auto g(tmp, T)
> > > > +    -> enable_if_t<!tmp::is_integral<T>()>;
> > > > +};
> > > > +
> > > > +template <class> constexpr bool tmp::is_integral() { return true; }
> > > > +
> > > > +template <class T> auto tmp::f()
> > > > +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> > > > +
> > > > +int main()
> > > > +{
> > > > +  tmp::f<int>();
> > > > +  g(tmp{}, 0);
> > > > +}
> > > > diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C
> > > > b/gcc/testsuite/g++.dg/template/pr97399b.C
> > > > new file mode 100644
> > > > index 00000000000..673ba6624e3
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
> > > > @@ -0,0 +1,23 @@
> > > > +// PR c++/97399
> > > > +// { dg-do compile { target c++11 } }
> > > > +
> > > > +template <bool> struct enable_if_t {};
> > > > +
> > > > +struct tmp {
> > > > +  template <class> constexpr bool is_integral(); // non-static
> > > > +  template <class T> static auto f()
> > > > +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-message "in template
> > > > argument" }
> > > > +  template <class T> friend auto g(tmp, T)
> > > > +    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without
> > > > object" }
> > > > +};
> > > > +
> > > > +template <class> constexpr bool tmp::is_integral() { return true; }
> > > > +
> > > > +template <class T> auto tmp::f() // { dg-error "'this' is not a
> > > > constant" }
> > > > +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> > > > +
> > > > +int main()
> > > > +{
> > > > +  tmp::f<int>(); // { dg-error "no match" }
> > > > +  g(tmp{}, 0); // { dg-error "no match" }
> > > > +}
> > > > -- 
> > > > 2.30.0.155.g66e871b664
> 
> 

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Jason Merrill]

On 1/22/21 1:58 PM, Patrick Palka wrote:
> On Fri, 22 Jan 2021, Jason Merrill wrote:
> 
>> On 1/22/21 12:59 PM, Patrick Palka wrote:
>>> On Fri, 22 Jan 2021, Patrick Palka wrote:
>>>
>>>> On Fri, 22 Jan 2021, Patrick Palka wrote:
>>>>
>>>>> On Thu, 21 Jan 2021, Jason Merrill wrote:
>>>>>
>>>>>> On 1/21/21 11:22 AM, Patrick Palka wrote:
>>>>>>> Here at parse time finish_qualified_id_expr adds an implicit
>>>>>>> 'this->' to
>>>>>>> the expression tmp::integral<T> (because it's type-dependent, and
>>>>>>> also
>>>>>>> current_class_ptr is set) within the trailing return type, and later
>>>>>>> during substitution we can't resolve the 'this' since
>>>>>>> tsubst_function_type does inject_this_parm only for non-static
>>>>>>> member
>>>>>>> functions which tmp::func is not.
>>>>>>>
>>>>>>> It seems the root of the problem is that we set current_class_ptr
>>>>>>> when
>>>>>>> parsing the signature of a static member function.  Since explicit
>>>>>>> uses
>>>>>>> of 'this' are already not allowed in this context, we probably
>>>>>>> shouldn't
>>>>>>> be doing inject_this_parm either.
>>>>>>
>>>>>> Hmm, 'this' is defined in a static member function, it's just
>>>>>> ill-formed to
>>>>>> use it:
>>>>>>
>>>>>> 7.5.2/2: "... [this] shall not appear within the declaration of a
>>>>>> static
>>>>>> member function (although its type and value category are defined
>>>>>> within a
>>>>>> static member function as they are within a non-static member
>>>>>> function).
>>>>>> [Note: This is because declaration matching does not occur until the
>>>>>> complete
>>>>>> declarator is known. — end note]"
>>>>>
>>>>> Ah, I see.
>>>>>
>>>>>>
>>>>>> Perhaps maybe_dummy_object needs to be smarter about recognizing
>>>>>> static
>>>>>> context.  Or perhaps there's no way to tell the difference between the
>>>>>> specified behavior above and the behavior with your patch, but:
>>>>>>
>>>>>> The note suggests that we need to test the case of an out-of-class
>>>>>> definition
>>>>>> of a static member function (template); we must inject_this_parm when
>>>>>> parsing
>>>>>> such a declaration, since we don't know it's static until we match it
>>>>>> to the
>>>>>> declaration in the class.  I'm guessing that this would lead to the
>>>>>> same
>>>>>> problem.
>>>>>
>>>>> Good point, we do get a signature mismatch error in this case, due to
>>>>> 'this->' appearing in the trailing return type out-of-class declaration
>>>>> but not in that of the in-class declaration, so the trailing return
>>>>> types don't match up.  In light of this case, it doesn't seem possible
>>>>> for maybe_dummy_object to distinguish between a static and non-static
>>>>> context when parsing the trailing return type of the declaration.
>>>>>
>>>>> So perhaps we should mirror at substitution time what we do at parse
>>>>> time, and inject 'this' also when substituting into the function type of
>>>>> a static member function?  That way, we'll resolve the use of 'this->'
>>>>> and then discard it the RHS of -> is a static member function, or
>>>>> complain that 'this' is not a constant expression if the RHS is a
>>>>> non-static member function.
>>>>>
>>>>> The following patch performs that, and seems to pass light testing.
>>>>> Full testing in progress.  Revised commit message is still a WIP.
>>>>> How does this approach look?
>>>
>>> Sorry for the spam, but I'd also like to propose a more conservative and
>>> targeted approach that basically undoes part of the r9-5972 patch which
>>> caused the regression.
>>>
>>> According to the email thread at
>>> https://gcc.gnu.org/pipermail/gcc-patches/2019-February/516421.html
>>> the hunk from r9-5972
>>>
>>> --- a/gcc/cp/semantics.c
>>> +++ b/gcc/cp/semantics.c
>>> @@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class,
>>>        {
>>>          /* See if any of the functions are non-static members.  */
>>>          /* If so, the expression may be relative to 'this'.  */
>>> -      if (!shared_member_p (expr)
>>> +      if ((type_dependent_expression_p (expr)
>>> +          || !shared_member_p (expr))
>>>             && current_class_ptr
>>>             && DERIVED_FROM_P (qualifying_class,
>>>                                current_nonlambda_class_type ()))
>>>
>>> was added to avoid calling shared_member_p here when the overload set
>>> contains a dependent USING_DECL.  But according to
>>> https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513237.html if
>>> an overload set contains a dependent USING_DECL, then it'll be the first
>>> in the overload set.  So we could partially revert the r9-5972 patch and
>>> do:
>>>
>>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
>>> index c6b4c70dc0f..6b0d68ae4bf 100644
>>> --- a/gcc/cp/semantics.c
>>> +++ b/gcc/cp/semantics.c
>>> @@ -2214,7 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
>>>        {
>>>          /* See if any of the functions are non-static members.  */
>>>          /* If so, the expression may be relative to 'this'.  */
>>> -      if ((type_dependent_expression_p (expr)
>>> +      if ((TREE_CODE (get_first_fn (expr)) == USING_DECL
>>>              || !shared_member_p (expr))
>>>             && current_class_ptr
>>>             && DERIVED_FROM_P (qualifying_class,
>>>
>>> The above pases 'make check-c++', and allows us to compile pr97399a.C
>>> successfully, but we'd still ICE on the invalid pr97399b.C.
>>
>> What if we flipped the sense of the type_dependent_expression_p check, so we
>> never add this-> if the function operand is dependent?
> 
> Looks like this doesn't survive 'make check-c++', we crash on the
> testcase g++.dg/cpp1y/lambda-generic-this1a.C:
> 
> gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C: In instantiation of ‘MyType::crash()::<lambda(auto:1)> [with auto:1 = A]’:
> gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:12:10:   required from here
> gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:9:28: internal compiler error: trying to capture ‘this’ in instantiation of generic lambda
>      9 |       MyType::make_crash<i>(); // Line (1)
>        |       ~~~~~~~~~~~~~~~~~~~~~^~
> 0x9bab23 add_capture(tree_node*, tree_node*, tree_node*, bool, bool)
>          /home/patrick/gcc-master/gcc/cp/lambda.c:633
> 0x9bac75 add_default_capture(tree_node*, tree_node*, tree_node*)
>          /home/patrick/gcc-master/gcc/cp/lambda.c:693
> 0x9bb3a1 lambda_expr_this_capture(tree_node*, int)
>          /home/patrick/gcc-master/gcc/cp/lambda.c:811
> 0x9bb4a0 maybe_resolve_dummy(tree_node*, bool)
>          /home/patrick/gcc-master/gcc/cp/lambda.c:894
> 0x8de510 build_new_method_call_1
>          /home/patrick/gcc-master/gcc/cp/call.c:10503
> 0x8df04f build_new_method_call(tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>**, tree_node*, int, tree_node**, int)
> 
> Presumably because we no longer add 'this->' to the dependent call
> MyType::make_crash<i> inside the generic lambda, so we don't capture it
> when we should have?

Ah, yes, we still need to add this-> in a generic lambda, or at least 
capture 'this'.

I think I was wrong in my assertion around Alex's patch that 
shared_member_p should abort on a dependent USING_DECL; it now seems 
appropriate for it to return false if we don't know, we just need to 
adjust the comment to say that.

And then drop the type-dependent check.

> To be clear, this is with
> 
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -2214,8 +2214,8 @@ finish_qualified_id_expr (tree qualifying_class,
>       {
>         /* See if any of the functions are non-static members.  */
>         /* If so, the expression may be relative to 'this'.  */
> -      if ((type_dependent_expression_p (expr)
> -          || !shared_member_p (expr))
> +      if (!type_dependent_expression_p (expr)
> +         && !shared_member_p (expr)
>            && current_class_ptr
>            && DERIVED_FROM_P (qualifying_class,
>                               current_nonlambda_class_type ()))
> 
>>
>>>>> -- >8 --
>>>>>
>>>>> Subject: [PATCH] c++: 'this' injection and static member functions
>>>>> [PR97399]
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 	PR c++/97399
>>>>> 	* parser.c (cp_parser_init_declarator): If the storage class
>>>>> 	specifier is sc_static, pass true for static_p to
>>>>> 	cp_parser_declarator.
>>
>> I don't see this change in the patch; it seems like a good cleanup even if it
>> isn't needed for this bug.
> 
> Sounds good.  I'll add it back in (and the testcase cpp0x/this2.C) in the next
> iteration of the patch.
> 
>>
>>>>> 	(cp_parser_direct_declarator): Don't do inject_this_parm when
>>>>> 	the function is specified as a friend.
>>>>> 	* pt.c (tsubst_function_type): Also inject 'this' when
>>>>> 	substituting into the function type of a static member
>>>>> 	function.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 	PR c++/88548
>>>>> 	PR c++/97399
>>>>> 	* g++.dg/cpp0x/this2.C: New test.
>>>>> 	* g++.dg/gomp/this-1.C: Adjust expected error for use of 'this'
>>>>> 	in signature of static member function.
>>>>> 	* g++.dg/template/pr97399a.C: New test.
>>>>> 	* g++.dg/template/pr97399b.C: New test.
>>>>> ---
>>>>>    gcc/cp/parser.c                          |  2 +-
>>>>>    gcc/cp/pt.c                              | 13 +++++++++++--
>>>>>    gcc/testsuite/g++.dg/template/pr97399a.C | 23 +++++++++++++++++++++++
>>>>>    gcc/testsuite/g++.dg/template/pr97399b.C | 23 +++++++++++++++++++++++
>>>>>    4 files changed, 58 insertions(+), 3 deletions(-)
>>>>>    create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
>>>>>    create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
>>>>>
>>>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>>>>> index 48437f23175..a0efa55d21e 100644
>>>>> --- a/gcc/cp/parser.c
>>>>> +++ b/gcc/cp/parser.c
>>>>> @@ -22122,7 +22122,7 @@ cp_parser_direct_declarator (cp_parser* parser,
>>>>>      		  tree save_ccp = current_class_ptr;
>>>>>    		  tree save_ccr = current_class_ref;
>>>>> -		  if (memfn)
>>>>> +		  if (memfn && !friend_p)
>>>>>    		    /* DR 1207: 'this' is in scope after the cv-quals.
>>>>> */
>>>>>    		    inject_this_parameter (current_class_type,
>>>>> cv_quals);
>>>>>    diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
>>>>> index ad855dbde72..d4763325e25 100644
>>>>> --- a/gcc/cp/pt.c
>>>>> +++ b/gcc/cp/pt.c
>>>>> @@ -15072,8 +15072,17 @@ tsubst_function_type (tree t,
>>>>>            tree save_ccp = current_class_ptr;
>>>>>          tree save_ccr = current_class_ref;
>>>>> -      tree this_type = (TREE_CODE (t) == METHOD_TYPE
>>>>> -			? TREE_TYPE (TREE_VALUE (arg_types)) : NULL_TREE);
>>>>> +      tree this_type = NULL_TREE;
>>>>> +      if (TREE_CODE (t) == METHOD_TYPE)
>>>>> +	this_type = TREE_TYPE (TREE_VALUE (arg_types));
>>>>> +      else if (in_decl
>>>>> +	       && DECL_FUNCTION_MEMBER_P (in_decl)
>>>>
>>>> Oops, this line should instead be checking DECL_STATIC_FUNCTION_P.
>>>> Consider it changed.
>>>>
>>>>> +	       && t == TREE_TYPE (in_decl))
>>>>> +	/* Also inject 'this' when substituting into the function type
>>>>> +	   of a static member function, as we may have conservatively
>>>>> +	   added 'this->' to a dependent member function call in its
>>>>> +	   trailing return type which we might need to resolve.  */
>>>>> +	this_type = DECL_CONTEXT (in_decl);
>>>>>          bool do_inject = this_type && CLASS_TYPE_P (this_type);
>>>>>          if (do_inject)
>>>>>    	{
>>>>> diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C
>>>>> b/gcc/testsuite/g++.dg/template/pr97399a.C
>>>>> new file mode 100644
>>>>> index 00000000000..4bb818908fd
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
>>>>> @@ -0,0 +1,23 @@
>>>>> +// PR c++/97399
>>>>> +// { dg-do compile { target c++11 } }
>>>>> +
>>>>> +template <bool> struct enable_if_t {};
>>>>> +
>>>>> +struct tmp {
>>>>> +  template <class> static constexpr bool is_integral();
>>>>> +  template <class T> static auto f()
>>>>> +    -> enable_if_t<tmp::is_integral<T>()>;
>>>>> +  template <class T> friend auto g(tmp, T)
>>>>> +    -> enable_if_t<!tmp::is_integral<T>()>;
>>>>> +};
>>>>> +
>>>>> +template <class> constexpr bool tmp::is_integral() { return true; }
>>>>> +
>>>>> +template <class T> auto tmp::f()
>>>>> +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
>>>>> +
>>>>> +int main()
>>>>> +{
>>>>> +  tmp::f<int>();
>>>>> +  g(tmp{}, 0);
>>>>> +}
>>>>> diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C
>>>>> b/gcc/testsuite/g++.dg/template/pr97399b.C
>>>>> new file mode 100644
>>>>> index 00000000000..673ba6624e3
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
>>>>> @@ -0,0 +1,23 @@
>>>>> +// PR c++/97399
>>>>> +// { dg-do compile { target c++11 } }
>>>>> +
>>>>> +template <bool> struct enable_if_t {};
>>>>> +
>>>>> +struct tmp {
>>>>> +  template <class> constexpr bool is_integral(); // non-static
>>>>> +  template <class T> static auto f()
>>>>> +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-message "in template
>>>>> argument" }
>>>>> +  template <class T> friend auto g(tmp, T)
>>>>> +    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without
>>>>> object" }
>>>>> +};
>>>>> +
>>>>> +template <class> constexpr bool tmp::is_integral() { return true; }
>>>>> +
>>>>> +template <class T> auto tmp::f() // { dg-error "'this' is not a
>>>>> constant" }
>>>>> +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
>>>>> +
>>>>> +int main()
>>>>> +{
>>>>> +  tmp::f<int>(); // { dg-error "no match" }
>>>>> +  g(tmp{}, 0); // { dg-error "no match" }
>>>>> +}
>>>>> -- 
>>>>> 2.30.0.155.g66e871b664
>>

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Patrick Palka]

On Fri, 22 Jan 2021, Jason Merrill wrote:

> On 1/22/21 1:58 PM, Patrick Palka wrote:
> > On Fri, 22 Jan 2021, Jason Merrill wrote:
> > 
> > > On 1/22/21 12:59 PM, Patrick Palka wrote:
> > > > On Fri, 22 Jan 2021, Patrick Palka wrote:
> > > > 
> > > > > On Fri, 22 Jan 2021, Patrick Palka wrote:
> > > > > 
> > > > > > On Thu, 21 Jan 2021, Jason Merrill wrote:
> > > > > > 
> > > > > > > On 1/21/21 11:22 AM, Patrick Palka wrote:
> > > > > > > > Here at parse time finish_qualified_id_expr adds an implicit
> > > > > > > > 'this->' to
> > > > > > > > the expression tmp::integral<T> (because it's type-dependent,
> > > > > > > > and
> > > > > > > > also
> > > > > > > > current_class_ptr is set) within the trailing return type, and
> > > > > > > > later
> > > > > > > > during substitution we can't resolve the 'this' since
> > > > > > > > tsubst_function_type does inject_this_parm only for non-static
> > > > > > > > member
> > > > > > > > functions which tmp::func is not.
> > > > > > > > 
> > > > > > > > It seems the root of the problem is that we set
> > > > > > > > current_class_ptr
> > > > > > > > when
> > > > > > > > parsing the signature of a static member function.  Since
> > > > > > > > explicit
> > > > > > > > uses
> > > > > > > > of 'this' are already not allowed in this context, we probably
> > > > > > > > shouldn't
> > > > > > > > be doing inject_this_parm either.
> > > > > > > 
> > > > > > > Hmm, 'this' is defined in a static member function, it's just
> > > > > > > ill-formed to
> > > > > > > use it:
> > > > > > > 
> > > > > > > 7.5.2/2: "... [this] shall not appear within the declaration of a
> > > > > > > static
> > > > > > > member function (although its type and value category are defined
> > > > > > > within a
> > > > > > > static member function as they are within a non-static member
> > > > > > > function).
> > > > > > > [Note: This is because declaration matching does not occur until
> > > > > > > the
> > > > > > > complete
> > > > > > > declarator is known. — end note]"
> > > > > > 
> > > > > > Ah, I see.
> > > > > > 
> > > > > > > 
> > > > > > > Perhaps maybe_dummy_object needs to be smarter about recognizing
> > > > > > > static
> > > > > > > context.  Or perhaps there's no way to tell the difference between
> > > > > > > the
> > > > > > > specified behavior above and the behavior with your patch, but:
> > > > > > > 
> > > > > > > The note suggests that we need to test the case of an out-of-class
> > > > > > > definition
> > > > > > > of a static member function (template); we must inject_this_parm
> > > > > > > when
> > > > > > > parsing
> > > > > > > such a declaration, since we don't know it's static until we match
> > > > > > > it
> > > > > > > to the
> > > > > > > declaration in the class.  I'm guessing that this would lead to
> > > > > > > the
> > > > > > > same
> > > > > > > problem.
> > > > > > 
> > > > > > Good point, we do get a signature mismatch error in this case, due
> > > > > > to
> > > > > > 'this->' appearing in the trailing return type out-of-class
> > > > > > declaration
> > > > > > but not in that of the in-class declaration, so the trailing return
> > > > > > types don't match up.  In light of this case, it doesn't seem
> > > > > > possible
> > > > > > for maybe_dummy_object to distinguish between a static and
> > > > > > non-static
> > > > > > context when parsing the trailing return type of the declaration.
> > > > > > 
> > > > > > So perhaps we should mirror at substitution time what we do at parse
> > > > > > time, and inject 'this' also when substituting into the function
> > > > > > type of
> > > > > > a static member function?  That way, we'll resolve the use of
> > > > > > 'this->'
> > > > > > and then discard it the RHS of -> is a static member function, or
> > > > > > complain that 'this' is not a constant expression if the RHS is a
> > > > > > non-static member function.
> > > > > > 
> > > > > > The following patch performs that, and seems to pass light testing.
> > > > > > Full testing in progress.  Revised commit message is still a WIP.
> > > > > > How does this approach look?
> > > > 
> > > > Sorry for the spam, but I'd also like to propose a more conservative and
> > > > targeted approach that basically undoes part of the r9-5972 patch which
> > > > caused the regression.
> > > > 
> > > > According to the email thread at
> > > > https://gcc.gnu.org/pipermail/gcc-patches/2019-February/516421.html
> > > > the hunk from r9-5972
> > > > 
> > > > --- a/gcc/cp/semantics.c
> > > > +++ b/gcc/cp/semantics.c
> > > > @@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class,
> > > >        {
> > > >          /* See if any of the functions are non-static members.  */
> > > >          /* If so, the expression may be relative to 'this'.  */
> > > > -      if (!shared_member_p (expr)
> > > > +      if ((type_dependent_expression_p (expr)
> > > > +          || !shared_member_p (expr))
> > > >             && current_class_ptr
> > > >             && DERIVED_FROM_P (qualifying_class,
> > > >                                current_nonlambda_class_type ()))
> > > > 
> > > > was added to avoid calling shared_member_p here when the overload set
> > > > contains a dependent USING_DECL.  But according to
> > > > https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513237.html if
> > > > an overload set contains a dependent USING_DECL, then it'll be the first
> > > > in the overload set.  So we could partially revert the r9-5972 patch and
> > > > do:
> > > > 
> > > > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> > > > index c6b4c70dc0f..6b0d68ae4bf 100644
> > > > --- a/gcc/cp/semantics.c
> > > > +++ b/gcc/cp/semantics.c
> > > > @@ -2214,7 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
> > > >        {
> > > >          /* See if any of the functions are non-static members.  */
> > > >          /* If so, the expression may be relative to 'this'.  */
> > > > -      if ((type_dependent_expression_p (expr)
> > > > +      if ((TREE_CODE (get_first_fn (expr)) == USING_DECL
> > > >              || !shared_member_p (expr))
> > > >             && current_class_ptr
> > > >             && DERIVED_FROM_P (qualifying_class,
> > > > 
> > > > The above pases 'make check-c++', and allows us to compile pr97399a.C
> > > > successfully, but we'd still ICE on the invalid pr97399b.C.
> > > 
> > > What if we flipped the sense of the type_dependent_expression_p check, so
> > > we
> > > never add this-> if the function operand is dependent?
> > 
> > Looks like this doesn't survive 'make check-c++', we crash on the
> > testcase g++.dg/cpp1y/lambda-generic-this1a.C:
> > 
> > gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C: In instantiation of
> > ‘MyType::crash()::<lambda(auto:1)> [with auto:1 = A]’:
> > gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:12:10:   required from
> > here
> > gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:9:28: internal compiler
> > error: trying to capture ‘this’ in instantiation of generic lambda
> >      9 |       MyType::make_crash<i>(); // Line (1)
> >        |       ~~~~~~~~~~~~~~~~~~~~~^~
> > 0x9bab23 add_capture(tree_node*, tree_node*, tree_node*, bool, bool)
> >          /home/patrick/gcc-master/gcc/cp/lambda.c:633
> > 0x9bac75 add_default_capture(tree_node*, tree_node*, tree_node*)
> >          /home/patrick/gcc-master/gcc/cp/lambda.c:693
> > 0x9bb3a1 lambda_expr_this_capture(tree_node*, int)
> >          /home/patrick/gcc-master/gcc/cp/lambda.c:811
> > 0x9bb4a0 maybe_resolve_dummy(tree_node*, bool)
> >          /home/patrick/gcc-master/gcc/cp/lambda.c:894
> > 0x8de510 build_new_method_call_1
> >          /home/patrick/gcc-master/gcc/cp/call.c:10503
> > 0x8df04f build_new_method_call(tree_node*, tree_node*, vec<tree_node*,
> > va_gc, vl_embed>**, tree_node*, int, tree_node**, int)
> > 
> > Presumably because we no longer add 'this->' to the dependent call
> > MyType::make_crash<i> inside the generic lambda, so we don't capture it
> > when we should have?
> 
> Ah, yes, we still need to add this-> in a generic lambda, or at least capture
> 'this'.
> 
> I think I was wrong in my assertion around Alex's patch that shared_member_p
> should abort on a dependent USING_DECL; it now seems appropriate for it to
> return false if we don't know, we just need to adjust the comment to say that.
> 
> And then drop the type-dependent check.

Sounds good, thanks a lot.  Here's what I'm testing.

Note that with this approach we still ICE on the invalid testcase
pr97399b.C (starting with r8-2724, it seems).  I'll create a separate PR
to track this issue.

-- >8 --

Subject: [PATCH] c++: 'this' injection and static member functions [PR97399]

In the testcase pr97399.C below, finish_qualified_id_expr at parse time
adds an implicit 'this->' to the expression tmp::integral<T> (because
it's type-dependent, and also current_class_ptr is set at this point)
within the trailing return type.  Later when substituting into the
trailing return type we crash due to the unresolved 'this', since
tsubst_function_type does inject_this_parm only for non-static member
functions, which tmp::func is not.

This patch fixes this issue by removing the type-dependence check
in finish_qualified_id_expr added by r9-5972, and in its place relaxing
shared_member_p to handle dependent USING_DECLs:

> I think I was wrong in my assertion around Alex's patch that
> shared_member_p should abort on a dependent USING_DECL; it now seems
> appropriate for it to return false if we don't know, we just need to
> adjust the comment to say that.

Moreover, for friend functions, we shouldn't be setting
current_class_ptr at all when parsing its signature, so this patch
suppresses inject_this_parm for friends.

Finally, the change to cp_parser_init_declarator below is needed so that
we properly communicate static-ness to cp_parser_direct_declarator when
parsing a member function template.  This lets us reject the explicit
use of 'this' in the testcase this2.C below.

Bootstrap and regtest re-running on x86_64-pc-linux-gnu, does this look
OK for trunk if successful?

gcc/cp/ChangeLog:

	PR c++/97399
	* cp-tree.h (shared_member_p): Adjust declaration.
	* parser.c (cp_parser_init_declarator): If the storage class
	specifier is sc_static, pass true for static_p to
	cp_parser_declarator.
	(cp_parser_direct_declarator): Don't do inject_this_parm when
	the function is specified as a friend.
	* search.c (shared_member_p): Change return type to bool and
	adjust return statements accordingly.  Return false for a
	dependent USING_DECL.
	* pt.c (tsubst_function_type): Also inject 'this' when
	substituting into the function type of a static member
	function.

gcc/testsuite/ChangeLog:

	PR c++/88548
	PR c++/97399
	* g++.dg/cpp0x/this2.C: New test.
	* g++.dg/template/pr97399.C: New test.
---
 gcc/cp/cp-tree.h                        |  2 +-
 gcc/cp/parser.c                         |  5 +++--
 gcc/cp/search.c                         | 20 +++++++++++---------
 gcc/cp/semantics.c                      |  3 +--
 gcc/testsuite/g++.dg/cpp0x/this2.C      |  8 ++++++++
 gcc/testsuite/g++.dg/template/pr97399.C | 23 +++++++++++++++++++++++
 6 files changed, 47 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/this2.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr97399.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index eb20de433b7..65b4cc7f5c8 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7312,7 +7312,7 @@ extern tree adjust_result_of_qualified_name_lookup
 						(tree, tree, tree);
 extern tree copied_binfo			(tree, tree);
 extern tree original_binfo			(tree, tree);
-extern int shared_member_p			(tree);
+extern bool shared_member_p			(tree);
 extern bool any_dependent_bases_p (tree = current_nonlambda_class_type ());
 extern bool maybe_check_overriding_exception_spec (tree, tree);
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 48437f23175..3a78ea49634 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -21413,6 +21413,7 @@ cp_parser_init_declarator (cp_parser* parser,
   bool is_non_constant_init;
   int ctor_dtor_or_conv_p;
   bool friend_p = cp_parser_friend_p (decl_specifiers);
+  bool static_p = decl_specifiers->storage_class == sc_static;
   tree pushed_scope = NULL_TREE;
   bool range_for_decl_p = false;
   bool saved_default_arg_ok_p = parser->default_arg_ok_p;
@@ -21446,7 +21447,7 @@ cp_parser_init_declarator (cp_parser* parser,
     = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
 			    flags, &ctor_dtor_or_conv_p,
 			    /*parenthesized_p=*/NULL,
-			    member_p, friend_p, /*static_p=*/false);
+			    member_p, friend_p, static_p);
   /* Gather up the deferred checks.  */
   stop_deferring_access_checks ();
 
@@ -22122,7 +22123,7 @@ cp_parser_direct_declarator (cp_parser* parser,
 
 		  tree save_ccp = current_class_ptr;
 		  tree save_ccr = current_class_ref;
-		  if (memfn)
+		  if (memfn && !friend_p)
 		    /* DR 1207: 'this' is in scope after the cv-quals.  */
 		    inject_this_parameter (current_class_type, cv_quals);
 
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index dd3773da4f7..9c60c80da44 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -910,7 +910,7 @@ struct lookup_field_info {
   const char *errstr;
 };
 
-/* Nonzero for a class member means that it is shared between all objects
+/* True for a class member means that it is shared between all objects
    of that class.
 
    [class.member.lookup]:If the resulting set of declarations are not all
@@ -920,25 +920,27 @@ struct lookup_field_info {
 
    This function checks that T contains no non-static members.  */
 
-int
+bool
 shared_member_p (tree t)
 {
-  if (VAR_P (t) || TREE_CODE (t) == TYPE_DECL \
+  if (VAR_P (t) || TREE_CODE (t) == TYPE_DECL
       || TREE_CODE (t) == CONST_DECL)
-    return 1;
+    return true;
   if (is_overloaded_fn (t))
     {
       for (ovl_iterator iter (get_fns (t)); iter; ++iter)
 	{
 	  tree decl = strip_using_decl (*iter);
-	  /* We don't expect or support dependent decls.  */
-	  gcc_assert (TREE_CODE (decl) != USING_DECL);
+	  if (TREE_CODE (decl) == USING_DECL)
+	    /* We don't know what this dependent using declaration
+	       will resolve to; conservatively return false.  */
+	    return false;
 	  if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
-	    return 0;
+	    return false;
 	}
-      return 1;
+      return true;
     }
-  return 0;
+  return false;
 }
 
 /* Routine to see if the sub-object denoted by the binfo PARENT can be
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 067095276af..51841dcf24a 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2214,8 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
     {
       /* See if any of the functions are non-static members.  */
       /* If so, the expression may be relative to 'this'.  */
-      if ((type_dependent_expression_p (expr)
-	   || !shared_member_p (expr))
+      if (!shared_member_p (expr)
 	  && current_class_ptr
 	  && DERIVED_FROM_P (qualifying_class,
 			     current_nonlambda_class_type ()))
diff --git a/gcc/testsuite/g++.dg/cpp0x/this2.C b/gcc/testsuite/g++.dg/cpp0x/this2.C
new file mode 100644
index 00000000000..3781bc5efec
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/this2.C
@@ -0,0 +1,8 @@
+// PR c++/88548
+// { dg-do compile { target c++11 } }
+
+struct S {
+  int a;
+  template <class> static auto m1 ()
+    -> decltype(this->a) { return 0; }; // { dg-error "'this'" }
+};
diff --git a/gcc/testsuite/g++.dg/template/pr97399.C b/gcc/testsuite/g++.dg/template/pr97399.C
new file mode 100644
index 00000000000..4bb818908fd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr97399.C
@@ -0,0 +1,23 @@
+// PR c++/97399
+// { dg-do compile { target c++11 } }
+
+template <bool> struct enable_if_t {};
+
+struct tmp {
+  template <class> static constexpr bool is_integral();
+  template <class T> static auto f()
+    -> enable_if_t<tmp::is_integral<T>()>;
+  template <class T> friend auto g(tmp, T)
+    -> enable_if_t<!tmp::is_integral<T>()>;
+};
+
+template <class> constexpr bool tmp::is_integral() { return true; }
+
+template <class T> auto tmp::f()
+  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
+
+int main()
+{
+  tmp::f<int>();
+  g(tmp{}, 0);
+}
-- 
2.30.0.155.g66e871b664


> 
> > To be clear, this is with
> > 
> > --- a/gcc/cp/semantics.c
> > +++ b/gcc/cp/semantics.c
> > @@ -2214,8 +2214,8 @@ finish_qualified_id_expr (tree qualifying_class,
> >       {
> >         /* See if any of the functions are non-static members.  */
> >         /* If so, the expression may be relative to 'this'.  */
> > -      if ((type_dependent_expression_p (expr)
> > -          || !shared_member_p (expr))
> > +      if (!type_dependent_expression_p (expr)
> > +         && !shared_member_p (expr)
> >            && current_class_ptr
> >            && DERIVED_FROM_P (qualifying_class,
> >                               current_nonlambda_class_type ()))
> > 
> > > 
> > > > > > -- >8 --
> > > > > > 
> > > > > > Subject: [PATCH] c++: 'this' injection and static member functions
> > > > > > [PR97399]
> > > > > > 
> > > > > > gcc/cp/ChangeLog:
> > > > > > 
> > > > > > 	PR c++/97399
> > > > > > 	* parser.c (cp_parser_init_declarator): If the storage class
> > > > > > 	specifier is sc_static, pass true for static_p to
> > > > > > 	cp_parser_declarator.
> > > 
> > > I don't see this change in the patch; it seems like a good cleanup even if
> > > it
> > > isn't needed for this bug.
> > 
> > Sounds good.  I'll add it back in (and the testcase cpp0x/this2.C) in the
> > next
> > iteration of the patch.
> > 
> > > 
> > > > > > 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> > > > > > 	the function is specified as a friend.
> > > > > > 	* pt.c (tsubst_function_type): Also inject 'this' when
> > > > > > 	substituting into the function type of a static member
> > > > > > 	function.
> > > > > > 
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > 
> > > > > > 	PR c++/88548
> > > > > > 	PR c++/97399
> > > > > > 	* g++.dg/cpp0x/this2.C: New test.
> > > > > > 	* g++.dg/gomp/this-1.C: Adjust expected error for use of
> > > > > > 'this'
> > > > > > 	in signature of static member function.
> > > > > > 	* g++.dg/template/pr97399a.C: New test.
> > > > > > 	* g++.dg/template/pr97399b.C: New test.
> > > > > > ---
> > > > > >    gcc/cp/parser.c                          |  2 +-
> > > > > >    gcc/cp/pt.c                              | 13 +++++++++++--
> > > > > >    gcc/testsuite/g++.dg/template/pr97399a.C | 23
> > > > > > +++++++++++++++++++++++
> > > > > >    gcc/testsuite/g++.dg/template/pr97399b.C | 23
> > > > > > +++++++++++++++++++++++
> > > > > >    4 files changed, 58 insertions(+), 3 deletions(-)
> > > > > >    create mode 100644 gcc/testsuite/g++.dg/template/pr97399a.C
> > > > > >    create mode 100644 gcc/testsuite/g++.dg/template/pr97399b.C
> > > > > > 
> > > > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > > > > > index 48437f23175..a0efa55d21e 100644
> > > > > > --- a/gcc/cp/parser.c
> > > > > > +++ b/gcc/cp/parser.c
> > > > > > @@ -22122,7 +22122,7 @@ cp_parser_direct_declarator (cp_parser*
> > > > > > parser,
> > > > > >      		  tree save_ccp = current_class_ptr;
> > > > > >    		  tree save_ccr = current_class_ref;
> > > > > > -		  if (memfn)
> > > > > > +		  if (memfn && !friend_p)
> > > > > >    		    /* DR 1207: 'this' is in scope after the cv-quals.
> > > > > > */
> > > > > >    		    inject_this_parameter (current_class_type,
> > > > > > cv_quals);
> > > > > >    diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > > > index ad855dbde72..d4763325e25 100644
> > > > > > --- a/gcc/cp/pt.c
> > > > > > +++ b/gcc/cp/pt.c
> > > > > > @@ -15072,8 +15072,17 @@ tsubst_function_type (tree t,
> > > > > >            tree save_ccp = current_class_ptr;
> > > > > >          tree save_ccr = current_class_ref;
> > > > > > -      tree this_type = (TREE_CODE (t) == METHOD_TYPE
> > > > > > -			? TREE_TYPE (TREE_VALUE (arg_types)) :
> > > > > > NULL_TREE);
> > > > > > +      tree this_type = NULL_TREE;
> > > > > > +      if (TREE_CODE (t) == METHOD_TYPE)
> > > > > > +	this_type = TREE_TYPE (TREE_VALUE (arg_types));
> > > > > > +      else if (in_decl
> > > > > > +	       && DECL_FUNCTION_MEMBER_P (in_decl)
> > > > > 
> > > > > Oops, this line should instead be checking DECL_STATIC_FUNCTION_P.
> > > > > Consider it changed.
> > > > > 
> > > > > > +	       && t == TREE_TYPE (in_decl))
> > > > > > +	/* Also inject 'this' when substituting into the function type
> > > > > > +	   of a static member function, as we may have conservatively
> > > > > > +	   added 'this->' to a dependent member function call in its
> > > > > > +	   trailing return type which we might need to resolve.  */
> > > > > > +	this_type = DECL_CONTEXT (in_decl);
> > > > > >          bool do_inject = this_type && CLASS_TYPE_P (this_type);
> > > > > >          if (do_inject)
> > > > > >    	{
> > > > > > diff --git a/gcc/testsuite/g++.dg/template/pr97399a.C
> > > > > > b/gcc/testsuite/g++.dg/template/pr97399a.C
> > > > > > new file mode 100644
> > > > > > index 00000000000..4bb818908fd
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/g++.dg/template/pr97399a.C
> > > > > > @@ -0,0 +1,23 @@
> > > > > > +// PR c++/97399
> > > > > > +// { dg-do compile { target c++11 } }
> > > > > > +
> > > > > > +template <bool> struct enable_if_t {};
> > > > > > +
> > > > > > +struct tmp {
> > > > > > +  template <class> static constexpr bool is_integral();
> > > > > > +  template <class T> static auto f()
> > > > > > +    -> enable_if_t<tmp::is_integral<T>()>;
> > > > > > +  template <class T> friend auto g(tmp, T)
> > > > > > +    -> enable_if_t<!tmp::is_integral<T>()>;
> > > > > > +};
> > > > > > +
> > > > > > +template <class> constexpr bool tmp::is_integral() { return true; }
> > > > > > +
> > > > > > +template <class T> auto tmp::f()
> > > > > > +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> > > > > > +
> > > > > > +int main()
> > > > > > +{
> > > > > > +  tmp::f<int>();
> > > > > > +  g(tmp{}, 0);
> > > > > > +}
> > > > > > diff --git a/gcc/testsuite/g++.dg/template/pr97399b.C
> > > > > > b/gcc/testsuite/g++.dg/template/pr97399b.C
> > > > > > new file mode 100644
> > > > > > index 00000000000..673ba6624e3
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/g++.dg/template/pr97399b.C
> > > > > > @@ -0,0 +1,23 @@
> > > > > > +// PR c++/97399
> > > > > > +// { dg-do compile { target c++11 } }
> > > > > > +
> > > > > > +template <bool> struct enable_if_t {};
> > > > > > +
> > > > > > +struct tmp {
> > > > > > +  template <class> constexpr bool is_integral(); // non-static
> > > > > > +  template <class T> static auto f()
> > > > > > +    -> enable_if_t<tmp::is_integral<T>()>; // { dg-message "in
> > > > > > template
> > > > > > argument" }
> > > > > > +  template <class T> friend auto g(tmp, T)
> > > > > > +    -> enable_if_t<!tmp::is_integral<T>()>; // { dg-error "without
> > > > > > object" }
> > > > > > +};
> > > > > > +
> > > > > > +template <class> constexpr bool tmp::is_integral() { return true; }
> > > > > > +
> > > > > > +template <class T> auto tmp::f() // { dg-error "'this' is not a
> > > > > > constant" }
> > > > > > +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> > > > > > +
> > > > > > +int main()
> > > > > > +{
> > > > > > +  tmp::f<int>(); // { dg-error "no match" }
> > > > > > +  g(tmp{}, 0); // { dg-error "no match" }
> > > > > > +}
> > > > > > -- 
> > > > > > 2.30.0.155.g66e871b664
> > > 
> 
> 

Re: [PATCH] c++: Suppress this injection for static member functions [PR97399]

[Jason Merrill]

On 1/22/21 4:45 PM, Patrick Palka wrote:
> On Fri, 22 Jan 2021, Jason Merrill wrote:
> 
>> On 1/22/21 1:58 PM, Patrick Palka wrote:
>>> On Fri, 22 Jan 2021, Jason Merrill wrote:
>>>
>>>> On 1/22/21 12:59 PM, Patrick Palka wrote:
>>>>> On Fri, 22 Jan 2021, Patrick Palka wrote:
>>>>>
>>>>>> On Fri, 22 Jan 2021, Patrick Palka wrote:
>>>>>>
>>>>>>> On Thu, 21 Jan 2021, Jason Merrill wrote:
>>>>>>>
>>>>>>>> On 1/21/21 11:22 AM, Patrick Palka wrote:
>>>>>>>>> Here at parse time finish_qualified_id_expr adds an implicit
>>>>>>>>> 'this->' to
>>>>>>>>> the expression tmp::integral<T> (because it's type-dependent,
>>>>>>>>> and
>>>>>>>>> also
>>>>>>>>> current_class_ptr is set) within the trailing return type, and
>>>>>>>>> later
>>>>>>>>> during substitution we can't resolve the 'this' since
>>>>>>>>> tsubst_function_type does inject_this_parm only for non-static
>>>>>>>>> member
>>>>>>>>> functions which tmp::func is not.
>>>>>>>>>
>>>>>>>>> It seems the root of the problem is that we set
>>>>>>>>> current_class_ptr
>>>>>>>>> when
>>>>>>>>> parsing the signature of a static member function.  Since
>>>>>>>>> explicit
>>>>>>>>> uses
>>>>>>>>> of 'this' are already not allowed in this context, we probably
>>>>>>>>> shouldn't
>>>>>>>>> be doing inject_this_parm either.
>>>>>>>>
>>>>>>>> Hmm, 'this' is defined in a static member function, it's just
>>>>>>>> ill-formed to
>>>>>>>> use it:
>>>>>>>>
>>>>>>>> 7.5.2/2: "... [this] shall not appear within the declaration of a
>>>>>>>> static
>>>>>>>> member function (although its type and value category are defined
>>>>>>>> within a
>>>>>>>> static member function as they are within a non-static member
>>>>>>>> function).
>>>>>>>> [Note: This is because declaration matching does not occur until
>>>>>>>> the
>>>>>>>> complete
>>>>>>>> declarator is known. — end note]"
>>>>>>>
>>>>>>> Ah, I see.
>>>>>>>
>>>>>>>>
>>>>>>>> Perhaps maybe_dummy_object needs to be smarter about recognizing
>>>>>>>> static
>>>>>>>> context.  Or perhaps there's no way to tell the difference between
>>>>>>>> the
>>>>>>>> specified behavior above and the behavior with your patch, but:
>>>>>>>>
>>>>>>>> The note suggests that we need to test the case of an out-of-class
>>>>>>>> definition
>>>>>>>> of a static member function (template); we must inject_this_parm
>>>>>>>> when
>>>>>>>> parsing
>>>>>>>> such a declaration, since we don't know it's static until we match
>>>>>>>> it
>>>>>>>> to the
>>>>>>>> declaration in the class.  I'm guessing that this would lead to
>>>>>>>> the
>>>>>>>> same
>>>>>>>> problem.
>>>>>>>
>>>>>>> Good point, we do get a signature mismatch error in this case, due
>>>>>>> to
>>>>>>> 'this->' appearing in the trailing return type out-of-class
>>>>>>> declaration
>>>>>>> but not in that of the in-class declaration, so the trailing return
>>>>>>> types don't match up.  In light of this case, it doesn't seem
>>>>>>> possible
>>>>>>> for maybe_dummy_object to distinguish between a static and
>>>>>>> non-static
>>>>>>> context when parsing the trailing return type of the declaration.
>>>>>>>
>>>>>>> So perhaps we should mirror at substitution time what we do at parse
>>>>>>> time, and inject 'this' also when substituting into the function
>>>>>>> type of
>>>>>>> a static member function?  That way, we'll resolve the use of
>>>>>>> 'this->'
>>>>>>> and then discard it the RHS of -> is a static member function, or
>>>>>>> complain that 'this' is not a constant expression if the RHS is a
>>>>>>> non-static member function.
>>>>>>>
>>>>>>> The following patch performs that, and seems to pass light testing.
>>>>>>> Full testing in progress.  Revised commit message is still a WIP.
>>>>>>> How does this approach look?
>>>>>
>>>>> Sorry for the spam, but I'd also like to propose a more conservative and
>>>>> targeted approach that basically undoes part of the r9-5972 patch which
>>>>> caused the regression.
>>>>>
>>>>> According to the email thread at
>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2019-February/516421.html
>>>>> the hunk from r9-5972
>>>>>
>>>>> --- a/gcc/cp/semantics.c
>>>>> +++ b/gcc/cp/semantics.c
>>>>> @@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class,
>>>>>         {
>>>>>           /* See if any of the functions are non-static members.  */
>>>>>           /* If so, the expression may be relative to 'this'.  */
>>>>> -      if (!shared_member_p (expr)
>>>>> +      if ((type_dependent_expression_p (expr)
>>>>> +          || !shared_member_p (expr))
>>>>>              && current_class_ptr
>>>>>              && DERIVED_FROM_P (qualifying_class,
>>>>>                                 current_nonlambda_class_type ()))
>>>>>
>>>>> was added to avoid calling shared_member_p here when the overload set
>>>>> contains a dependent USING_DECL.  But according to
>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2018-December/513237.html if
>>>>> an overload set contains a dependent USING_DECL, then it'll be the first
>>>>> in the overload set.  So we could partially revert the r9-5972 patch and
>>>>> do:
>>>>>
>>>>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
>>>>> index c6b4c70dc0f..6b0d68ae4bf 100644
>>>>> --- a/gcc/cp/semantics.c
>>>>> +++ b/gcc/cp/semantics.c
>>>>> @@ -2214,7 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
>>>>>         {
>>>>>           /* See if any of the functions are non-static members.  */
>>>>>           /* If so, the expression may be relative to 'this'.  */
>>>>> -      if ((type_dependent_expression_p (expr)
>>>>> +      if ((TREE_CODE (get_first_fn (expr)) == USING_DECL
>>>>>               || !shared_member_p (expr))
>>>>>              && current_class_ptr
>>>>>              && DERIVED_FROM_P (qualifying_class,
>>>>>
>>>>> The above pases 'make check-c++', and allows us to compile pr97399a.C
>>>>> successfully, but we'd still ICE on the invalid pr97399b.C.
>>>>
>>>> What if we flipped the sense of the type_dependent_expression_p check, so
>>>> we
>>>> never add this-> if the function operand is dependent?
>>>
>>> Looks like this doesn't survive 'make check-c++', we crash on the
>>> testcase g++.dg/cpp1y/lambda-generic-this1a.C:
>>>
>>> gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C: In instantiation of
>>> ‘MyType::crash()::<lambda(auto:1)> [with auto:1 = A]’:
>>> gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:12:10:   required from
>>> here
>>> gcc/testsuite/g++.dg/cpp1y/lambda-generic-this1a.C:9:28: internal compiler
>>> error: trying to capture ‘this’ in instantiation of generic lambda
>>>       9 |       MyType::make_crash<i>(); // Line (1)
>>>         |       ~~~~~~~~~~~~~~~~~~~~~^~
>>> 0x9bab23 add_capture(tree_node*, tree_node*, tree_node*, bool, bool)
>>>           /home/patrick/gcc-master/gcc/cp/lambda.c:633
>>> 0x9bac75 add_default_capture(tree_node*, tree_node*, tree_node*)
>>>           /home/patrick/gcc-master/gcc/cp/lambda.c:693
>>> 0x9bb3a1 lambda_expr_this_capture(tree_node*, int)
>>>           /home/patrick/gcc-master/gcc/cp/lambda.c:811
>>> 0x9bb4a0 maybe_resolve_dummy(tree_node*, bool)
>>>           /home/patrick/gcc-master/gcc/cp/lambda.c:894
>>> 0x8de510 build_new_method_call_1
>>>           /home/patrick/gcc-master/gcc/cp/call.c:10503
>>> 0x8df04f build_new_method_call(tree_node*, tree_node*, vec<tree_node*,
>>> va_gc, vl_embed>**, tree_node*, int, tree_node**, int)
>>>
>>> Presumably because we no longer add 'this->' to the dependent call
>>> MyType::make_crash<i> inside the generic lambda, so we don't capture it
>>> when we should have?
>>
>> Ah, yes, we still need to add this-> in a generic lambda, or at least capture
>> 'this'.
>>
>> I think I was wrong in my assertion around Alex's patch that shared_member_p
>> should abort on a dependent USING_DECL; it now seems appropriate for it to
>> return false if we don't know, we just need to adjust the comment to say that.
>>
>> And then drop the type-dependent check.
> 
> Sounds good, thanks a lot.  Here's what I'm testing.
> 
> Note that with this approach we still ICE on the invalid testcase
> pr97399b.C (starting with r8-2724, it seems).  I'll create a separate PR
> to track this issue.

OK, thanks.

> -- >8 --
> 
> Subject: [PATCH] c++: 'this' injection and static member functions [PR97399]
> 
> In the testcase pr97399.C below, finish_qualified_id_expr at parse time
> adds an implicit 'this->' to the expression tmp::integral<T> (because
> it's type-dependent, and also current_class_ptr is set at this point)
> within the trailing return type.  Later when substituting into the
> trailing return type we crash due to the unresolved 'this', since
> tsubst_function_type does inject_this_parm only for non-static member
> functions, which tmp::func is not.
> 
> This patch fixes this issue by removing the type-dependence check
> in finish_qualified_id_expr added by r9-5972, and in its place relaxing
> shared_member_p to handle dependent USING_DECLs:
> 
>> I think I was wrong in my assertion around Alex's patch that
>> shared_member_p should abort on a dependent USING_DECL; it now seems
>> appropriate for it to return false if we don't know, we just need to
>> adjust the comment to say that.
> 
> Moreover, for friend functions, we shouldn't be setting
> current_class_ptr at all when parsing its signature, so this patch
> suppresses inject_this_parm for friends.
> 
> Finally, the change to cp_parser_init_declarator below is needed so that
> we properly communicate static-ness to cp_parser_direct_declarator when
> parsing a member function template.  This lets us reject the explicit
> use of 'this' in the testcase this2.C below.
> 
> Bootstrap and regtest re-running on x86_64-pc-linux-gnu, does this look
> OK for trunk if successful?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/97399
> 	* cp-tree.h (shared_member_p): Adjust declaration.
> 	* parser.c (cp_parser_init_declarator): If the storage class
> 	specifier is sc_static, pass true for static_p to
> 	cp_parser_declarator.
> 	(cp_parser_direct_declarator): Don't do inject_this_parm when
> 	the function is specified as a friend.
> 	* search.c (shared_member_p): Change return type to bool and
> 	adjust return statements accordingly.  Return false for a
> 	dependent USING_DECL.
> 	* pt.c (tsubst_function_type): Also inject 'this' when
> 	substituting into the function type of a static member
> 	function.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/88548
> 	PR c++/97399
> 	* g++.dg/cpp0x/this2.C: New test.
> 	* g++.dg/template/pr97399.C: New test.
> ---
>   gcc/cp/cp-tree.h                        |  2 +-
>   gcc/cp/parser.c                         |  5 +++--
>   gcc/cp/search.c                         | 20 +++++++++++---------
>   gcc/cp/semantics.c                      |  3 +--
>   gcc/testsuite/g++.dg/cpp0x/this2.C      |  8 ++++++++
>   gcc/testsuite/g++.dg/template/pr97399.C | 23 +++++++++++++++++++++++
>   6 files changed, 47 insertions(+), 14 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/this2.C
>   create mode 100644 gcc/testsuite/g++.dg/template/pr97399.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index eb20de433b7..65b4cc7f5c8 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7312,7 +7312,7 @@ extern tree adjust_result_of_qualified_name_lookup
>   						(tree, tree, tree);
>   extern tree copied_binfo			(tree, tree);
>   extern tree original_binfo			(tree, tree);
> -extern int shared_member_p			(tree);
> +extern bool shared_member_p			(tree);
>   extern bool any_dependent_bases_p (tree = current_nonlambda_class_type ());
>   extern bool maybe_check_overriding_exception_spec (tree, tree);
>   
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 48437f23175..3a78ea49634 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -21413,6 +21413,7 @@ cp_parser_init_declarator (cp_parser* parser,
>     bool is_non_constant_init;
>     int ctor_dtor_or_conv_p;
>     bool friend_p = cp_parser_friend_p (decl_specifiers);
> +  bool static_p = decl_specifiers->storage_class == sc_static;
>     tree pushed_scope = NULL_TREE;
>     bool range_for_decl_p = false;
>     bool saved_default_arg_ok_p = parser->default_arg_ok_p;
> @@ -21446,7 +21447,7 @@ cp_parser_init_declarator (cp_parser* parser,
>       = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
>   			    flags, &ctor_dtor_or_conv_p,
>   			    /*parenthesized_p=*/NULL,
> -			    member_p, friend_p, /*static_p=*/false);
> +			    member_p, friend_p, static_p);
>     /* Gather up the deferred checks.  */
>     stop_deferring_access_checks ();
>   
> @@ -22122,7 +22123,7 @@ cp_parser_direct_declarator (cp_parser* parser,
>   
>   		  tree save_ccp = current_class_ptr;
>   		  tree save_ccr = current_class_ref;
> -		  if (memfn)
> +		  if (memfn && !friend_p)
>   		    /* DR 1207: 'this' is in scope after the cv-quals.  */
>   		    inject_this_parameter (current_class_type, cv_quals);
>   
> diff --git a/gcc/cp/search.c b/gcc/cp/search.c
> index dd3773da4f7..9c60c80da44 100644
> --- a/gcc/cp/search.c
> +++ b/gcc/cp/search.c
> @@ -910,7 +910,7 @@ struct lookup_field_info {
>     const char *errstr;
>   };
>   
> -/* Nonzero for a class member means that it is shared between all objects
> +/* True for a class member means that it is shared between all objects
>      of that class.
>   
>      [class.member.lookup]:If the resulting set of declarations are not all
> @@ -920,25 +920,27 @@ struct lookup_field_info {
>   
>      This function checks that T contains no non-static members.  */
>   
> -int
> +bool
>   shared_member_p (tree t)
>   {
> -  if (VAR_P (t) || TREE_CODE (t) == TYPE_DECL \
> +  if (VAR_P (t) || TREE_CODE (t) == TYPE_DECL
>         || TREE_CODE (t) == CONST_DECL)
> -    return 1;
> +    return true;
>     if (is_overloaded_fn (t))
>       {
>         for (ovl_iterator iter (get_fns (t)); iter; ++iter)
>   	{
>   	  tree decl = strip_using_decl (*iter);
> -	  /* We don't expect or support dependent decls.  */
> -	  gcc_assert (TREE_CODE (decl) != USING_DECL);
> +	  if (TREE_CODE (decl) == USING_DECL)
> +	    /* We don't know what this dependent using declaration
> +	       will resolve to; conservatively return false.  */
> +	    return false;
>   	  if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
> -	    return 0;
> +	    return false;
>   	}
> -      return 1;
> +      return true;
>       }
> -  return 0;
> +  return false;
>   }
>   
>   /* Routine to see if the sub-object denoted by the binfo PARENT can be
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index 067095276af..51841dcf24a 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -2214,8 +2214,7 @@ finish_qualified_id_expr (tree qualifying_class,
>       {
>         /* See if any of the functions are non-static members.  */
>         /* If so, the expression may be relative to 'this'.  */
> -      if ((type_dependent_expression_p (expr)
> -	   || !shared_member_p (expr))
> +      if (!shared_member_p (expr)
>   	  && current_class_ptr
>   	  && DERIVED_FROM_P (qualifying_class,
>   			     current_nonlambda_class_type ()))
> diff --git a/gcc/testsuite/g++.dg/cpp0x/this2.C b/gcc/testsuite/g++.dg/cpp0x/this2.C
> new file mode 100644
> index 00000000000..3781bc5efec
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/this2.C
> @@ -0,0 +1,8 @@
> +// PR c++/88548
> +// { dg-do compile { target c++11 } }
> +
> +struct S {
> +  int a;
> +  template <class> static auto m1 ()
> +    -> decltype(this->a) { return 0; }; // { dg-error "'this'" }
> +};
> diff --git a/gcc/testsuite/g++.dg/template/pr97399.C b/gcc/testsuite/g++.dg/template/pr97399.C
> new file mode 100644
> index 00000000000..4bb818908fd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/pr97399.C
> @@ -0,0 +1,23 @@
> +// PR c++/97399
> +// { dg-do compile { target c++11 } }
> +
> +template <bool> struct enable_if_t {};
> +
> +struct tmp {
> +  template <class> static constexpr bool is_integral();
> +  template <class T> static auto f()
> +    -> enable_if_t<tmp::is_integral<T>()>;
> +  template <class T> friend auto g(tmp, T)
> +    -> enable_if_t<!tmp::is_integral<T>()>;
> +};
> +
> +template <class> constexpr bool tmp::is_integral() { return true; }
> +
> +template <class T> auto tmp::f()
> +  -> enable_if_t<tmp::is_integral<T>()> { return {}; }
> +
> +int main()
> +{
> +  tmp::f<int>();
> +  g(tmp{}, 0);
> +}
>