From: Nathaniel Shead Date: Sun, 2 Nov 2025 04:58:39 +0000 (+1100) Subject: c++/modules: Complain on imported GMF TU-local entities in instantiation [PR121574] X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f5db79b06eb85f715dc3180065cb326ed180ab23;p=thirdparty%2Fgcc.git c++/modules: Complain on imported GMF TU-local entities in instantiation [PR121574] An unfortunate side effect of the previous patch is that even with -pedantic-errors, unless the user specifies -Wtemplate-names-tu-local when building the module interface there will be no diagnostic at all from instantiating a template that exposes global TU-local entities, either when building the module or its importer. This patch solves this by recognising imported TU-local dependencies, even if they weren't streamed as TU_LOCAL_ENTITY nodes. The warnings here are deliberately conservative for when we can be sure this was actually an imported TU-local entity; in particular, we bail on any TU-local entity that originated from a header module, without attempting to determine if the entity came via a named module first. PR c++/121574 gcc/cp/ChangeLog: * cp-tree.h (instantiating_tu_local_entity): Declare. * module.cc (is_tu_local_entity): Extract from depset::hash. (is_tu_local_value): Likewise. (has_tu_local_tmpl_arg): Likewise. (depset::hash::is_tu_local_entity): Remove. (depset::hash::has_tu_local_tmpl_arg): Remove. (depset::hash::is_tu_local_value): Remove. (instantiating_tu_local_entity): New function. (depset::hash::add_binding_entity): No longer go through depset::hash to check is_tu_local_entity. * pt.cc (complain_about_tu_local_entity): Remove. (tsubst): Use instantiating_tu_local_entity. (tsubst_expr): Likewise. gcc/testsuite/ChangeLog: * g++.dg/modules/internal-17_b.C: Check for diagnostics when instantiating imported TU-local entities. Signed-off-by: Nathaniel Shead Reviewed-by: Jason Merrill --- diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 8c211ab7874..4f077e70151 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7745,6 +7745,8 @@ extern module_state *get_module (tree name, module_state *parent = NULL, bool partition = false); extern bool module_may_redeclare (tree olddecl, tree newdecl = NULL); +extern bool instantiating_tu_local_entity (tree decl); + extern bool module_global_init_needed (); extern bool module_determine_import_inits (); extern void module_add_import_initializers (); diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 3aa6686fd86..0610ee5f65a 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -2663,11 +2663,6 @@ public: depset *add_dependency (tree decl, entity_kind); void add_namespace_context (depset *, tree ns); - private: - bool has_tu_local_tmpl_arg (tree decl, tree args, bool explain); - bool is_tu_local_entity (tree decl, bool explain = false); - bool is_tu_local_value (tree decl, tree expr, bool explain = false); - private: static bool add_binding_entity (tree, WMB_Flags, void *); @@ -13934,11 +13929,15 @@ depset::hash::find_binding (tree ctx, tree name) return slot ? *slot : NULL; } +static bool is_tu_local_entity (tree decl, bool explain = false); +static bool is_tu_local_value (tree decl, tree expr, bool explain = false); +static bool has_tu_local_tmpl_arg (tree decl, tree args, bool explain); + /* Returns true if DECL is a TU-local entity, as defined by [basic.link]. If EXPLAIN is true, emit an informative note about why DECL is TU-local. */ -bool -depset::hash::is_tu_local_entity (tree decl, bool explain/*=false*/) +static bool +is_tu_local_entity (tree decl, bool explain/*=false*/) { gcc_checking_assert (DECL_P (decl)); location_t loc = DECL_SOURCE_LOCATION (decl); @@ -14104,8 +14103,8 @@ depset::hash::is_tu_local_entity (tree decl, bool explain/*=false*/) /* Helper for is_tu_local_entity. Returns true if one of the ARGS of DECL is TU-local. Emits an explanation if EXPLAIN is true. */ -bool -depset::hash::has_tu_local_tmpl_arg (tree decl, tree args, bool explain) +static bool +has_tu_local_tmpl_arg (tree decl, tree args, bool explain) { if (!args || TREE_CODE (args) != TREE_VEC) return false; @@ -14154,8 +14153,8 @@ depset::hash::has_tu_local_tmpl_arg (tree decl, tree args, bool explain) /* Returns true if EXPR (part of the initializer for DECL) is a TU-local value or object. Emits an explanation if EXPLAIN is true. */ -bool -depset::hash::is_tu_local_value (tree decl, tree expr, bool explain) +static bool +is_tu_local_value (tree decl, tree expr, bool explain/*=false*/) { if (!expr) return false; @@ -14208,6 +14207,63 @@ depset::hash::is_tu_local_value (tree decl, tree expr, bool explain) return false; } +/* Complains if DECL is a TU-local entity imported from a named module. + Returns TRUE if instantiation should fail. */ + +bool +instantiating_tu_local_entity (tree decl) +{ + if (!modules_p ()) + return false; + + if (TREE_CODE (decl) == TU_LOCAL_ENTITY) + { + auto_diagnostic_group d; + error ("instantiation exposes TU-local entity %qD", + TU_LOCAL_ENTITY_NAME (decl)); + inform (TU_LOCAL_ENTITY_LOCATION (decl), "declared here"); + return true; + } + + /* Currently, only TU-local variables and functions will be emitted + from named modules. */ + if (!VAR_OR_FUNCTION_DECL_P (decl)) + return false; + + /* From this point we will only be emitting warnings; if we're not + warning about this case then there's no need to check further. */ + if (!warn_expose_global_module_tu_local + || !warning_enabled_at (DECL_SOURCE_LOCATION (decl), + OPT_Wexpose_global_module_tu_local)) + return false; + + if (!is_tu_local_entity (decl)) + return false; + + tree origin = get_originating_module_decl (decl); + if (!DECL_LANG_SPECIFIC (origin) + || !DECL_MODULE_IMPORT_P (origin)) + return false; + + /* Referencing TU-local entities from a header is generally OK. + We don't have an easy way to detect if this declaration came + from a header via a separate named module, but we can just + ignore that case for warning purposes. */ + unsigned index = import_entity_index (origin); + module_state *mod = import_entity_module (index); + if (mod->is_header ()) + return false; + + auto_diagnostic_group d; + warning (OPT_Wexpose_global_module_tu_local, + "instantiation exposes TU-local entity %qD", decl); + inform (DECL_SOURCE_LOCATION (decl), "declared here"); + + /* We treat TU-local entities from the GMF as not actually being + TU-local as an extension, so allow instantation to proceed. */ + return false; +} + /* DECL is a newly discovered dependency. Create the depset, if it doesn't already exist. Add it to the worklist if so. @@ -14624,7 +14680,7 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_) return false; bool internal_decl = false; - if (!header_module_p () && data->hash->is_tu_local_entity (decl)) + if (!header_module_p () && is_tu_local_entity (decl)) { /* A TU-local entity. For ADL we still need to create bindings for internal-linkage functions attached to a named module. */ diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index f61223f0bab..c233bb95cad 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -9941,17 +9941,6 @@ add_pending_template (tree d) pop_tinst_level (); } -/* Emit a diagnostic about instantiating a reference to TU-local entity E. */ - -static void -complain_about_tu_local_entity (tree e) -{ - auto_diagnostic_group d; - error ("instantiation exposes TU-local entity %qD", - TU_LOCAL_ENTITY_NAME (e)); - inform (TU_LOCAL_ENTITY_LOCATION (e), "declared here"); -} - /* Return a TEMPLATE_ID_EXPR corresponding to the indicated FNS and ARGLIST. Valid choices for FNS are given in the cp-tree.def documentation for TEMPLATE_ID_EXPR. */ @@ -16614,12 +16603,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) return t; /* Any instantiation of a template containing a TU-local entity is an - exposure, so always issue a hard error irrespective of complain. */ - if (TREE_CODE (t) == TU_LOCAL_ENTITY) - { - complain_about_tu_local_entity (t); - return error_mark_node; - } + exposure, so always issue a diagnostic irrespective of complain. */ + if (instantiating_tu_local_entity (t)) + return error_mark_node; tsubst_flags_t tst_ok_flag = (complain & tf_tst_ok); complain &= ~tf_tst_ok; @@ -20865,6 +20851,9 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) tsubst_flags_t no_name_lookup_flag = (complain & tf_no_name_lookup); complain &= ~tf_no_name_lookup; + if (instantiating_tu_local_entity (t)) + RETURN (error_mark_node); + if (!no_name_lookup_flag) if (tree d = maybe_dependent_member_ref (t, args, complain, in_decl)) return d; @@ -22507,11 +22496,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) case OVERLOAD: if (modules_p ()) for (tree ovl : lkp_range (t)) - if (TREE_CODE (ovl) == TU_LOCAL_ENTITY) - { - complain_about_tu_local_entity (ovl); - RETURN (error_mark_node); - } + if (instantiating_tu_local_entity (ovl)) + RETURN (error_mark_node); RETURN (t); case TEMPLATE_DECL: @@ -22791,10 +22777,6 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) RETURN (op); } - case TU_LOCAL_ENTITY: - complain_about_tu_local_entity (t); - RETURN (error_mark_node); - default: /* Handle Objective-C++ constructs, if appropriate. */ if (tree subst = objcp_tsubst_expr (t, args, complain, in_decl)) diff --git a/gcc/testsuite/g++.dg/modules/internal-17_b.C b/gcc/testsuite/g++.dg/modules/internal-17_b.C index fded871d82a..f900926dd96 100644 --- a/gcc/testsuite/g++.dg/modules/internal-17_b.C +++ b/gcc/testsuite/g++.dg/modules/internal-17_b.C @@ -27,12 +27,14 @@ import M; void test_usage() { a(); b(); - c(); - d(); + c(); // { dg-message "required from here" } + d(); // { dg-bogus "" } e(); - f(); + f(); // { dg-message "required from here" } g(); - h(); + h(); // { dg-bogus "" } + + // { dg-warning "instantiation exposes TU-local entity" "" { target *-*-* } 0 } } inline void expose() { // { dg-warning "exposes TU-local" }