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 + + template <typename> friend class ::C; + + so we just need to return it directly. */ + return TREE_TYPE (friend_tmpl); + tree context = CP_DECL_CONTEXT (friend_tmpl); if (TREE_CODE (context) == NAMESPACE_DECL) push_nested_namespace (context); @@ -11764,23 +11773,19 @@ tsubst_friend_class (tree friend_tmpl, tree args) compatible with the attachment of the friend template. */ module_may_redeclare (tmpl, friend_tmpl); - if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (friend_tmpl)) - > TMPL_ARGS_DEPTH (args)) - { - tree parms = tsubst_template_parms (DECL_TEMPLATE_PARMS (friend_tmpl), - args, tf_warning_or_error); - tsubst_each_template_parm_constraints (parms, args, - tf_warning_or_error); - location_t saved_input_location = input_location; - input_location = DECL_SOURCE_LOCATION (friend_tmpl); - tree cons = get_constraints (friend_tmpl); - ++processing_template_decl; - cons = tsubst_constraint_info (cons, args, tf_warning_or_error, - DECL_FRIEND_CONTEXT (friend_tmpl)); - --processing_template_decl; - redeclare_class_template (TREE_TYPE (tmpl), parms, cons); - input_location = saved_input_location; - } + tree parms = tsubst_template_parms (DECL_TEMPLATE_PARMS (friend_tmpl), + args, tf_warning_or_error); + tsubst_each_template_parm_constraints (parms, args, + tf_warning_or_error); + location_t saved_input_location = input_location; + input_location = DECL_SOURCE_LOCATION (friend_tmpl); + tree cons = get_constraints (friend_tmpl); + ++processing_template_decl; + cons = tsubst_constraint_info (cons, args, tf_warning_or_error, + DECL_FRIEND_CONTEXT (friend_tmpl)); + --processing_template_decl; + redeclare_class_template (TREE_TYPE (tmpl), parms, cons); + input_location = saved_input_location; } else { diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-16_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-16_a.C new file mode 100644 index 00000000000..e1cdcd98e1e --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-16_a.C @@ -0,0 +1,40 @@ +// PR c++/115801 +// { dg-additional-options "-fmodules-ts -Wno-global-module" } +// { dg-module-cmi test } + +module; + +template <typename T> struct GMF; +template <typename T> struct GMF_Hidden { + int go() { GMF<T> gmf; return gmf.x; } +}; + +template <typename T> struct GMF { +private: + template <typename> friend struct ::GMF_Hidden; + int x = 1; +}; + +template <typename T> int test_gmf() { + GMF_Hidden<T> h; return h.go(); +} + +export module test; + +export using ::GMF; +export using ::test_gmf; + +export template <typename> struct Attached; +template <typename T> struct Attached_Hidden { + int go() { Attached<T> attached; return attached.x; } +}; + +template <typename T> struct Attached { +private: + template <typename> friend struct ::Attached_Hidden; + int x = 2; +}; + +export template <typename T> int test_attached() { + Attached_Hidden<T> h; return h.go(); +} diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-16_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-16_b.C new file mode 100644 index 00000000000..d3484ab19b1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-16_b.C @@ -0,0 +1,17 @@ +// PR c++/115801 +// { dg-additional-options "-fmodules-ts" } + +import test; + +int main() { + GMF<int> gmf; + Attached<int> attached; + + int a = test_gmf<double>(); + int b = test_attached<double>(); + + GMF_Hidden<int> gmf_hidden; // { dg-error "not declared" } + Attached_Hidden<int> attached_hidden; // { dg-error "not declared" } +} + +// { dg-prune-output "expected primary-expression" } -- 2.43.2
