On 6/27/25 5:03 AM, Nathaniel Shead wrote:
On Wed, Jun 25, 2025 at 01:05:39PM -0400, Jason Merrill wrote:
On 5/21/25 10:14 PM, Nathaniel Shead wrote:
This patch isn't currently necessary with how I've currently done the
follow-up patches, but is needed for avoiding any potential issues in
the future with DECL_CONTEXT'ful types getting created in the compiler
with no names on the fields. (For instance, this change would make much
of r15-7342-gd3627c78be116e unnecessary.)
It does take up another flag though in the frontend though. Another
possible approach would be to instead do a walk through all the fields
first to see if this is the target of a DECL_BIT_FIELD_REPRESENTATIVE;
thoughts? Or would you prefer to skip this patch entirely?
It seems like the only way to reach such a FIELD_DECL is through
DECL_BIT_FIELD_REPRESENTATIVE, so we ought to be able to use that without
adding another walk?
Fair enough, how does this look instead? Bootstrapped and tested (so
far just modules.exp) on x86_64-pc-linux-gnu, OK for trunk if full
regtest passes?
OK.
-- >8 --
Subject: [PATCH] c++/modules: Make bitfield storage unit detection more robust
Modules streaming needs to handle these differently from other unnamed
FIELD_DECLs that are streamed for internal RECORD_DECLs, and there
doesn't seem to be a good way to detect this case otherwise.
This matters only to allow for compiler-generated type definitions that
build FIELD_DECLs with no name, as otherwise they get confused.
Currently the only such types left I hadn't earlier fixed by giving
names to are contextless, for which we have an early check to mark their
fields as MK_unique anyway, but there may be other cases in the future.
gcc/cp/ChangeLog:
* module.cc (trees_out::walking_bit_field_unit): New flag.
(trees_out::trees_out): Initialize it.
(trees_out::core_vals): Set it.
(trees_out::get_merge_kind): Use it, move previous ad-hoc check
into assertion.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/module.cc | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 42a1b83e164..7bc3e576293 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3097,6 +3097,9 @@ private:
unsigned section;
bool writing_local_entities; /* Whether we might walk into a TU-local
entity we need to emit placeholders for. */
+ bool walking_bit_field_unit; /* Whether we're walking the underlying
+ storage for a bit field. There's no other
+ great way to detect this. */
#if CHECKING_P
int importedness; /* Checker that imports not occurring
inappropriately. +ve imports ok,
@@ -3263,7 +3266,7 @@ trees_out::trees_out (allocator *mem, module_state *state,
depset::hash &deps,
unsigned section)
:parent (mem), state (state), tree_map (500),
dep_hash (&deps), ref_num (0), section (section),
- writing_local_entities (false)
+ writing_local_entities (false), walking_bit_field_unit (false)
{
#if CHECKING_P
importedness = 0;
@@ -6512,7 +6515,10 @@ trees_out::core_vals (tree t)
case FIELD_DECL:
WT (t->field_decl.offset);
WT (t->field_decl.bit_field_type);
- WT (t->field_decl.qualifier); /* bitfield unit. */
+ {
+ auto ovr = make_temp_override (walking_bit_field_unit, true);
+ WT (t->field_decl.qualifier); /* bitfield unit. */
+ }
WT (t->field_decl.bit_offset);
WT (t->field_decl.fcontext);
WT (t->decl_common.initial);
@@ -11268,15 +11274,16 @@ trees_out::get_merge_kind (tree decl, depset *dep)
return MK_named;
}
- if (!DECL_NAME (decl)
- && !RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
- && !DECL_BIT_FIELD_REPRESENTATIVE (decl))
+ if (walking_bit_field_unit)
{
/* The underlying storage unit for a bitfield. We do not
need to dedup it, because it's only reachable through
the bitfields it represents. And those are deduped. */
// FIXME: Is that assertion correct -- do we ever fish it
// out and put it in an expr?
+ gcc_checking_assert (!DECL_NAME (decl)
+ && !RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
+ && !DECL_BIT_FIELD_REPRESENTATIVE (decl));
gcc_checking_assert ((TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE
? TREE_CODE (TREE_TYPE (TREE_TYPE (decl)))
: TREE_CODE (TREE_TYPE (decl)))
