Bootstrapped on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
Subject: [PATCH] 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.
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-16_b.C: Check for diagnostics when
instantiating imported TU-local entities.
Signed-off-by: Nathaniel Shead <[email protected]>
---
gcc/cp/cp-tree.h | 2 +
gcc/cp/module.cc | 80 +++++++++++++++++---
gcc/cp/pt.cc | 34 ++-------
gcc/testsuite/g++.dg/modules/internal-16_b.C | 10 ++-
4 files changed, 84 insertions(+), 42 deletions(-)
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 017dfd6598d..7426dc54c3f 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 *);
@@ -13931,11 +13926,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);
@@ -14101,8 +14100,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;
@@ -14151,8 +14150,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;
@@ -14205,6 +14204,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.
@@ -14621,7 +14677,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-16_b.C
b/gcc/testsuite/g++.dg/modules/internal-16_b.C
index fded871d82a..f900926dd96 100644
--- a/gcc/testsuite/g++.dg/modules/internal-16_b.C
+++ b/gcc/testsuite/g++.dg/modules/internal-16_b.C
@@ -27,12 +27,14 @@ import M;
void test_usage() {
a();
b();
- c<int>();
- d<int>();
+ c<int>(); // { dg-message "required from here" }
+ d<int>(); // { dg-bogus "" }
e();
- f<int>();
+ f<int>(); // { dg-message "required from here" }
g();
- h<int>();
+ h<int>(); // { dg-bogus "" }
+
+ // { dg-warning "instantiation exposes TU-local entity" "" { target *-*-* }
0 }
}
inline void expose() { // { dg-warning "exposes TU-local" }
