From c368ea51b3bae27e3007b9bd89f6162c4c80259f Mon Sep 17 00:00:00 2001 From: Nathaniel Shead Date: Fri, 5 Dec 2025 00:03:46 +1100 Subject: [PATCH] c++/modules: Ignore exposures in lambdas in initializers [PR122994] 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 requires a small amount of testsuite adjustment because we no longer diagnose errors we used to. 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 us considering TU-local entities in static initializers of inline variables being exposures (see PR c++/119551). This is legal by the current wording of the standard, which does not consider the definition of any variable to be an exposure (even an inline one). 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 Reviewed-by: Jason Merrill --- gcc/cp/module.cc | 74 +++++++++++++++----- 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, 177 insertions(+), 22 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 2ca381140ea..5eed1d01590 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 () || (strict && get_flag_bit ())); } + bool is_ignored_exposure_context () const + { + return get_flag_bit (); + } public: bool is_import () const @@ -2625,7 +2630,9 @@ 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. */ private: @@ -2646,7 +2653,7 @@ 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); dep_adl_entity_list.create (16); @@ -2663,6 +2670,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 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); @@ -8519,20 +8535,30 @@ 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. */ + /* Stream the keyed entities. There may be keyed entities that we + choose not to stream, such as a lambda in a non-inline variable's + initializer, so don't build dependencies for them here; any deps + we need should be acquired during write_definition (possibly + indirectly). */ 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]; + 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); } } @@ -8844,7 +8870,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 (); + } } } @@ -12948,8 +12984,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)); } @@ -13087,8 +13122,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); @@ -13287,7 +13322,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); @@ -14446,6 +14481,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 (); + if (ek != EK_USING) { tree not_tmpl = STRIP_TEMPLATE (decl); @@ -14569,6 +14607,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 (); dump (dumper::DEPEND) && dump ("%s on %s %C:%N found", @@ -14627,7 +14667,7 @@ depset::hash::add_dependency (depset *dep) else current->set_flag_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 (); @@ -15428,6 +15468,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 int c + = []{ return internal(); }(); // { dg-warning "refers to TU-local entity" } +export template int d + = []{ return internal(); }(); // { dg-warning "refers to TU-local entity" } +template int d; + +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; // { dg-message "required from here" } +int& use_d = d; // { dg-bogus "" } +int& use_d2 = d; // { 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); -- 2.47.3