On Thu, Dec 18, 2025 at 11:03:12AM +0700, Jason Merrill wrote:
> On 12/16/25 6:34 PM, Nathaniel Shead wrote:
> > Here's attempt two at this patch, with a testcase for the linker issue
> > I'd inadvertantly caused with my previous attempt.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > 
> > -- >8 --
> > 
> > Modules allow temploid friends to no longer be implicitly inline, as
> > functions defined in a class body will not be implicitly inline if
> > attached to a named module.
> > 
> > This requires us to clean up linkage handling a little bit, mostly by
> > replacing usages of 'DECL_TEMPLATE_INSTANTIATION' with
> > 'DECL_TEMPLOID_INSTANTIATION' when determining if an entity has vague
> > linkage.
> > 
> > This caused the friend88.C testcase to miscompile however, as 'foo' was
> > incorrectly having 'DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION' getting
> > set because it was keeping its tinfo.
> > 
> > This is because 'non_templated_friend_p' was returning 'false', since
> > the function didn't have a primary template.  But that's expected I
> > think here, so fixed by also returning true for friend declarations
> > pushed into namespace scope, which still allows dependent nested friends
> > to be considered templated.
> 
> Will that wrongly return true for a friend namespace-scope template? The
> test should check that case.

It does not; in cases like

  template <typename T> friend void namespace_scope();

this is a primary template and so primary is non-null anyway, and
'primary != tmpl' returns false.  I'm not sure how to test this though,
as the test I've been using here only works because the affected
functions should not be comdat, but for a function template it should be
comdat anyway.  And this would not be a pseudo template instantiation
regardless, as it's actually a template.

> It seems like a bug that TPARMS_PRIMARY_TEMPLATE would ever be null.

Assuming this is about 'template <class T> friend A<T>::f()', this
appears to be deliberate from this hunk in push_template_decl added
in r0-76180-gc9cbfca6 (PR c++/27714):

  bool is_primary = false;
  if (is_friend && ctx
      && uses_template_parms_level (ctx, current_template_depth))
    /* A friend template that specifies a class context, i.e.
         template <typename T> friend void A<T>::f();
       is not primary.  */
    ;

And indeed removing this causes redeclaration errors.  I'm not sure how
(or indeed, if it would be sensible) to change this.

