Skip to content

project_struct_literal reports a successful redirect and changes nothing, so an inlined field accessor keeps its aggregate alive #1963

Description

@gfx

Hand-off from perf-json-canada. That branch hit this while restoring a lost
constant-object globalization and routed around it, so nothing depends on the
fix — this records what is known so the next attempt starts from evidence.

The defect

project_struct_literal
folds Struct { f: v, .. }.f into v when the receiver is the literal itself
and every non-projected field is pure. On the body below it reports success
and changes nothing
: redirect_expr returns true three times, and the
projection survives every dumped round of the pass that made the call.

Reproduction

wado-compiler/tests/fixtures/opt_value_copy_string_literal_pattern.wado is the
shape (a string-literal pattern, whose String^Eq::eq inlines into the matcher):

$ WADO_DUMP_PASS_AFTER=nir/const_fold \
    wado compile -O2 --world test -o /tmp/x.wasm \
    wado-compiler/tests/fixtures/opt_value_copy_string_literal_pattern.wado

At 90dd0fbf8f, over the three dumped rounds:

form occurrences
String { repr: packed"alpha", used: 5 }.repr 3
&packed"alpha" (what the fold should leave) 0

The fixture itself passes today — the branch added a rewrite inside
const_object_globalization that reaches the same shape over the arena — so this
is a silent miss, not a red test.

What the rule sees

Instrumenting the rule (probe removed again) at the same commit, on the three
match arms:

PSL ok=true parent=Some("Unary { op: Ref, expr: Expr(expr71) }")  proj=Expr(expr71)
PSL ok=true parent=Some("Unary { op: Ref, expr: Expr(expr728) }") proj=Expr(expr728)
PSL ok=true parent=Some("Unary { op: Ref, expr: Expr(expr854) }") proj=Expr(expr854)

Every precondition holds — the receiver is a StructLiteral, the one
non-projected field (used) is a pooled constant, redirect_expr finds a parent
and reports the operand replaced — and the parent it names is exactly the & the
fold wants to end up wrapping the packed array. The printed operand is already
the fold's target, so the write itself happened.

Leading hypothesis

redirect_expr writes
through parent_of(NodeRef::Expr(id)), the engine's parent map, and returns
true on replace_operand_to succeeding. If that map still names a node the
tree has since replaced — the surrounding … as &Array<u8> cast is rewritten in
the same walk — the write lands on an orphan: real, reported, and unreachable.
That would explain a true with no observable effect, but it is inference from
the three facts above, not something I confirmed by reading the map.

Worth ruling out first: whether the rule runs on a body that is later discarded,
and whether ExprShape::None's post-order (walk_children then
project_struct_literal, at
const_folding.rs#L905)
can leave the parent stale by the time the rule fires.

Why it is worth fixing

The rule's own doc names the payoff — removing the otherwise-dead struct.new
after copy-prop substitutes a single-field literal into its sole .field use.
The shape is what any inlined field accessor leaves behind:

fn repr(&self) -> &Array<u8> { self.repr }

Once that inlines at a literal receiver, the caller holds
Struct { .. }.field and, unless this fold lands, keeps the whole aggregate
alive to read one field out of it. On the branch that found it, the same shape
cost a string literal its globalization: every match arm rebuilt the literal's
backing array, which is what
opt_value_copy_string_literal_pattern's wir_not_expect was written to catch.

A fix here would also make the branch's
const_object_globalization::deref_const_field_borrows redundant — it exists
only because this fold does not stick, and its doc comment says so.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions