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
