The ordering logic of class_members_of is based on the DECL_UIDs for the class members. While this works for non-imported types, it breaks for imported classes and their members.
As DECL_UID is the only existing indicator of declaration order, we
preserve the original DECL_UID in the CMI for relevant class members.
When a class member decl is loaded from the CMI, we store the original
DECL_UID in a side-table. This happens only if -freflection is enabled
for the current, consuming TU.
While this will increase the size of the CMIs, it should only
add minimal overhead during module streaming. Alternatives I have found
would require either changes to core data structures (like decl_lang*)
or more complex logic in trees_out.
gcc/cp/ChangeLog:
* module.cc
(trees_out::decl_value): Persist DECL_UID.
(trees_in::decl_value): Read and register old DECL_UID.
* name-lookup.cc
(register_member_orig_uid): New function.
(get_member_sort_uid): Helper for class_member_cmp.
(class_member_cmp): Comparator for qsort.
(walk_class_members): New function.
* name-lookup.h
(register_member_orig_uid): Declare.
(walk_class_members): Declare.
* reflect.cc
(class_members_of): Make use of walk_class_members.
gcc/testsuite/ChangeLog:
* g++.dg/modules/reflect-class-order_a.C: New test.
* g++.dg/modules/reflect-class-order_b.C: New test.
* g++.dg/modules/reflect-clone-dtor_a.C: New test.
* g++.dg/modules/reflect-clone-dtor_b.C: New test.
Signed-off-by: Thomas Berger <[email protected]>
---
gcc/cp/module.cc | 28 +++
gcc/cp/name-lookup.cc | 112 +++++++++++
gcc/cp/name-lookup.h | 5 +
gcc/cp/reflect.cc | 181 ++++++------------
.../g++.dg/modules/reflect-class-order_a.C | 15 ++
.../g++.dg/modules/reflect-class-order_b.C | 24 +++
.../g++.dg/modules/reflect-clone-dtor_a.C | 10 +
.../g++.dg/modules/reflect-clone-dtor_b.C | 9 +
8 files changed, 265 insertions(+), 119 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/reflect-class-order_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/reflect-class-order_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/reflect-clone-dtor_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/reflect-clone-dtor_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ccbf124876d..127e1767ad8 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -8645,6 +8645,17 @@ trees_out::decl_value (tree decl, depset *dep)
dump (dumper::TREE) && dump ("Written decl:%d %C:%N", tag,
TREE_CODE (decl), decl);
+ /* Write the original DECL_UID for class members so importers can
+ restore declaration order when sorting members_of results.
+ While this is only required for reflection, we write this information
+ unconditionally to keep CMI files compatible across translation units with
+ and without reflection. */
+ if (streaming_p ()
+ && container
+ && TREE_CODE (container) == TYPE_DECL
+ && CLASS_TYPE_P (TREE_TYPE (container)))
+ u (DECL_UID (decl));
+
if (NAMESPACE_SCOPE_P (inner))
gcc_checking_assert (!dep == (VAR_OR_FUNCTION_DECL_P (inner)
&& DECL_LOCAL_DECL_P (inner)));
@@ -8781,6 +8792,13 @@ trees_in::decl_value ()
tree container = decl_container ();
unsigned tpl_levels = 0;
+ /* Determine if this is a class member. If true, the writer side has
+ placed the original DECL_UID after this DECL and we need to consume
+ it later */
+ bool is_class_member = (container
+ && TREE_CODE (container) == TYPE_DECL
+ && CLASS_TYPE_P (TREE_TYPE (container)));
+
/* If this is an imported temploid friend, get the owning decl its
attachment is determined by (or NULL_TREE otherwise). */
tree temploid_friend = NULL_TREE;
@@ -9147,6 +9165,16 @@ trees_in::decl_value ()
set_decl_tls_model (decl, model);
}
+ /* Read the original DECL_UID written by the export side for class
+ members. We only register the original UIDs if reflection is enabled
+ for the current TU. */
+ if (is_class_member)
+ {
+ unsigned orig_uid = u ();
+ if (is_new && flag_reflection)
+ register_member_orig_uid (existing, orig_uid);
+ }
+
if (!NAMESPACE_SCOPE_P (inner)
&& ((TREE_CODE (inner) == TYPE_DECL
&& !is_typedef
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index b7fc43391cf..4ba6fe408b9 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -9664,6 +9664,118 @@ dependent_local_decl_p (tree d)
return uses_template_parms (l->this_entity);
}
+/* Side map from imported class-member tree nodes to their original DECL_UIDs
+ as recorded in the exporting TU.
+ Used by walk_class_members to sort members in declaration order.
+ Populated during module stream-in when -freflection is active.
+ Intentionally never freed; valid for TU lifetime because lazy imports can
+ arrive at any time */
+static hash_map<tree, unsigned> *member_orig_uid_map;
+/* Register ORIG_UID as the original DECL_UID of DECL, which is a newly
+ imported class member. Called from module.cc during CMI stream-in. */
+
+void
+register_member_orig_uid (tree decl, unsigned orig_uid)
+{
+ if (!member_orig_uid_map)
+ member_orig_uid_map = new hash_map<tree, unsigned>;
+ member_orig_uid_map->put (decl, orig_uid);
+}
+
+/* Return the sort key for DECL when ordering class members in declaration
+ order. For imported members registered in the side map this is the
+ original DECL_UID from the exporting TU; for locally-defined members it
+ is DECL_UID (DECL), which already reflects declaration order. */
+
+static unsigned
+get_member_sort_uid (tree decl)
+{
+ tree scope = TYPE_NAME (DECL_CONTEXT (decl));
+ /* We only compare members, so at least make sure this as a scope */
+ gcc_assert (scope);
+
+ if (!modules_p ()
+ || (DECL_ARTIFICIAL (decl) && (TREE_CODE (decl) != TYPE_DECL))
+ || !DECL_LANG_SPECIFIC (scope) || !DECL_MODULE_IMPORT_P (scope))
+ return DECL_UID (decl);
+
+ /* FIXME: clones have currently no preserved UID */
+ if (DECL_CLONED_FUNCTION_P (decl))
+ return DECL_UID (decl);
+
+ gcc_assert (member_orig_uid_map);
+ unsigned *slot = member_orig_uid_map->get (decl);
+
+ gcc_assert (slot);
+ return *slot;
+}
+
+/* Comparison function for qsort: order class members by declaration-order
+ sort key. */
+
+static int
+class_member_cmp (const void *a, const void *b)
+{
+ const tree ta = *(const tree *)(a);
+ const tree tb = *(const tree *)(b);
+
+ auto const is_implicit = [] (tree t)
+ { return (TREE_CODE (t) == FUNCTION_DECL) && DECL_ARTIFICIAL (t); };
+
+ /* Implicit members are not streamed and therefore have no original DECL_UID.
+ As those are sorted at the end, they always compare "higher" against
+ non-implicit members */
+ if (is_implicit (ta) != is_implicit (tb))
+ {
+ if (is_implicit (ta))
+ return 1;
+
+ return -1;
+ }
+
+ unsigned ua = get_member_sort_uid (ta);
+ unsigned ub = get_member_sort_uid (tb);
+ if (ua < ub)
+ return -1;
+ if (ua > ub)
+ return 1;
+ return 0;
+}
+
+/* Walk all members of TYPE in declaration order, calling CALLBACK for
+ each member. Unlike direct iteration over TYPE_FIELDS this function
+ compensates for the "stat hack" reordering (TYPE_DECLs moved to the
+ end of TYPE_FIELDS) and, for imported types, restores the original
+ declaration order using UIDs recorded during CMI stream-in. */
+
+void
+walk_class_members (tree type, bool declare_implicits,
+ void (*callback) (tree, void *), void *data)
+{
+ if (modules_p ())
+ lazy_load_pendings (TYPE_NAME (type));
+ if (declare_implicits)
+ {
+ if (CLASSTYPE_LAZY_DEFAULT_CTOR (type))
+ lazily_declare_fn (sfk_constructor, type);
+ if (CLASSTYPE_LAZY_COPY_CTOR (type))
+ lazily_declare_fn (sfk_copy_constructor, type);
+ if (CLASSTYPE_LAZY_MOVE_CTOR (type))
+ lazily_declare_fn (sfk_move_constructor, type);
+ if (CLASSTYPE_LAZY_DESTRUCTOR (type))
+ lazily_declare_fn (sfk_destructor, type);
+ if (CLASSTYPE_LAZY_COPY_ASSIGN (type))
+ lazily_declare_fn (sfk_copy_assignment, type);
+ if (CLASSTYPE_LAZY_MOVE_ASSIGN (type))
+ lazily_declare_fn (sfk_move_assignment, type);
+ }
+ auto_vec<tree> members;
+ for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
+ members.safe_push (field);
+ members.qsort (class_member_cmp);
+ for (tree field : members)
+ callback (field, data);
+}
#include "gt-cp-name-lookup.h"
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 12d75b3437b..0617120c444 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -522,6 +522,11 @@ extern void maybe_pop_from_top_level (bool);
extern void push_using_decl_bindings (tree, tree);
extern void expose_existing_namespace (tree);
+extern void register_member_orig_uid (tree decl, unsigned orig_uid);
+extern void walk_class_members (tree type, bool declare_implicits,
+ void (*callback) (tree, void *data),
+ void *data);
+
/* Lower level interface for modules. */
extern tree *mergeable_namespace_slots (tree ns, tree name, bool is_attached,
tree *mvec);
diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc
index 09b0632a619..fd9944cfd97 100644
--- a/gcc/cp/reflect.cc
+++ b/gcc/cp/reflect.cc
@@ -6558,43 +6558,42 @@ class_members_of (location_t loc, const constexpr_ctx
*ctx, tree r,
tree actx, tree call, bool *non_constant_p,
tree *jump_target, enum metafn_code kind, tree fun)
{
- r = TYPE_MAIN_VARIANT (r);
- if (kind == METAFN_MEMBERS_OF)
- {
- if (modules_p ())
- lazy_load_pendings (TYPE_NAME (r));
- if (CLASSTYPE_LAZY_DEFAULT_CTOR (r))
- lazily_declare_fn (sfk_constructor, r);
- if (CLASSTYPE_LAZY_COPY_CTOR (r))
- lazily_declare_fn (sfk_copy_constructor, r);
- if (CLASSTYPE_LAZY_MOVE_CTOR (r))
- lazily_declare_fn (sfk_move_constructor, r);
- if (CLASSTYPE_LAZY_DESTRUCTOR (r))
- lazily_declare_fn (sfk_destructor, r);
- if (CLASSTYPE_LAZY_COPY_ASSIGN (r))
- lazily_declare_fn (sfk_copy_assignment, r);
- if (CLASSTYPE_LAZY_MOVE_ASSIGN (r))
- lazily_declare_fn (sfk_move_assignment, r);
- }
- auto_vec <tree, 8> implicitly_declared;
- vec<constructor_elt, va_gc> *elts = nullptr;
- for (tree field = TYPE_FIELDS (r); field; field = DECL_CHAIN (field))
+ struct walk_data_t
+ {
+ location_t loc;
+ const constexpr_ctx *ctx;
+ tree r;
+ tree actx;
+ tree call;
+ bool *non_constant_p;
+ tree *jump_target;
+ enum metafn_code kind;
+ tree fun;
+ vec<constructor_elt, va_gc> *elts = nullptr;
+ bool stop = false;
+ } walk_data = { loc, ctx, r, actx, call, non_constant_p, jump_target,
+ kind, fun };
+
+ const auto walker = [] (tree field, void *data_)
{
+ walk_data_t *d = static_cast<walk_data_t *> (data_);
+ if (d->stop)
+ return;
tree m = field;
if (TREE_CODE (field) == FIELD_DECL && DECL_ARTIFICIAL (field))
- continue; /* Ignore bases and the vptr. */
+ return; /* Ignore bases and the vptr. */
else if (DECL_SELF_REFERENCE_P (field))
- continue;
+ return;
else if (TREE_CODE (field) == TYPE_DECL)
m = TREE_TYPE (field);
else if (TREE_CODE (field) == FUNCTION_DECL)
{
/* Ignore cloned cdtors. */
if (DECL_CLONED_FUNCTION_P (field))
- continue;
+ return;
/* Ignore functions with unsatisfied constraints. */
if (!constraints_satisfied_p (field))
- continue;
+ return;
if (DECL_MAYBE_DELETED (field))
{
++function_depth;
@@ -6602,102 +6601,46 @@ class_members_of (location_t loc, const constexpr_ctx
*ctx, tree r,
--function_depth;
}
}
- if (members_of_representable_p (r, m))
+ if (!members_of_representable_p (d->r, m))
+ return;
+ if (d->kind == METAFN_STATIC_DATA_MEMBERS_OF
+ && eval_is_variable (m, REFLECT_UNDEF) != boolean_true_node)
+ return; /* For static_data_members_of only include is_variable. */
+ else if ((d->kind == METAFN_NONSTATIC_DATA_MEMBERS_OF
+ || d->kind == METAFN_HAS_INACCESSIBLE_NONSTATIC_DATA_MEMBERS)
+ && eval_is_nonstatic_data_member (m) != boolean_true_node)
+ return; /* For nonstatic_data_members_of only include
+ is_nonstatic_data_member. */
+ tree a = eval_is_accessible (d->loc, d->ctx, m, REFLECT_UNDEF,
+ d->actx, d->call, d->non_constant_p,
+ d->jump_target, d->fun);
+ if (*d->jump_target || *d->non_constant_p)
{
- if (kind == METAFN_STATIC_DATA_MEMBERS_OF
- && eval_is_variable (m, REFLECT_UNDEF) != boolean_true_node)
- continue; /* For static_data_members_of only include
- is_variable. */
- else if ((kind == METAFN_NONSTATIC_DATA_MEMBERS_OF
- || kind == METAFN_HAS_INACCESSIBLE_NONSTATIC_DATA_MEMBERS)
- && eval_is_nonstatic_data_member (m) != boolean_true_node)
- continue; /* For nonstatic_data_members_of only include
- is_nonstatic_data_member. */
- tree a = eval_is_accessible (loc, ctx, m, REFLECT_UNDEF, actx, call,
- non_constant_p, jump_target, fun);
- if (*jump_target || *non_constant_p)
- return nullptr;
- if (a == boolean_false_node)
- {
- if (kind == METAFN_HAS_INACCESSIBLE_NONSTATIC_DATA_MEMBERS
- && elts == nullptr)
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE, boolean_true_node);
- continue;
- }
- gcc_assert (a == boolean_true_node);
- if (kind == METAFN_MEMBERS_OF
- && TREE_CODE (m) == FUNCTION_DECL
- && DECL_ARTIFICIAL (m))
- {
- /* Implicitly-declared special members or operator== members
- appear after any user declared members. */
- implicitly_declared.safe_push (m);
- continue;
- }
- else if (kind == METAFN_HAS_INACCESSIBLE_NONSTATIC_DATA_MEMBERS)
- continue;
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
+ d->stop = true;
+ return;
}
- }
- /* TYPE_DECLs in TYPE_FIELDS come after other decls due to the "struct
- stat hack" (see finish_member_declaration), so for members_of the
- declaration order is not preserved. */
- if (kind == METAFN_MEMBERS_OF && elts)
- elts->qsort (members_cmp);
- if (kind == METAFN_MEMBERS_OF && !implicitly_declared.is_empty ())
- {
- gcc_assert (implicitly_declared.length () <= 8);
- for (tree m : implicitly_declared)
- if (default_ctor_p (m))
- {
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- break;
- }
- for (tree m : implicitly_declared)
- if (DECL_COPY_CONSTRUCTOR_P (m))
- {
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- break;
- }
- for (tree m : implicitly_declared)
- if (special_function_p (m) == sfk_copy_assignment)
- {
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- break;
- }
- for (tree m : implicitly_declared)
- if (DECL_MOVE_CONSTRUCTOR_P (m))
- {
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- break;
- }
- for (tree m : implicitly_declared)
- if (special_function_p (m) == sfk_move_assignment)
- {
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- break;
- }
- for (tree m : implicitly_declared)
- if (DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (m))
- {
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- break;
- }
- for (tree m : implicitly_declared)
- if (DECL_OVERLOADED_OPERATOR_IS (m, EQ_EXPR))
- {
- CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE,
- get_reflection_raw (loc, m));
- break;
- }
- }
+ if (a == boolean_false_node)
+ {
+ if (d->kind == METAFN_HAS_INACCESSIBLE_NONSTATIC_DATA_MEMBERS
+ && d->elts == nullptr)
+ CONSTRUCTOR_APPEND_ELT (d->elts, NULL_TREE, boolean_true_node);
+ return;
+ }
+ gcc_assert (a == boolean_true_node);
+ if (d->kind == METAFN_HAS_INACCESSIBLE_NONSTATIC_DATA_MEMBERS)
+ return;
+ CONSTRUCTOR_APPEND_ELT (d->elts, NULL_TREE,
+ get_reflection_raw (d->loc, m));
+ };
+
+ /* walk_class_members sorts TYPE_FIELDS by original declaration UID,
+ compensating for the stat-hack reordering and for stream-order UIDs
+ in imported types. No final qsort is needed. */
+ walk_class_members (r, kind == METAFN_MEMBERS_OF, walker, &walk_data);
+ if (walk_data.stop)
+ return nullptr;
+
+ vec<constructor_elt, va_gc> *elts = walk_data.elts;
return elts;
}
diff --git a/gcc/testsuite/g++.dg/modules/reflect-class-order_a.C
b/gcc/testsuite/g++.dg/modules/reflect-class-order_a.C
new file mode 100644
index 00000000000..083ce8582e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/reflect-class-order_a.C
@@ -0,0 +1,15 @@
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fmodules -freflection" }
+// { dg-module-cmi reflect_class_order }
+
+export module reflect_class_order;
+
+export struct A
+{
+ int a;
+ typedef int b;
+ struct C {};
+ int foo () { return 42; }
+ using D = C;
+ long e;
+};
diff --git a/gcc/testsuite/g++.dg/modules/reflect-class-order_b.C
b/gcc/testsuite/g++.dg/modules/reflect-class-order_b.C
new file mode 100644
index 00000000000..5ad513afa1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/reflect-class-order_b.C
@@ -0,0 +1,24 @@
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fmodules -freflection" }
+
+#include <meta>
+
+import reflect_class_order;
+
+/* Class members must be returned in declaration order
+ ([meta.reflection.member.queries] para. 4):
+ "Reflections of class members [...] appear in the order in which
+ they are declared."
+ Synthesized special members follow after ([Note 2]).
+
+ Expected order for A: a, b, C, foo, D, e. */
+
+constexpr auto unchecked = std::meta::access_context::unchecked ();
+
+static_assert (identifier_of (members_of (^^A, unchecked)[0]) == "a");
+static_assert (identifier_of (members_of (^^A, unchecked)[1]) == "b");
+static_assert (identifier_of (members_of (^^A, unchecked)[2]) == "C");
+static_assert (identifier_of (members_of (^^A, unchecked)[3]) == "foo");
+static_assert (identifier_of (members_of (^^A, unchecked)[4]) == "D");
+static_assert (identifier_of (members_of (^^A, unchecked)[5]) == "e");
+static_assert (members_of (^^A, unchecked).size () == 12); // 6 defined +
ctor, dtor, operators
diff --git a/gcc/testsuite/g++.dg/modules/reflect-clone-dtor_a.C
b/gcc/testsuite/g++.dg/modules/reflect-clone-dtor_a.C
new file mode 100644
index 00000000000..f56d7ff8c26
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/reflect-clone-dtor_a.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fmodules -freflection" }
+// { dg-module-cmi reflect_clone_dtor }
+
+export module reflect_clone_dtor;
+
+export struct B
+{
+ virtual ~B ();
+};
diff --git a/gcc/testsuite/g++.dg/modules/reflect-clone-dtor_b.C
b/gcc/testsuite/g++.dg/modules/reflect-clone-dtor_b.C
new file mode 100644
index 00000000000..35dc6accb0a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/reflect-clone-dtor_b.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fmodules -freflection" }
+
+#include <meta>
+import reflect_clone_dtor;
+
+/* make sure we don't break with cloned functions */
+static_assert (
+ members_of (^^B, std::meta::access_context::unchecked ()).size () == 4);
--
2.52.0
signature.asc
Description: This is a digitally signed message part.
