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

Reply via email to