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 <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

Author: wreien

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 ();

gcc/cp/module.cc

@@ -2663,11 +2663,6 @@ class depset {
     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.  */

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))

gcc/testsuite/g++.dg/modules/internal-17_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" }