Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
I'm slightly concerned about ordering based on DECL_UID for imported
namespace-scope names as that will depend on when lazy loading occurs
for the entity in question, which will not generally be predictable at
all. Maybe we should check for DECL_MODULE_ENTITY_P, and always order
them at the end based on their index in entity_ary or something? Not
sure if that is actually much better though.
But it should still be at least deterministic as far as I'm aware, so I
think this could be improved upon incrementally.
One question I had is how we should determine the visibility of names
for members_of; does this get affected by instantiation context, e.g.
for
export module M;
import std;
constexpr auto ctx = std::meta::access_context::unchecked();
export template <std::meta::info scope>
consteval std::size_t num_members() {
constexpr auto result = num_members(scope, ctx).size(); // #1
return result;
}
namespace ns {
void foo() {}
export void bar() {}
}
does doing 'num_members<^^ns>()' from an unrelated importing module
return '1' or '2' (that is, does it include 'foo')? This patch
implements so that it always returns 1, evaluation is done from the
lookup context of the current module, but I wonder if evaluation here
should be done from the context of the constant expression at #1.
-- >8 --
namespace_members_of needs to iterate over all VECTOR_BINDINGs
for the given namespace, and handle deduplication.
PR c++/124200
gcc/cp/ChangeLog:
* name-lookup.h (walk_namespace_bindings): Declare.
* name-lookup.cc (walk_namespace_bindings): New function.
* reflect.cc (namespace_members_of): Use it.
gcc/testsuite/ChangeLog:
* g++.dg/modules/reflect-2_a.H: New test.
* g++.dg/modules/reflect-2_b.C: New test.
* g++.dg/modules/reflect-2_c.C: New test.
Signed-off-by: Nathaniel Shead <[email protected]>
Co-authored-by: Thomas Berger <[email protected]>
---
gcc/cp/name-lookup.cc | 112 +++++++++++++++++
gcc/cp/name-lookup.h | 3 +
gcc/cp/reflect.cc | 132 ++++++++++-----------
gcc/testsuite/g++.dg/modules/reflect-2_a.H | 9 ++
gcc/testsuite/g++.dg/modules/reflect-2_b.C | 23 ++++
gcc/testsuite/g++.dg/modules/reflect-2_c.C | 38 ++++++
6 files changed, 251 insertions(+), 66 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/reflect-2_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/reflect-2_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/reflect-2_c.C
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 9a732e14979..5b821800b7e 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -4347,6 +4347,118 @@ ovl_iterator::exporting_p () const
return OVL_EXPORT_P (ovl);
}
+/* Given a namespace NS, walk all of its bindings, calling CALLBACK
+ for all visible decls. Any lazy module bindings will be loaded
+ at this point. */
+
+void
+walk_namespace_bindings (tree ns, void (*callback) (tree decl, void *data),
+ void *data)
+{
+ for (tree o : *DECL_NAMESPACE_BINDINGS (ns))
+ if (TREE_CODE (o) == BINDING_VECTOR)
+ {
+ /* Modules may have duplicate entities on the binding slot.
+ Keep track of this as needed, and ensure we only call
+ the callback once per entity. */
+ hash_set<tree> seen;
+ const auto callback_maybe_dup = [&](tree decl)
+ {
+ if (!seen.add (decl))
+ callback (decl, data);
+ };
+
+ /* First the current module. There should be no dups yet,
+ but track anything that might be dup'd later. */
+ binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (o);
+ if (tree bind = cluster->slots[BINDING_SLOT_CURRENT])
+ {
+ tree value = bind;
+ if (STAT_HACK_P (bind))
+ {
+ if (STAT_TYPE (bind) && !STAT_TYPE_HIDDEN_P (bind))
+ callback_maybe_dup (STAT_TYPE (bind));
+ if (STAT_DECL_HIDDEN_P (bind))
+ value = NULL_TREE;
+ else
+ value = STAT_DECL (bind);
+ }
+ value = ovl_skip_hidden (value);
+ for (tree decl : ovl_range (value))
+ callback_maybe_dup (decl);
+ }
+
+ /* Now the imported bindings. */
+ bitmap imports = get_import_bitmap ();
+ tree name = BINDING_VECTOR_NAME (o);
+
+ unsigned ix = BINDING_VECTOR_NUM_CLUSTERS (o);
+ if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
+ {
+ ix--;
+ cluster++;
+ }
+
+ /* Do this in forward order, so we load modules in an order
+ the user expects. */
+ for (; ix--; cluster++)
+ for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
+ {
+ /* Are we importing this module? */
+ if (unsigned base = cluster->indices[jx].base)
+ if (unsigned span = cluster->indices[jx].span)
+ do
+ if (bitmap_bit_p (imports, base))
+ goto found;
+ while (++base, --span);
+ continue;
+
+ found:;
+ /* Is it loaded? */
+ unsigned mod = cluster->indices[jx].base;
+ if (cluster->slots[jx].is_lazy ())
+ {
+ gcc_assert (cluster->indices[jx].span == 1);
+ lazy_load_binding (mod, ns, name, &cluster->slots[jx]);
+ }
+
+ tree bind = cluster->slots[jx];
+ if (!bind)
+ /* Load errors could mean there's nothing here. */
+ continue;
+
+ /* Extract what we can see from here. If there's no
+ stat_hack, then everything was exported. */
+ tree value = bind;
+ if (STAT_HACK_P (bind))
+ {
+ if (STAT_TYPE_VISIBLE_P (bind))
+ callback_maybe_dup (STAT_TYPE (bind));
+ value = STAT_VISIBLE (bind);
+ }
+ value = ovl_skip_hidden (value);
+ for (tree decl : ovl_range (value))
+ callback_maybe_dup (decl);
+ }
+ }
+ else
+ {
+ tree bind = o;
+ if (STAT_HACK_P (bind))
+ {
+ if (STAT_TYPE (bind) && !STAT_TYPE_HIDDEN_P (bind))
+ callback (STAT_TYPE (bind), data);
+ if (STAT_DECL_HIDDEN_P (bind))
+ bind = NULL_TREE;
+ else
+ bind = STAT_DECL (bind);
+ }
+ bind = ovl_skip_hidden (bind);
+ for (tree decl : ovl_range (bind))
+ callback (decl, data);
+ }
+}
+
/* Given a namespace-level binding BINDING, walk it, calling CALLBACK
for all decls of the current module. When partitions are involved,
decls might be mentioned more than once. Return the accumulation of
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 4c167cd135c..69a8d6065b5 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -497,6 +497,9 @@ extern bool maybe_push_to_top_level (tree);
extern void maybe_pop_from_top_level (bool);
extern void push_using_decl_bindings (tree, tree);
extern void expose_existing_namespace (tree);
+extern void walk_namespace_bindings (tree ns,
+ void (*callback) (tree, void *data),
+ void *data);
/* Lower level interface for modules. */
extern tree *mergeable_namespace_slots (tree ns, tree name, bool is_attached,
diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc
index 46355251bcd..b2bd2241337 100644
--- a/gcc/cp/reflect.cc
+++ b/gcc/cp/reflect.cc
@@ -6469,77 +6469,77 @@ members_cmp (const void *a, const void *b)
static vec<constructor_elt, va_gc> *
namespace_members_of (location_t loc, tree ns)
{
- vec<constructor_elt, va_gc> *elts = nullptr;
- hash_set<tree> *seen = nullptr;
- for (tree o : *DECL_NAMESPACE_BINDINGS (ns))
+ struct data_t {
+ location_t loc = UNKNOWN_LOCATION;
+ tree ns = NULL_TREE;
+ vec<constructor_elt, va_gc> *elts = nullptr;
+ hash_set<tree> *seen = nullptr;
+
+ data_t (location_t loc, tree ns) : loc (loc), ns (ns) {}
+ ~data_t () { delete seen; }
+ };
+
+ const auto walker = [](tree b, void *data_)
{
- if (TREE_CODE (o) == OVERLOAD && OVL_LOOKUP_P (o))
+ auto *data = static_cast<data_t *>(data_);
+ tree m = b;
+
+ if (VAR_P (b) && DECL_ANON_UNION_VAR_P (b))
{
- if (TREE_TYPE (o))
- {
- tree m = TREE_TYPE (TREE_TYPE (o));
- if (members_of_representable_p (ns, m))
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- }
- if (OVL_DEDUP_P (o) || !OVL_FUNCTION (o))
- continue;
- o = OVL_FUNCTION (o);
+ /* TODO: This doesn't handle namespace N { static union {}; }
+ but we pedwarn on that, so perhaps it doesn't need to be
+ handled. */
+ tree v = DECL_VALUE_EXPR (b);
+ gcc_assert (v && TREE_CODE (v) == COMPONENT_REF);
+ tree var = TREE_OPERAND (v, 0);
+ tree type = TREE_TYPE (var);
+ if (!data->seen)
+ data->seen = new hash_set<tree>;
+ if (members_of_representable_p (data->ns, type)
+ && !data->seen->add (type))
+ CONSTRUCTOR_APPEND_ELT (data->elts, NULL_TREE,
+ get_reflection_raw (data->loc, type));
+ if (members_of_representable_p (data->ns, var)
+ && !data->seen->add (var))
+ CONSTRUCTOR_APPEND_ELT (data->elts, NULL_TREE,
+ get_reflection_raw (data->loc, var));
+ return;
}
- for (ovl_iterator iter (o); iter; ++iter)
- {
- if (iter.hidden_p ())
- continue;
- tree b = *iter;
- tree m = b;
- if (VAR_P (b) && DECL_ANON_UNION_VAR_P (b))
- {
- /* TODO: This doesn't handle namespace N { static union {}; }
- but we pedwarn on that, so perhaps it doesn't need to be
- handled. */
- tree v = DECL_VALUE_EXPR (b);
- gcc_assert (v && TREE_CODE (v) == COMPONENT_REF);
- tree var = TREE_OPERAND (v, 0);
- tree type = TREE_TYPE (var);
- if (!seen)
- seen = new hash_set<tree>;
- if (members_of_representable_p (ns, type) && !seen->add (type))
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, type));
- if (members_of_representable_p (ns, var) && !seen->add (var))
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, var));
- continue;
- }
- if (TREE_CODE (b) == TYPE_DECL)
- m = TREE_TYPE (b);
- if (!members_of_representable_p (ns, m))
- continue;
- if (DECL_DECOMPOSITION_P (m) && !DECL_DECOMP_IS_BASE (m))
- {
- tree base = DECL_DECOMP_BASE (m);
- if (!seen)
- seen = new hash_set<tree>;
- if (members_of_representable_p (ns, base) && !seen->add (base))
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, base));
- if (!DECL_HAS_VALUE_EXPR_P (m))
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m,
- REFLECT_VAR));
- continue;
- }
- /* eval_is_accessible should be always true for namespace members,
- so don't bother calling it here. */
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
+ if (TREE_CODE (b) == TYPE_DECL)
+ m = TREE_TYPE (b);
+
+ if (!members_of_representable_p (data->ns, m))
+ return;
+
+ if (DECL_DECOMPOSITION_P (m) && !DECL_DECOMP_IS_BASE (m))
+ {
+ tree base = DECL_DECOMP_BASE (m);
+ if (!data->seen)
+ data->seen = new hash_set<tree>;
+ if (members_of_representable_p (data->ns, base)
+ && !data->seen->add (base))
+ CONSTRUCTOR_APPEND_ELT (data->elts, NULL_TREE,
+ get_reflection_raw (data->loc, base));
+ if (!DECL_HAS_VALUE_EXPR_P (m))
+ CONSTRUCTOR_APPEND_ELT (data->elts, NULL_TREE,
+ get_reflection_raw (data->loc, m,
+ REFLECT_VAR));
+ return;
}
- }
- delete seen;
- if (elts)
- elts->qsort (members_cmp);
- return elts;
+
+ /* eval_is_accessible should be always true for namespace members,
+ so don't bother calling it here. */
+ CONSTRUCTOR_APPEND_ELT (data->elts, NULL_TREE,
+ get_reflection_raw (data->loc, m));
+ };
+
+ data_t data (loc, ns);
+ walk_namespace_bindings (ns, walker, &data);
+
+ if (data.elts)
+ data.elts->qsort (members_cmp);
+ return data.elts;
}
/* Enumerate members of class R for eval_*members_of. KIND is
diff --git a/gcc/testsuite/g++.dg/modules/reflect-2_a.H
b/gcc/testsuite/g++.dg/modules/reflect-2_a.H
new file mode 100644
index 00000000000..fa5932a4f29
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/reflect-2_a.H
@@ -0,0 +1,9 @@
+// PR c++/124200
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fmodule-header -freflection" }
+// { dg-module-cmi {} }
+
+namespace ns {
+ inline void a() {}
+ inline void b() {}
+}
diff --git a/gcc/testsuite/g++.dg/modules/reflect-2_b.C
b/gcc/testsuite/g++.dg/modules/reflect-2_b.C
new file mode 100644
index 00000000000..f710e6c464e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/reflect-2_b.C
@@ -0,0 +1,23 @@
+// PR c++/124200
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fmodules -freflection -Wno-global-module" }
+// { dg-module-cmi M }
+
+module;
+namespace ns {
+ inline void b() {}
+ inline void c() {}
+}
+export module M;
+namespace ns {
+ export using ns::b;
+ export extern "C++" inline void d() {}
+ void e() {}
+ export void f() {}
+
+ export struct S {} S;
+ export struct T {};
+ T T;
+ struct U {};
+ export U U;
+}
diff --git a/gcc/testsuite/g++.dg/modules/reflect-2_c.C
b/gcc/testsuite/g++.dg/modules/reflect-2_c.C
new file mode 100644
index 00000000000..15e4b92bb48
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/reflect-2_c.C
@@ -0,0 +1,38 @@
+// PR c++/124200
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fmodules -freflection" }
+// Test that reflecting names from modules works, and we deduplicate correctly.
+
+#include <meta>
+
+namespace ns {
+ inline void b() {} // dedup with reflect-1_a.H and M GMF
+ inline void d() {} // dedup with M purview
+ inline void f() {}
+}
+
+import "reflect-2_a.H";
+import M;
+
+constexpr auto ctx = std::meta::access_context::unchecked();
+consteval int count_name(std::meta::info i, const char* name) {
+ int count = 0;
+ for (std::meta::info x : members_of(i, ctx))
+ if (has_identifier(x) && identifier_of(x) == name)
+ ++count;
+ return count;
+}
+
+static_assert(count_name(^^ns, "a") == 1);
+static_assert(count_name(^^ns, "b") == 1);
+static_assert(count_name(^^ns, "c") == 0); // discarded from M's GMF
+static_assert(count_name(^^ns, "d") == 1);
+static_assert(count_name(^^ns, "e") == 0); // e@M is not visible from here
+static_assert(count_name(^^ns, "f") == 2); // f and f@M are separate entities
+
+// handle imported stat hack
+static_assert(count_name(^^ns, "S") == 2);
+static_assert(count_name(^^ns, "T") == 1);
+static_assert(count_name(^^ns, "U") == 1);
+
+static_assert(members_of(^^ns, ctx).size() == 9);
--
2.51.0