> >     PR c++/122819
> > 
> > gcc/cp/ChangeLog:
> > 
> >     * decl.cc (start_preparsed_function): Use
> >     DECL_TEMPLOID_INSTANTIATION instead of
> >     DECL_TEMPLATE_INSTANTIATION to check vague linkage.
> >     * decl2.cc (vague_linkage_p): Likewise.
> >     (c_parse_final_cleanups): Simplify condition.
> >     * pt.cc (non_templated_friend_p): Namespace-scope friend
> >     function declarations without a primary template are still
> >     non-templated.
> >     * semantics.cc (expand_or_defer_fn_1): Also check for temploid
> >     friend functions.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> >     * g++.dg/modules/tpl-friend-22.C: New test.
> >     * g++.dg/template/friend88: New test.
> > 
> > Signed-off-by: Nathaniel Shead <[email protected]>
> > ---
> >   gcc/cp/decl.cc                               |  6 ++--
> >   gcc/cp/decl2.cc                              |  5 ++-
> >   gcc/cp/pt.cc                                 |  2 +-
> >   gcc/cp/semantics.cc                          |  4 ++-
> >   gcc/testsuite/g++.dg/modules/tpl-friend-22.C | 24 +++++++++++++++
> >   gcc/testsuite/g++.dg/template/friend88.C     | 32 ++++++++++++++++++++
> >   6 files changed, 65 insertions(+), 8 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-22.C
> >   create mode 100644 gcc/testsuite/g++.dg/template/friend88.C
> > 
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index 74c862ec1c7..31081b390fa 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -19750,7 +19750,7 @@ start_preparsed_function (tree decl1, tree attrs, 
> > int flags)
> >     }
> >       }
> > -  bool honor_interface = (!DECL_TEMPLATE_INSTANTIATION (decl1)
> > +  bool honor_interface = (!DECL_TEMPLOID_INSTANTIATION (decl1)
> >                       /* Implicitly-defined methods (like the
> >                          destructor for a class in which no destructor
> >                          is explicitly declared) must not be defined
> > @@ -19781,7 +19781,7 @@ start_preparsed_function (tree decl1, tree attrs, 
> > int flags)
> >     else if (!finfo->interface_unknown && honor_interface)
> >       {
> >         if (DECL_DECLARED_INLINE_P (decl1)
> > -     || DECL_TEMPLATE_INSTANTIATION (decl1))
> > +     || DECL_TEMPLOID_INSTANTIATION (decl1))
> >     {
> >       DECL_EXTERNAL (decl1)
> >         = (finfo->interface_only
> > @@ -19823,7 +19823,7 @@ start_preparsed_function (tree decl1, tree attrs, 
> > int flags)
> >     DECL_EXTERNAL (decl1) = 0;
> >         if ((DECL_DECLARED_INLINE_P (decl1)
> > -      || DECL_TEMPLATE_INSTANTIATION (decl1))
> > +      || DECL_TEMPLOID_INSTANTIATION (decl1))
> >       && ! DECL_INTERFACE_KNOWN (decl1))
> >     DECL_DEFER_OUTPUT (decl1) = 1;
> >         else
> > diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> > index 8ec9740c8a9..01bf26b1e4f 100644
> > --- a/gcc/cp/decl2.cc
> > +++ b/gcc/cp/decl2.cc
> > @@ -2510,7 +2510,7 @@ vague_linkage_p (tree decl)
> >         || (TREE_CODE (decl) == FUNCTION_DECL
> >       && DECL_DECLARED_INLINE_P (decl))
> >         || (DECL_LANG_SPECIFIC (decl)
> > -     && DECL_TEMPLATE_INSTANTIATION (decl))
> > +     && DECL_TEMPLOID_INSTANTIATION (decl))
> >         || (VAR_P (decl) && DECL_INLINE_VAR_P (decl)))
> >       return true;
> >     else if (DECL_FUNCTION_SCOPE_P (decl))
> > @@ -5850,8 +5850,7 @@ c_parse_final_cleanups (void)
> >       && !(header_module_p ()
> >            && (DECL_DEFAULTED_FN (decl) || decl_tls_wrapper_p (decl)))
> >       /* Don't complain if the template was defined.  */
> > -     && !((DECL_TEMPLATE_INSTANTIATION (decl)
> > -           || DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (decl))
> > +     && !(DECL_TEMPLOID_INSTANTIATION (decl)
> >            && DECL_INITIAL (DECL_TEMPLATE_RESULT
> >                             (template_for_substitution (decl))))
> >       && warning_at (DECL_SOURCE_LOCATION (decl), 0,
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 341e5ab8808..b37de8723ce 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -11652,7 +11652,7 @@ non_templated_friend_p (tree t)
> >      template <class T> friend A<T>::f(); */
> >         tree tmpl = TI_TEMPLATE (ti);
> >         tree primary = DECL_PRIMARY_TEMPLATE (tmpl);
> > -      return (primary && primary != tmpl);
> > +      return ((primary || DECL_NAMESPACE_SCOPE_P (t)) && primary != tmpl);
> >       }
> >     else
> >       return false;
> > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> > index e420fd4ebaf..02a1e6aee0e 100644
> > --- a/gcc/cp/semantics.cc
> > +++ b/gcc/cp/semantics.cc
> > @@ -5537,7 +5537,9 @@ expand_or_defer_fn_1 (tree fn)
> >        of the compilation.  Until that point, we do not want the back
> >        end to output them -- but we do want it to see the bodies of
> >        these functions so that it can inline them as appropriate.  */
> > -  if (DECL_DECLARED_INLINE_P (fn) || DECL_IMPLICIT_INSTANTIATION (fn))
> > +  if (DECL_DECLARED_INLINE_P (fn)
> > +      || DECL_IMPLICIT_INSTANTIATION (fn)
> > +      || DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (fn))
> >       {
> >         if (DECL_INTERFACE_KNOWN (fn))
> >     /* We've already made a decision as to how this function will
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-22.C 
> > b/gcc/testsuite/g++.dg/modules/tpl-friend-22.C
> > new file mode 100644
> > index 00000000000..a77d1cbecad
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-22.C
> > @@ -0,0 +1,24 @@
> > +// PR c++/122819
> > +// { dg-do compile { target *-*-*gnu* } }
> > +// { dg-additional-options "-fmodules" }
> > +
> > +export module M;
> > +
> > +template <typename T> struct S;
> > +void foo(S<float>);
> > +
> > +template <typename T> struct S {
> > +  friend void foo(S) {}
> > +};
> > +
> > +void foo(S<double>);
> > +
> > +void use() {
> > +  foo(S<int>{});
> > +  foo(S<float>{});
> > +  foo(S<double>{});
> > +}
> > +
> > +// { dg-final { scan-assembler "_ZW1M3fooS_1SIiE,comdat" } }
> > +// { dg-final { scan-assembler "_ZW1M3fooS_1SIfE,comdat" } }
> > +// { dg-final { scan-assembler "_ZW1M3fooS_1SIdE,comdat" } }
> > diff --git a/gcc/testsuite/g++.dg/template/friend88.C 
> > b/gcc/testsuite/g++.dg/template/friend88.C
> > new file mode 100644
> > index 00000000000..73d3d525900
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/friend88.C
> > @@ -0,0 +1,32 @@
> > +// PR c++/122819
> > +// { dg-do compile { target c++11 } }
> > +
> > +template <typename T> struct basic_streambuf;
> > +using streambuf = basic_streambuf<char>;
> > +
> > +struct S {
> > +  void foo();
> > +  template <typename T> void bar();
> > +};
> > +
> > +template <typename T> struct X {
> > +  void foo();
> > +};
> > +
> > +template <typename T> struct basic_streambuf {
> > +  void qux();
> > +
> > +  friend void foo();
> > +  friend void S::foo();
> > +  template <typename U> friend void S::bar();
> > +  template <typename U> friend void X<U>::foo();
> > +  template <typename U> friend void basic_streambuf<U>::qux();
> > +};
> > +extern template struct basic_streambuf<char>;
> > +
> > +void foo() {}
> > +void S::foo() {}
> > +
> > +// { dg-final { scan-assembler {_Z3foov:} } }
> > +// { dg-final { scan-assembler {_ZN1S3fooEv:} } }
> > +// { dg-final { scan-assembler-not {comdat} } }
> 

Reply via email to