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