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
>
