On Fri, Dec 19, 2025 at 12:29:04PM +0700, Jason Merrill wrote: > On 12/4/25 9:40 PM, Nathaniel Shead wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > As the PR rightly points out, a lambda is not really a declaration in > > and of itself by the standard, and so a lambda only used in a context > > where exposures are ignored should not itself cause an error. > > > > This patch implements this by way of a new flag set on deps that are > > first found in an ignored context. This flag gets cleared if we ever > > see the dep in a context where exposures are not ignored. Then, while > > walking a declaration with this flag set, we re-establish an ignored > > context. This is done for all decls (not just lambdas) to handle > > block-scope classes as well. > > > > Additionally, we prevent walking of attached declarations for a > > DECL_MODULE_KEYED_DECLS_P entity during dependency gathering, so that we > > don't think we've seen the decl at this point. This means we may not > > have an appropriate entity to stream for this walk; to prevent any > > potential issues with merging we stream a NULL_TREE 'hole' in the vector > > and handle this carefully on import. > > > > This causes a small amount of testsuite fallout because we no longer > > diagnose errors we really should. Because our ABI for inline variables > > with dynamic initialization is to just do the initialization in the > > module's initializer function (and importers only perform the static > > initialization) we don't bother to walk the definition of inline > > variables containing lambdas and so don't see the exposures, despite > > this being a non-ignored context. > > Hmm, looking back at the standard it seems that any variable initializer is > an ignored context under [basic.link]/14.2, inline only matters for > functions. So this actually sounds like an improvement, not a regression. >
Right; this is related to r15-9136 (PR119551) where we started complaining about TU-local entities in inline variables because strictly following the standard here is worse (wrong-code). But by standard we shouldn't be complaining about any of these scenarios to start with. This patch is the opposite, though, about us complaining about TU-local entities in the body of a lambda within a non-inline variable definition (i.e. rejects-valid), which I think is still a bug worth fixing. But yes, it's not a regression (we always complained about this case), so if you'd prefer to leave it till GCC17 I'm OK with that. > > Perhaps a fix would be to do a tentative walk just during the initial > > walk to build the dependencies but then just ignore them and throw them > > away, but by the point of modules streaming we've thrown away the > > initializers of such variables anyway. We could rediscover some of the > > lambdas by walking the keyed decls for inline variables just for this > > purpose, but I'm not sure that would work for all cases anyway. > > > > PR c++/122994 > > > > gcc/cp/ChangeLog: > > > > * module.cc (depset::disc_bits): New flag > > DB_IGNORED_EXPOSURE_BIT. > > (depset::is_ignored_exposure_context): New getter. > > (depset::hash::ignore_tu_local): Rename to... > > (depset::hash::ignore_exposure): ...this, and make private. > > (depset::hash::hash): Rename ignore_tu_local. > > (depset::hash::ignore_exposure_if): New function. > > (trees_out::decl_value): Don't build deps for keyed entities. > > (trees_in::decl_value): Handle missing keys. > > (trees_out::write_function_def): Use ignore_exposure_if. > > (trees_out::write_var_def): Likewise. > > (trees_out::write_class_def): Likewise. > > (depset::hash::make_dependency): Set DB_IGNORED_EXPOSURE_BIT if > > appropriate, or clear it otherwise. > > (depset::hash::add_dependency): Rename ignore_tu_local. > > (depset::hash::find_dependencies): Set ignore_exposure if in > > such a context. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/internal-17_b.C: Use functions and internal > > types rather than lambdas. > > * g++.dg/modules/internal-4_b.C: Correct expected result. > > * g++.dg/modules/internal-20_a.C: New test. > > * g++.dg/modules/internal-20_b.C: New test. > > * g++.dg/modules/internal-20_c.C: New test. > > * g++.dg/modules/internal-21_a.C: New test. > > * g++.dg/modules/internal-21_b.C: New test. > > > > Signed-off-by: Nathaniel Shead <[email protected]> > > --- > > gcc/cp/module.cc | 71 +++++++++++++++----- > > gcc/testsuite/g++.dg/modules/internal-17_b.C | 21 ++++-- > > gcc/testsuite/g++.dg/modules/internal-20_a.C | 37 ++++++++++ > > gcc/testsuite/g++.dg/modules/internal-20_b.C | 18 +++++ > > gcc/testsuite/g++.dg/modules/internal-20_c.C | 19 ++++++ > > gcc/testsuite/g++.dg/modules/internal-21_a.C | 14 ++++ > > gcc/testsuite/g++.dg/modules/internal-21_b.C | 13 ++++ > > gcc/testsuite/g++.dg/modules/internal-4_b.C | 3 +- > > 8 files changed, 175 insertions(+), 21 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/modules/internal-20_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/internal-20_b.C > > create mode 100644 gcc/testsuite/g++.dg/modules/internal-20_c.C > > create mode 100644 gcc/testsuite/g++.dg/modules/internal-21_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/internal-21_b.C > > > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > > index 5c70e9bb469..c5f4e70872b 100644 > > --- a/gcc/cp/module.cc > > +++ b/gcc/cp/module.cc > > @@ -2370,6 +2370,7 @@ private: > > DB_REF_PURVIEW_BIT, /* Refers to a purview TU-local entity. > > */ > > DB_EXPOSE_GLOBAL_BIT, /* Exposes a GMF TU-local entity. */ > > DB_EXPOSE_PURVIEW_BIT, /* Exposes a purview TU-local entity. > > */ > > + DB_IGNORED_EXPOSURE_BIT, /* Only seen where exposures are > > ignored. */ > > DB_IMPORTED_BIT, /* An imported entity. */ > > DB_UNREACHED_BIT, /* A yet-to-be reached entity. */ > > DB_MAYBE_RECURSIVE_BIT, /* An entity maybe in a recursive > > cluster. */ > > @@ -2491,6 +2492,10 @@ public: > > return (get_flag_bit<DB_EXPOSE_PURVIEW_BIT> () > > || (strict && get_flag_bit <DB_EXPOSE_GLOBAL_BIT> ())); > > } > > + bool is_ignored_exposure_context () const > > + { > > + return get_flag_bit<DB_IGNORED_EXPOSURE_BIT> (); > > + } > > public: > > bool is_import () const > > @@ -2625,14 +2630,16 @@ public: > > unsigned section; /* When writing out, the section. */ > > bool reached_unreached; /* We reached an unreached entity. */ > > bool writing_merge_key; /* We're writing merge key information. */ > > - bool ignore_tu_local; /* In a context where referencing a TU-local > > + > > + private: > > + bool ignore_exposure; /* In a context where referencing a TU-local > > entity is not an exposure. */ > > public: > > hash (size_t size, hash *c = NULL) > > : parent (size), chain (c), current (NULL), section (0), > > reached_unreached (false), writing_merge_key (false), > > - ignore_tu_local (false) > > + ignore_exposure (false) > > { > > worklist.create (size); > > } > > @@ -2647,6 +2654,15 @@ public: > > return chain != NULL; > > } > > + public: > > + /* Returns a temporary override that will additionally consider this > > + to be a context where exposures of TU-local entities are ignored > > + if COND is true. */ > > + temp_override<bool> ignore_exposure_if (bool cond) > > + { > > + return make_temp_override (ignore_exposure, ignore_exposure || cond); > > + } > > + > > private: > > depset **entity_slot (tree entity, bool = true); > > depset **binding_slot (tree ctx, tree name, bool = true); > > @@ -8488,20 +8504,29 @@ trees_out::decl_value (tree decl, depset *dep) > > if (DECL_LANG_SPECIFIC (inner) > > && DECL_MODULE_KEYED_DECLS_P (inner) > > - && !is_key_order ()) > > + && streaming_p ()) > > { > > /* Stream the keyed entities. */ > > auto *attach_vec = keyed_table->get (inner); > > unsigned num = attach_vec->length (); > > - if (streaming_p ()) > > - u (num); > > + u (num); > > for (unsigned ix = 0; ix != num; ix++) > > { > > tree attached = (*attach_vec)[ix]; > > + > > + /* There may be keyed entities that we chose not to stream, such > > + as a lambda in a non-inline variable's initializer. */ > > + if (attached) > > + { > > + tree ti = TYPE_TEMPLATE_INFO (TREE_TYPE (attached)); > > + if (!dep_hash->find_dependency (attached) > > + && !(ti && dep_hash->find_dependency (TI_TEMPLATE (ti)))) > > + attached = NULL_TREE; > > + } > > + > > tree_node (attached); > > - if (streaming_p ()) > > - dump (dumper::MERGE) > > - && dump ("Written %d[%u] attached decl %N", tag, ix, attached); > > + dump (dumper::MERGE) > > + && dump ("Written %d[%u] attached decl %N", tag, ix, attached); > > } > > } > > @@ -8813,7 +8838,17 @@ trees_in::decl_value () > > if (is_new) > > set.quick_push (attached); > > else if (set[ix] != attached) > > - set_overrun (); > > + { > > + if (!set[ix] || !attached) > > + /* One import left a hole for a lambda dep we chose not > > + to stream, but another import chose to stream that lambda. > > + Let's not error here: hopefully we'll complain later in > > + is_matching_decl about whatever caused us to make a > > + different decision. */ > > + ; > > + else > > + set_overrun (); > > + } > > } > > } > > @@ -12910,8 +12945,7 @@ trees_out::write_function_def (tree decl) > > is ignored for determining exposures. This should only matter > > for templates (we don't emit the bodies of non-inline functions > > to begin with). */ > > - auto ovr = make_temp_override (dep_hash->ignore_tu_local, > > - !DECL_DECLARED_INLINE_P (decl)); > > + auto ovr = dep_hash->ignore_exposure_if (!DECL_DECLARED_INLINE_P > > (decl)); > > tree_node (DECL_INITIAL (decl)); > > tree_node (DECL_SAVED_TREE (decl)); > > } > > @@ -13049,8 +13083,8 @@ trees_out::write_var_def (tree decl) > > { > > /* The initializer of a non-inline variable or variable template is > > ignored for determining exposures. */ > > - auto ovr = make_temp_override (dep_hash->ignore_tu_local, > > - VAR_P (decl) && !DECL_INLINE_VAR_P (decl)); > > + auto ovr = dep_hash->ignore_exposure_if (VAR_P (decl) > > + && !DECL_INLINE_VAR_P (decl)); > > tree init = DECL_INITIAL (decl); > > tree_node (init); > > @@ -13249,7 +13283,7 @@ trees_out::write_class_def (tree defn) > > { > > /* Friend declarations in class definitions are ignored when > > determining exposures. */ > > - auto ovr = make_temp_override (dep_hash->ignore_tu_local, true); > > + auto ovr = dep_hash->ignore_exposure_if (true); > > /* Write the friend classes. */ > > tree_list (CLASSTYPE_FRIEND_CLASSES (type), false); > > @@ -14408,6 +14442,9 @@ depset::hash::make_dependency (tree decl, > > entity_kind ek) > > gcc_checking_assert ((*eslot)->get_entity_kind () == EK_REDIRECT > > && !(*eslot)->deps.length ()); > > + if (ignore_exposure) > > + dep->set_flag_bit<DB_IGNORED_EXPOSURE_BIT> (); > > + > > if (ek != EK_USING) > > { > > tree not_tmpl = STRIP_TEMPLATE (decl); > > @@ -14531,6 +14568,8 @@ depset::hash::make_dependency (tree decl, > > entity_kind ek) > > if (!dep->is_import ()) > > worklist.safe_push (dep); > > } > > + else if (!ignore_exposure) > > + dep->clear_flag_bit<DB_IGNORED_EXPOSURE_BIT> (); > > dump (dumper::DEPEND) > > && dump ("%s on %s %C:%N found", > > @@ -14589,7 +14628,7 @@ depset::hash::add_dependency (depset *dep) > > else > > current->set_flag_bit<DB_REF_GLOBAL_BIT> (); > > - if (!ignore_tu_local && !is_exposure_of_member_type (current, dep)) > > + if (!ignore_exposure && !is_exposure_of_member_type (current, dep)) > > { > > if (dep->is_tu_local ()) > > current->set_flag_bit<DB_EXPOSE_PURVIEW_BIT> (); > > @@ -15306,6 +15345,8 @@ depset::hash::find_dependencies (module_state > > *module) > > add_namespace_context (item, ns); > > } > > + auto ovr = make_temp_override > > + (ignore_exposure, item->is_ignored_exposure_context ()); > > walker.decl_value (decl, current); > > if (current->has_defn ()) > > walker.write_definition (decl, current->refs_tu_local ()); > > diff --git a/gcc/testsuite/g++.dg/modules/internal-17_b.C > > b/gcc/testsuite/g++.dg/modules/internal-17_b.C > > index f900926dd96..d6398fd68cd 100644 > > --- a/gcc/testsuite/g++.dg/modules/internal-17_b.C > > +++ b/gcc/testsuite/g++.dg/modules/internal-17_b.C > > @@ -4,11 +4,22 @@ > > module; > > -static inline int x // { dg-error "TU-local" } > > - // { dg-message "exposed elsewhere" "" { target *-*-* } > > .-1 } > > - = []{ return 1; }(); // { dg-message "internal" } > > +namespace { > > + struct InternalX {}; // { dg-message "internal" } > > + // Only used by '::y', so should be discarded and not complain > > + struct InternalY {}; // { dg-bogus "" } > > +} > > -static inline int y = []{ return 2; }(); // { dg-bogus "" } > > +static inline int x() { // { dg-error "TU-local" } > > + // { dg-message "exposed elsewhere" "" { target *-*-* } > > .-1 } > > + InternalX x; > > + return 1; > > +} > > + > > +static inline int y() { // { dg-bogus "" } > > + InternalY y; > > + return 2; > > +} > > namespace { > > struct S {}; > > @@ -38,7 +49,7 @@ void test_usage() { > > } > > inline void expose() { // { dg-warning "exposes TU-local" } > > - int result = x; > > + int result = x(); > > } > > // Internal linkage types always hard error > > diff --git a/gcc/testsuite/g++.dg/modules/internal-20_a.C > > b/gcc/testsuite/g++.dg/modules/internal-20_a.C > > new file mode 100644 > > index 00000000000..c16e6a999b5 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/internal-20_a.C > > @@ -0,0 +1,37 @@ > > +// PR c++/122994 > > +// { dg-additional-options "-fmodules -Wtemplate-names-tu-local" } > > +// { dg-module-cmi m } > > + > > +export module m; > > + > > +extern "C++" { > > + static int internal() { return 42; } > > +} > > + > > +export int a = internal(); > > +export int b = []{ return internal(); }(); > > + > > +export template <typename T> int c > > + = []{ return internal(); }(); // { dg-warning "refers to TU-local > > entity" } > > +export template <typename T> int d > > + = []{ return internal(); }(); // { dg-warning "refers to TU-local > > entity" } > > +template int d<int>; > > + > > +export int e = []{ > > + return []{ > > + return internal(); > > + }(); > > +}(); > > + > > +export int f = []{ > > + struct S { > > + inline int foo() { > > + return internal(); > > + } > > + }; > > + return S{}.foo(); > > +}(); > > + > > +export extern "C++" { > > + int merge = []{ return internal(); }(); > > +} > > diff --git a/gcc/testsuite/g++.dg/modules/internal-20_b.C > > b/gcc/testsuite/g++.dg/modules/internal-20_b.C > > new file mode 100644 > > index 00000000000..ca109c6353d > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/internal-20_b.C > > @@ -0,0 +1,18 @@ > > +// PR c++/122994 > > +// { dg-additional-options "-fmodules" } > > + > > +static int internal() { return 42; } > > +int merge = []{ return internal(); }(); > > + > > +import m; > > + > > +int& use_a = a; > > +int& use_b = b; > > +int& use_c = c<int>; // { dg-message "required from here" } > > +int& use_d = d<int>; // { dg-bogus "" } > > +int& use_d2 = d<double>; // { dg-message "required from here" } > > +int& use_e = e; > > +int& use_f = f; > > +int& use_merge = merge; > > + > > +// { dg-error "instantiation exposes TU-local entity" "" { target *-*-* } > > 0 } > > diff --git a/gcc/testsuite/g++.dg/modules/internal-20_c.C > > b/gcc/testsuite/g++.dg/modules/internal-20_c.C > > new file mode 100644 > > index 00000000000..03e79659be0 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/internal-20_c.C > > @@ -0,0 +1,19 @@ > > +// PR c++/122994 > > +// { dg-additional-options "-fmodules" } > > +// { dg-module-cmi !x } > > + > > +export module x; > > + > > +static int internal() { return 42; } > > + > > +auto a > > + = []{ return internal(); }; // { dg-error "exposes TU-local" } > > + > > +auto b = []{ > > + struct S { > > + inline int foo() { // { dg-error "exposes TU-local" } > > + return internal(); > > + } > > + }; > > + return S{}; > > +}(); > > diff --git a/gcc/testsuite/g++.dg/modules/internal-21_a.C > > b/gcc/testsuite/g++.dg/modules/internal-21_a.C > > new file mode 100644 > > index 00000000000..f50b51a1da6 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/internal-21_a.C > > @@ -0,0 +1,14 @@ > > +// { dg-additional-options "-fmodules" } > > +// { dg-module-cmi M } > > + > > +export module M; > > + > > +static int internal() { return 42; } > > + > > +// We don't error here, despite this being an exposure we really should > > +// probably complain about, because we never actually expose the lambda > > +// (the dynamic initialization just occurs in this TU and nowhere else) > > +// and so this appears to function "correctly"... > > +export inline int x = internal(); // { dg-error "exposes TU-local" "" { > > xfail *-*-* } } > > +export inline int y > > + = []{ return internal(); }(); // { dg-error "exposes TU-local" "" { > > xfail *-*-* } } > > diff --git a/gcc/testsuite/g++.dg/modules/internal-21_b.C > > b/gcc/testsuite/g++.dg/modules/internal-21_b.C > > new file mode 100644 > > index 00000000000..e40b99793c5 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/internal-21_b.C > > @@ -0,0 +1,13 @@ > > +// { dg-module-do run } > > +// { dg-additional-options "-fmodules" } > > + > > +import M; > > + > > +// Ideally M was not built and so this file is not needed at all, > > +// but while it is, let's at least check we function correctly. > > +int main() { > > + if (x != 42) > > + __builtin_abort (); > > + if (y != 42) > > + __builtin_abort (); > > +} > > diff --git a/gcc/testsuite/g++.dg/modules/internal-4_b.C > > b/gcc/testsuite/g++.dg/modules/internal-4_b.C > > index a6ccb43b446..6ca72a33393 100644 > > --- a/gcc/testsuite/g++.dg/modules/internal-4_b.C > > +++ b/gcc/testsuite/g++.dg/modules/internal-4_b.C > > @@ -66,8 +66,9 @@ V* expose_v; // { dg-error "exposes TU-local entity" } > > static auto internal_lambda = []{ internal_f(); }; // OK > > auto expose_lambda = internal_lambda; // { dg-error "exposes TU-local > > entity" } > > +// This is OK because we ignore exposures in an initializer. > > int not_in_tu_local > > - = ([]{ internal_f(); }(), // { dg-error "exposes TU-local entity" } > > + = ([]{ internal_f(); }(), // { dg-bogus "exposes TU-local entity" } > > 0); >
