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

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 2ca381140ea27bbfc124d8df7d71d000bf93e470..5eed1d01590f3f73c4cf1cb9d17c24f38297ed2d 100644 (file)
--- 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<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 @@ 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<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 ());

diff --git a/gcc/testsuite/g++.dg/modules/internal-17_b.C b/gcc/testsuite/g++.dg/modules/internal-17_b.C
index f900926dd968a7146f6d2eaae004d3a92c505a20..d6398fd68cdc3b2fe4cac62af240cb9783a62584 100644 (file)
--- 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 (file)
index 0000000..c16e6a9
--- /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 <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(); }();
+}

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 (file)
index 0000000..ca109c6
--- /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<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 }

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 (file)
index 0000000..03e7965
--- /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 (file)
index 0000000..f50b51a
--- /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 (file)
index 0000000..e40b997
--- /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 a6ccb43b446656496a2f4a08a26b9443519d6992..6ca72a33393c61bb246dae19374f44c61d2d69d3 100644 (file)
--- 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);