On Sat, Nov 01, 2025 at 01:10:43PM +1100, Nathaniel Shead wrote:
> On Thu, Oct 30, 2025 at 07:15:01PM +0200, Jason Merrill wrote:
> > On 10/28/25 4:53 AM, Patrick Palka wrote:
> > > On Sat, 18 Oct 2025, Patrick Palka wrote:
> > >
> > > > On Sat, 18 Oct 2025, Jason Merrill wrote:
> > > >
> > > > Note local_variable_p also includes DECL_LOCAL_DECL_P decls (i.e.
> > > > block-scope externs), not sure what the linkage of those is but their
> > > > address could in theory be needed in a less contrived way inside a
> > > > template argument where it doesn't get folded at instantiation time.
> >
> > Ah, good point, block-scope externs have linkage, so we don't want to
> > include them.
> >
> > But perhaps local_variable_p itself is the problem; most uses of the term in
> > the standard have changed to "variables with automatic storage duration".
> > The primary use for a diagnostic in a default argument has now been changed
> > in the standard to refer to whether a local *entity* is odr-usable, which
> > does not apply to block-scope externs. The other uses of "local variable"
> > in the standard are talking about cleanup of automatic variables.
> >
> > I think the only current use of local_variable_p where it would make sense
> > to include local externs is in tsubst_expr, but that function handles
> > DECL_LOCAL_DECL_P in the previous clause, so it's not needed there either.
> >
> > So I'd be inclined to change local_variable_p to exclude block-scope
> > externs.
> >
> > Local statics are another question; they can have "vague linkage" and
> > visibility even if they don't have linkage in the standard. And are also
> > not included in "local entity". But tsubst_expr seems to rely on them being
> > included in local_variable_p.
> >
> > We might check decl_storage_duration == dk_auto instead of local_variable_p?
> >
>
> I think using decl_storage_duration makes the most sense, from my
> understanding here. Below is an updated patch that uses this instead.
>
Actually, no, I don't think this is sufficient still; consider
template <typename T> inline void foo() {
static T t;
A<t> a;
}
In this case 't' is still !TREE_PUBLIC, but we have exactly the same
case as with automatic variables in that we might just be using it for
its value, but the A<t> instantiation still gets internal linkage.
As such I think something like my v1 patch is probably the most accurate
in this case (though then we're back to the question of where to check
dependence and in what situations...).
I'm not sure entirely about the DECL_LOCAL_DECL_P case because it turns
out that modules crashes on instantiations naming such decls anyway;
I've made PR c++/122514 for this. But it looks like they may generally
be considered TREE_PUBLIC and so won't have this issue.
> > > > And if we're considering backporting the change moving up the check to
> > > > before the 'addressable' case should be strictly safer and still fix the
> > > > issue. Or we could change it to directly check VAR/PARM_P &&
> > > > DECL_FUNCTION_SCOPE_P (excluding DECL_LOCAL_DECL_P) and keep the check
> > > > in the same spot. Just some ideas, I don't have a strong preference.
> > >
> > > So shall we go with Nathaniel's v2 patch, or a variant of it where say
> > > the added local_variable_p handling is combined with the existing
> > > decl_constant_var_p handling (occurring before the 'addressable' label),
> > > or something else?
> > >
>
> It's a bit of a silly example but I adjusted the testcase with a
> situation where the check needs to go after addressable; consider:
>
> template <int X> struct A {};
> template <typename T>
> void foo() {
> T t;
> A<&t> a;
> }
> struct B { constexpr int operator&() const { return 0; } };
> template void foo<B>();
>
> should instantiate A<0> and not be internal linkage, I don't think.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
>
> -- >8 --
>
> When finding the minimal visibility of a template, any reference to a
> dependent automatic variable will cause the instantiation to be marked
> as internal linkage. However, when processing the template decl we
> don't yet know whether that should actually be the case, as a given
> instantiation may not require referencing the local decl in its
> mangling. So this patch bails on automatic variables.
>
> PR c++/122253
>
> gcc/cp/ChangeLog:
>
> * decl2.cc (min_vis_expr_r): Don't mark as VISIBILITY_ANON for
> an automatic variable.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/internal-16.C: New test.
>
> Signed-off-by: Nathaniel Shead <[email protected]>
> Reviewed-by: Patrick Palka <[email protected]>
> Reviewed-by: Jason Merrill <[email protected]>
> ---
> gcc/cp/decl2.cc | 7 +++++-
> gcc/testsuite/g++.dg/modules/internal-16.C | 27 ++++++++++++++++++++++
> 2 files changed, 33 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-16.C
>
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 0073f83a10c..d78d53bfbcd 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -2885,7 +2885,12 @@ min_vis_expr_r (tree *tp, int */*walk_subtrees*/, void
> *data)
> break;
> }
> addressable:
> - if (! TREE_PUBLIC (t))
> + if (VAR_P (t) && decl_storage_duration (t) == dk_auto)
> + /* Likewise, automatic variables being used as a template argument
> are
> + presumably only used for their value, so shouldn't constrain its
> + visibility. */
> + tpvis = type_visibility (TREE_TYPE (t));
> + else if (!TREE_PUBLIC (t))
> tpvis = VISIBILITY_ANON;
> else
> tpvis = DECL_VISIBILITY (t);
> diff --git a/gcc/testsuite/g++.dg/modules/internal-16.C
> b/gcc/testsuite/g++.dg/modules/internal-16.C
> new file mode 100644
> index 00000000000..95693ebed6c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/internal-16.C
> @@ -0,0 +1,27 @@
> +// PR c++/122253
> +// { dg-additional-options "-fmodules -Wtemplate-names-tu-local" }
> +
> +export module M;
> +
> +template <int> struct ic {};
> +struct S {
> + constexpr operator int() const { return 5; }
> + constexpr int operator&() const { return 8; }
> +};
> +
> +template <typename T> inline void a(T) {
> + T s;
> + ic<s>{};
> + ic<&s>{};
> +}
> +
> +template <typename T> inline auto b(T x) {
> + return [&](auto y) {
> + return [=](auto z) {
> + return ic<(int)x + (int)y + (int)z>{};
> + };
> + };
> +}
> +
> +template void a(S);
> +ic<5 * 3> x = b(S{})(S{})(S{});
> --
> 2.51.0
>
