Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
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 causes a small amount of testsuite fallout because we no longer
diagnose errors we really should. 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
this being a non-ignored context.
Perhaps a fix would be to do a tentative walk just during the initial
walk to build the dependencies but then just ignore them and throw them
away, but by the point of modules streaming we've thrown away the
initializers of such variables anyway. We could rediscover some of the
lambdas by walking the keyed decls for inline variables just for this
purpose, but I'm not sure that would work for all cases anyway.
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]>
---
gcc/cp/module.cc | 71 +++++++++++++++-----
gcc/testsuite/g++.dg/modules/internal-17_b.C | 21 ++++--
gcc/testsuite/g++.dg/modules/internal-20_a.C | 37 ++++++++++
gcc/testsuite/g++.dg/modules/internal-20_b.C | 18 +++++
gcc/testsuite/g++.dg/modules/internal-20_c.C | 19 ++++++
gcc/testsuite/g++.dg/modules/internal-21_a.C | 14 ++++
gcc/testsuite/g++.dg/modules/internal-21_b.C | 13 ++++
gcc/testsuite/g++.dg/modules/internal-4_b.C | 3 +-
8 files changed, 175 insertions(+), 21 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/internal-20_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/internal-20_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/internal-20_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/internal-21_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/internal-21_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 5c70e9bb469..c5f4e70872b 100644
--- 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,14 +2630,16 @@ 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. */
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);
}
@@ -2647,6 +2654,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);
@@ -8488,20 +8504,29 @@ 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. */
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];
+
+ /* There may be keyed entities that we chose not to stream, such
+ as a lambda in a non-inline variable's initializer. */
+ 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);
}
}
@@ -8813,7 +8838,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 ();
+ }
}
}
@@ -12910,8 +12945,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));
}
@@ -13049,8 +13083,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);
@@ -13249,7 +13283,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);
@@ -14408,6 +14442,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);
@@ -14531,6 +14568,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",
@@ -14589,7 +14628,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> ();
@@ -15306,6 +15345,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 f900926dd96..d6398fd68cd 100644
--- 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
index 00000000000..c16e6a999b5
--- /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
index 00000000000..ca109c6353d
--- /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
index 00000000000..03e79659be0
--- /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
index 00000000000..f50b51a1da6
--- /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
index 00000000000..e40b99793c5
--- /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 a6ccb43b446..6ca72a33393 100644
--- 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);
--
2.51.0
