On 8/7/24 7:45 PM, Nathaniel Shead wrote:
On Wed, Aug 07, 2024 at 01:44:31PM -0400, Jason Merrill wrote:
On 8/6/24 2:35 AM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
Another potential approach would be to go searching for this unexported
type and load it, either with a new LOOK_want::ANY_REACHABLE flag or by
expanding on the lookup_imported_temploid_friend hack. I'm still not
exactly sure how name lookup for template friends is supposed to behave
though, specifically in terms of when and where they should conflict
with other entities with the same name.
CWG2588 tried to clarify this in https://eel.is/c++draft/basic#link-4.8 --
if there's a matching reachable declaration, the friend refers to it even if
it isn't visible to name lookup.
It seems like an oversight that the new second bullet refers specifically to
functions, it seems natural for it to apply to classes as well.
So, they correspond but do not conflict because they declare the same
entity.
Right, makes sense. OK, I'll work on filling out our testcases to make
sure that we cover everything under that interpretation and potentially
come back to making an ANY_REACHABLE flag for this.
The relevant paragraphs seem to be https://eel.is/c++draft/temp.friend#2
and/or https://eel.is/c++draft/dcl.meaning.general#2.2.2, in addition to
the usual rules in [basic.link] and [basic.scope.scope], but how these
all are supposed to interact isn't super clear to me right now.
Additionally I wonder if maybe the better approach long-term would be to
focus on getting textual redefinitions working first, and then reuse
whatever logic we build for that to handle template friends rather than
relying on finding these hidden 'mergeable' slots first.
I have a WIP patch to allow textual redefinitions by teaching
duplicate_decls that it's OK to redefine an imported GM entity, so
check_module_override works.
My current plan is to then just token-skip the bodies. This won't diagnose
ODR problems, but our module merging doesn't do that consistently either.
@@ -11800,6 +11800,15 @@ tsubst_friend_class (tree friend_tmpl, tree args)
input_location = saved_input_location;
}
}
+ else if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (friend_tmpl))
+ <= TMPL_ARGS_DEPTH (args))
This condition seems impossible normally; it's only satisfied in this
testcase because friend_tmpl doesn't actually represent the friend
declaration, it's already the named class template. So the substitution in
the next else fails because it was done already.
If this condition is true, we could set tmpl = friend_tmpl earlier, and skip
doing name lookup at all.
It's interesting that the previous if does the same depth comparison, and
that dates back to 2002; I wonder why it was needed then?
Jason
Ah right, I see. I think the depth comparison in the previous if
actually is for exactly the same reason, just for the normal case when
the template *is* found by name lookup, e.g.
template <typename> struct A {};
template <typename> struct B {
template <typename> friend struct ::A;
};
This is g++.dg/template/friend5. Here's an updated patch which is so
far very lightly tested, OK for trunk if full bootstrap+regtest
succeeds?
-- >8 --
With modules it may be the case that a template friend class provided
with a qualified name is not found by name lookup at instantiation time,
due to the class not being exported from its module. This causes issues
in tsubst_friend_class which did not handle this case.
This is a more general issue, in fact, caused by the named friend class
not actually requiring tsubsting. This was already worked around for
the "found by name lookup" case (g++.dg/template/friend5.C), but it
looks like there's no need to do name lookup at all for this to work.
PR c++/115801
gcc/cp/ChangeLog:
* pt.cc (tsubst_friend_class): Return the type directly when no
tsubsting is required.
gcc/testsuite/ChangeLog:
* g++.dg/modules/tpl-friend-16_a.C: New test.
* g++.dg/modules/tpl-friend-16_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/pt.cc | 39 ++++++++++--------
.../g++.dg/modules/tpl-friend-16_a.C | 40 +++++++++++++++++++
.../g++.dg/modules/tpl-friend-16_b.C | 17 ++++++++
3 files changed, 79 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-16_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-16_b.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2db59213c54..ea00577fd37 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11732,6 +11732,15 @@ tsubst_friend_class (tree friend_tmpl, tree args)
return TREE_TYPE (tmpl);
}
+ if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (friend_tmpl))
+ <= TMPL_ARGS_DEPTH (args))
+ /* The template has already been subsituted, e.g. for
"substituted"
OK with that typo fix.
Jason
