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 <[email protected]>
Reviewed-by: Jason Merrill <[email protected]>

Author: wreien

gcc/cp/module.cc

@@ -2370,6 +2370,7 @@ class depset {
     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 @@ class depset {
     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,7 +2630,9 @@ class depset {
     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 @@ class depset {
     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 @@ class depset {
       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);
@@ -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<DB_IGNORED_EXPOSURE_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<DB_IGNORED_EXPOSURE_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<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> ();
@@ -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 ());

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

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

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 }

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{};
+}();

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 *-*-* } }

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

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