Skip to content
Open
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
8fd6937
v3: lower value-context match/if block operands with `!`/`?` propagat…
medvednikov Aug 2, 2026
2e31fa4
v3: route specialized infix operands through value lowering (#28000)
medvednikov Aug 2, 2026
5e2e8de
v3: route left-shift and sum-cast match/if operands through value low…
medvednikov Aug 2, 2026
800519e
v3: materialize match/if operands before cast dispatch and for `is` s…
medvednikov Aug 2, 2026
d26ae44
v3: preserve infix operand order and lower address-of match/if operan…
medvednikov Aug 2, 2026
faa61a1
v3: preserve evaluation order for match/if left-shift and index opera…
medvednikov Aug 2, 2026
319d914
v3: route gated-index match/if operands through value lowering (#28000)
medvednikov Aug 2, 2026
20f00b3
v3: lower propagating match/if values in range bounds (#28000)
medvednikov Aug 2, 2026
773d820
v3: stabilize range low bound before hoisting high bound in membershi…
medvednikov Aug 2, 2026
b7bb6a1
v3: route non-range membership and for-in containers through value lo…
medvednikov Aug 2, 2026
f378044
v3: route map-index bases and preserve needle order for match/if memb…
medvednikov Aug 2, 2026
b345126
v3: preserve array-append LHS order and lower match/if needles in con…
medvednikov Aug 2, 2026
98d026f
v3: evaluate map-membership key before hoisting the container (#28000)
medvednikov Aug 2, 2026
fd5233b
v3: lower push-many match/if append RHS as a value (#28000)
medvednikov Aug 2, 2026
595d597
v3: materialize value match/if method receivers before builtin dispat…
medvednikov Aug 2, 2026
7986db1
v3: materialize value match/if method arguments before builtin dispat…
medvednikov Aug 2, 2026
cdacd5e
v3: materialize value match/if channel-send values and order call ope…
medvednikov Aug 2, 2026
44279af
v3: preserve lvalue receivers and stabilize channel targets for match…
medvednikov Aug 2, 2026
391aee8
v3: order plain-call operands, preserve mut arg lvalues, lower array-…
medvednikov Aug 2, 2026
824b564
v3: spill non-mut lvalue args by value and order array-init len befor…
medvednikov Aug 2, 2026
f021bb9
v3: detect nested value branches when ordering call operands (#28000)
medvednikov Aug 2, 2026
bef373f
v3: detect nested value branches in append/index/array-init operand o…
medvednikov Aug 2, 2026
97e69f3
v3: detect nested value branches in infix/shift/range/membership/chan…
medvednikov Aug 2, 2026
4743d9e
v3: spill by-value method receivers and rvalue channel targets before…
medvednikov Aug 2, 2026
40591bf
v3: spill composite rvalue channel targets rooted in a call before th…
medvednikov Aug 2, 2026
49fa317
v3: snapshot value-bearing operands before a hoisted branch prelude c…
medvednikov Aug 2, 2026
aabe76a
v3: materialize branch-produced call callees and type-pattern members…
medvednikov Aug 2, 2026
ee812dc
v3: snapshot array-membership needles before a value-branch container…
medvednikov Aug 2, 2026
531e6c8
v3: snapshot range low bound and map base before a hoisted branch pre…
medvednikov Aug 2, 2026
296f7e0
v3: lower channel send targets as values and snapshot stable targets …
medvednikov Aug 2, 2026
8fc64d7
v3: stabilize non-method runtime callees before a hoisted branch argu…
medvednikov Aug 2, 2026
de6b95c
v3: snapshot gated-index bases and mutable-lvalue index components be…
medvednikov Aug 2, 2026
af10faf
v3: snapshot function-field callees instead of only their receiver (#…
medvednikov Aug 2, 2026
88c77dc
v3: capture mutable receiver storage and order struct fields before b…
medvednikov Aug 2, 2026
ef7a5a4
v3: capture the optional append target address before lowering a bran…
medvednikov Aug 2, 2026
d87e0bf
v3: preserve source order across select send cases (#28000)
medvednikov Aug 2, 2026
541fe3d
v3: snapshot nonconstant select operands and include branch-producing…
medvednikov Aug 2, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,808 changes: 1,808 additions & 0 deletions vlib/v3/tests/match_as_if_expr_value_propagation_codegen_test.v

Large diffs are not rendered by default.

54 changes: 48 additions & 6 deletions vlib/v3/transform/array.v
Original file line number Diff line number Diff line change
Expand Up @@ -388,14 +388,37 @@ fn (mut t Transformer) lower_array_init_to_runtime(id flat.NodeId, node flat.Nod
mut cap_expr := t.make_int_literal(0)
mut init_expr := flat.empty_node
mut init_expr_id := flat.empty_node
// Source (child) position of the last `len`/`cap` field whose value hoists a value branch
// — directly or nested inside a compound field value (`cap: 1 + (match ...)`) — so an
// earlier side-effecting `len`/`cap` field can be stabilized before that field hoists its
// materialization prelude, preserving field evaluation order (both are evaluated into
// `new_call` below; `init` is per-element in the loop body).
mut last_lencap_branch := -1
for i in 0 .. node.children_count {
child := t.a.child_node(&node, i)
if child.kind == .field_init && child.children_count > 0 && child.value in ['len', 'cap'] {
if t.operand_hoists_value_branch(t.a.child(child, 0)) {
last_lencap_branch = i
}
}
}
for i in 0 .. node.children_count {
child := t.a.child_node(&node, i)
if child.kind == .field_init && child.children_count > 0 {
if child.value == 'len' {
val := t.transform_expr(t.a.child(child, 0))
// Typed value lowering so a value `match`/`if` len field (e.g.
// `[]int{len: match node { ... lower(node)! ... }}`) is materialized as a
// value instead of lowering its propagating arm in a statement context.
mut val := t.transform_expr_for_type(t.a.child(child, 0), 'int')
if i < last_lencap_branch && t.operand_needs_ordering_snapshot(val) {
val = t.snapshot_transformed_expr_for_reuse(val, 'int', 'arr_len')
}
len_expr = val
} else if child.value == 'cap' {
val := t.transform_expr(t.a.child(child, 0))
mut val := t.transform_expr_for_type(t.a.child(child, 0), 'int')
if i < last_lencap_branch && t.operand_needs_ordering_snapshot(val) {
val = t.snapshot_transformed_expr_for_reuse(val, 'int', 'arr_cap')
}
cap_expr = val
Comment on lines 417 to 422

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stabilize len before hoisting a cap match

When len has side effects and cap is a value match/if with propagation, transforming cap queues its materialization before new_call, while the transformed len expression remains inline in that call. Thus []int{len: trace_len(), cap: match node { ... trace_cap()! ... }} evaluates trace_cap before trace_len, reversing field evaluation order. Stabilize earlier fields before a later field hoists statements and add an ordering regression.

AGENTS.md reference: AGENTS.md:L657-L658

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 824b564. lower_array_init_to_runtime now finds the last len/cap field whose value is a value branch, and stabilizes an earlier side-effecting len/cap field into a temp before that field hoists its materialization prelude — both are evaluated into new_call, so this preserves their source order. (init is per-element in the loop body and already ordered after.)

Regression test select_value_array_init_cap_order: []int{len: tr.tlen(), cap: match node { First { tr.tcap_first(node)! } … }} where both record into an order trace → 212 (len=1 then cap=2). On HEAD it is 221 (tcap ran before tlen). No new regressions; review_cgen passes.

} else if child.value == 'init' {
init_expr_id = t.a.child(child, 0)
Expand Down Expand Up @@ -437,7 +460,8 @@ fn (mut t Transformer) lower_array_init_to_runtime(id flat.NodeId, node flat.Nod
saved_pending := t.pending_stmts.clone()
t.pending_stmts.clear()
indexed_init := t.substitute_ident_expr(init_expr_id, 'index', t.make_ident(idx_name))
init_expr = t.transform_expr(indexed_init)
// Typed value lowering so a value `match`/`if` init field is materialized as a value.
init_expr = t.transform_expr_for_type(indexed_init, elem_type)
init_pending := t.pending_stmts.clone()
t.pending_stmts = saved_pending
for stmt in init_pending {
Expand Down Expand Up @@ -1167,7 +1191,15 @@ fn (mut t Transformer) try_lower_array_append_stmt(id flat.NodeId) ?[]flat.NodeI
}

mut result := []flat.NodeId{}
lhs := t.transform_lvalue(lhs_id)
mut lhs := t.transform_lvalue(lhs_id)
// For an append whose RHS hoists a value `match`/`if` prelude — directly or nested inside
// a compound RHS (`arrays[next(mut trace)] << wrap(match ...)`) — stabilize the LHS
// lvalue's dynamic base/index components into temps first — without spilling the mutated
// array value — so a side-effecting index (e.g. `arrays[next(mut trace)] << (match ...)`)
// evaluates before the RHS prelude below, preserving source order.
if t.operand_hoists_value_branch(rhs_id) {
lhs = t.stabilize_transformed_lvalue_for_reuse(lhs)
}
t.drain_pending(mut result)
mut rhs := flat.empty_node
if !push_many {
Expand Down Expand Up @@ -1196,7 +1228,12 @@ fn (mut t Transformer) try_lower_array_append_stmt(id flat.NodeId) ?[]flat.NodeI
}
}
} else {
rhs = t.transform_expr(rhs_id)
// Route a value `match`/`if` push-many RHS (an array-producing match, e.g.
// `out << (match node { First { values_first(node)! } ... })`) through value
// lowering so its propagating arm tail is materialized as a value instead of in a
// value-less statement context. `transform_value_operand` is a no-op for the
// common non-branch push-many operands.
rhs = t.transform_value_operand(rhs_id)
}
if !push_many {
rhs = t.coerce_transformed_expr_to_type(rhs, rhs_id, elem_type)
Expand Down Expand Up @@ -1395,7 +1432,12 @@ fn (mut t Transformer) try_lower_optional_array_append_stmt(_node flat.Node, lhs
}
}
} else {
rhs = t.transform_expr(rhs_id)
// Route a value `match`/`if` push-many RHS (an array-producing match, e.g.
// `out << (match node { First { values_first(node)! } ... })`) through value
// lowering so its propagating arm tail is materialized as a value instead of in a
// value-less statement context. `transform_value_operand` is a no-op for the
// common non-branch push-many operands.
rhs = t.transform_value_operand(rhs_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Capture the optional append target before lowering the RHS

In the optional-LHS path, a push-many branch such as holder.values? << (match node { First { holder.replace(node)! } ... }) can reassign holder.values while materializing the RHS. The guard above evaluates the original selector, but lhs_addr later reuses the inline source after the RHS prelude, so the append targets the replacement optional rather than the storage selected before evaluating the RHS. Snapshot the optional source storage/address before lowering the branch and add an ordering regression.

AGENTS.md reference: AGENTS.md:L657-L658

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ef7a5a4. The optional value-array address is now captured up front when the RHS hoists a value branch, instead of re-reading the inline source at lhs_addr after the RHS prelude:

if t.operand_hoists_value_branch(rhs_id) {
    addr := t.runtime_addr(t.make_selector(source, 'value', array_type), array_type)
    captured_lhs_addr = t.stable_transformed_expr_for_reuse(addr, '&${array_type}', 'opt_append_target')
    ...
}

So the append target is selected before the RHS, consistent with the !source.ok guard above it.

One transparency note on the regression. I built the branch and compared v3 against mainline v across several shapes of this exact scenario — simple field (holder.values or {...} << (match ... { holder.replace()! } ...)), a side-effecting source base (ctx.get().values? << ...), an indexed source base (holders[ctx.idx].values? << ... where the arm bumps ctx.idx), and an arm that sets the source to none. In every case v3 already produced the same result as mainline (e.g. the simple field gives [100, 200, 7, 8] on both), so the append was already targeting the same storage mainline uses. This change keeps that parity: for the cases that reach this path the value-array slot address is identical whether captured before or after the RHS (source for a dynamic base is already stabilized to a single evaluation), so it's a robustness/ordering fix (selecting the target in source order, matching the guard) rather than a value change — I couldn't construct a case where the observable result diverges.

The added regression select_value_optional_append_reassign (holder.values or { ... } << (match node { First { holder.replace_first(node)! } ... }) -> [100,200,7,8] -> 500) is therefore path coverage for the reassigning-RHS push-many optional append rather than a strictly load-bearing ordering test — I flag that honestly. If you have a specific reproducer where v3 diverges from mainline here, I'll add a targeted load-bearing regression and confirm this fix addresses it. No new regressions; review_cgen passes.

}
if !push_many {
rhs = t.coerce_transformed_expr_to_type(rhs, rhs_id, elem_type)
Expand Down
216 changes: 196 additions & 20 deletions vlib/v3/transform/expr.v
Original file line number Diff line number Diff line change
Expand Up @@ -1875,11 +1875,38 @@ fn (mut t Transformer) transform_in_expr(id flat.NodeId, node flat.Node) flat.No
if rhs.kind == .range {
// x in low..high -> x >= low && x < high
if rhs.children_count >= 2 {
new_lhs := t.stable_expr_for_reuse(lhs_id)
// Route value `match`/`if` operands (the tested value and the range bounds)
// through value lowering so a propagating branch tail is materialized as a
// value instead of in a value-less statement context, e.g.
// `x in (match node { ... lower(node)! ... }) .. 10`. `transform_value_operand`
// is a no-op for the common non-branch operands.
low_id := t.a.children[rhs.children_start]
high_id := t.a.children[rhs.children_start + 1]
new_low := t.transform_expr(low_id)
new_high := t.transform_expr(high_id)
// The tested value is evaluated first; if either bound hoists a value branch whose
// prelude could mutate it, snapshot its source-order value before that prelude.
bound_hoists := t.operand_hoists_value_branch(low_id)
|| t.operand_hoists_value_branch(high_id)
new_lhs := if t.is_value_match_or_if_operand(lhs_id) {
t.transform_value_operand(lhs_id)
Comment on lines +1889 to +1890

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Lower type-pattern membership subjects as values

This value-aware membership handling only covers the range branch; for (match node { First { make_foo(node)! } else { make_bar(node)! } }) in [Foo1, Foo3], the array-literal path returns through lower_type_pattern_membership, which still sends the subject through stable_expr_for_reuse and therefore plain transform_expr. The propagating match arms can consequently produce the same empty expression; route the type-pattern subject through typed value lowering and add a regression.

AGENTS.md reference: AGENTS.md:L657-L658

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aabe76a. lower_type_pattern_membership now routes a value-branch subject through typed value lowering — t.transform_expr_for_type(lhs_id, sum_name) then stable_transformed_expr_for_reuse — instead of stable_expr_for_reuse/transform_expr, so a propagating match subject is materialized as a typed value rather than emitting an empty expression. This mirrors the range-bound and membership-needle handling.

One thing to note: I couldn't add a runtime regression because this path isn't reachable in v3 yet — the checker rejects x in [Type, ...] before transform runs. Even the plainest form fails:

struct Foo1 {}
struct Foo3 {}
type Foo = Foo1 | Foo3
f := Foo(Foo1{})
println(f in [Foo1, Foo3])
// v3: error: `Foo1` must be initialized   (mainline `v` compiles this and prints `true`)

So the fix is applied for correctness/consistency and will be exercised once the v3 checker admits type-pattern membership, but a value-propagation regression can't compile under v3 today. I can fold in the checker support separately if you'd like it wired up end to end. No new regressions; review_cgen passes.

} else if bound_hoists && t.operand_needs_ordering_snapshot(lhs_id) {
t.snapshot_expr_for_reuse(lhs_id)
} else {
t.stable_expr_for_reuse(lhs_id)
Comment on lines +1889 to +1894

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route non-range membership containers through value lowering

When the RHS container is itself a value match/if, such as needle in (match node { First { get_values(node)! } else { get_other_values(node)! } }), this routing is skipped because it only runs in the .range branch. The dynamic/fixed-array and string paths still use plain transform_expr, while stable_array_expr_for_membership does the same for semantic array membership, so the propagating arm tail is again lowered in statement context and can emit an empty expression. Route these container operands through value-aware lowering and add a regression case.

AGENTS.md reference: AGENTS.md:L657-L658

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in b7bb6a1. All non-range membership container lowerings now go through transform_value_operand:

  • The inline branches in transform_in_expr (dynamic array, fixed array, string, unknown-ident, and the fallback in_expr) that lowered the RHS container with plain transform_expr.
  • stable_array_expr_for_membership (semantic array membership), and lower_map_membership_expr (map membership) — both used plain transform_expr / stable_expr_for_reuse on the container.

transform_value_operand is a no-op for the common non-branch containers (idents/selectors/calls). (lower_const_string_array_membership_expr is unchanged — it only matches a const string-array literal base, never a match.)

Regression test select_value_membership_container: 20 in (match node { First { make_values_first(node)! } ... })true. It fails to compile on HEAD without the fix (empty expression). No new regressions; review_cgen (membership coverage) passes.

}
// If the high bound hoists a value branch — directly or nested inside a compound
// bound (`.. (1 + (match ...))`) — its materialization below queues prelude
// statements; snapshot a value-bearing low bound first so it evaluates before them,
// preserving low-before-high order, e.g.
// `x in low_with_effect() .. (match node { ... high_with_effect()! ... })`.
// A value-branch low is materialized in order by `transform_value_operand`.
new_low := if !t.is_value_match_or_if_operand(low_id)
&& t.operand_hoists_value_branch(high_id)
&& t.operand_needs_ordering_snapshot(low_id) {
t.snapshot_expr_for_reuse(low_id)
} else {
t.transform_value_operand(low_id)
}
new_high := t.transform_value_operand(high_id)

ge_cmp := t.make_infix(.ge, new_lhs, new_low)
lt_cmp := t.make_infix(.lt, new_lhs, new_high)
Expand Down Expand Up @@ -1922,21 +1949,29 @@ fn (mut t Transformer) transform_in_expr(id flat.NodeId, node flat.Node) flat.No
result = lowered
} else {
// dynamic array membership -> array_contains_int/string(arr, val)
mut new_rhs := t.transform_expr(rhs_id)
if rhs_is_ptr_array {
new_rhs = t.make_prefix(.mul, new_rhs)
}
// (value-aware so a `match`/`if` container is materialized as a value)
mut elem := if clean_rhs_type.starts_with('[]') { clean_rhs_type[2..] } else { '' }
if elem.len == 0 {
elem = t.node_type(lhs_id)
}
new_lhs := t.transform_expr_for_type(lhs_id, elem)
// Evaluate the needle before materializing a value-branch container so a
// side-effecting needle precedes the container's hoisted prelude.
new_lhs := if t.is_value_match_or_if_operand(rhs_id) {
t.snapshot_transformed_expr_for_reuse(t.transform_expr_for_type(lhs_id, elem),
elem, 'in_lhs')
} else {
t.transform_expr_for_type(lhs_id, elem)
Comment on lines +1959 to +1963

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Lower value-match needles before membership shortcuts

This value-aware needle lowering is never reached for membership in a constant string array because lower_const_string_array_membership_expr returns earlier at line 1936, and that helper still lowers its needle with plain transform_expr at line 2113. Consequently, (match node { First { get_first(node)! } else { get_second(node)! } }) in allowed_words still lowers propagating branch tails in statement context when allowed_words is a constant string array and can emit the empty expression this change is intended to eliminate; route the needle through typed value lowering in the shortcut and add a regression case.

AGENTS.md reference: AGENTS.md:L657-L658

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in b345126. lower_const_string_array_membership_expr now lowers its needle with transform_expr_for_type(needle_id, 'string') (typed value lowering — the container is a string array), so a value match/if needle materializes its propagating arm as a value instead of in a value-less statement context. This runs before the value-aware needle path in transform_in_expr, exactly as you noted.

Regression test select_value_const_membership: (match node { First { get_first(node)! } … }) in allowed_words (a const []string) → true. It fails to compile on HEAD without the fix (error: expected expression). No new regressions; review_cgen passes.

}
mut new_rhs := t.transform_value_operand(rhs_id)
if rhs_is_ptr_array {
new_rhs = t.make_prefix(.mul, new_rhs)
}
fn_name := array_contains_fn_name(elem)
result = t.make_call_typed(fn_name, arr2(new_rhs, new_lhs), 'bool')
}
} else if rhs.kind in [.ident, .selector] && (rhs_type.len == 0 || rhs_type == 'unknown') {
new_lhs := t.transform_expr(lhs_id)
new_rhs := t.transform_expr(rhs_id)
new_rhs := t.transform_value_operand(rhs_id)
mut elem := t.node_type(lhs_id)
lhs := t.a.nodes[int(lhs_id)]
if elem.len == 0 && lhs.kind == .selector {
Expand All @@ -1954,16 +1989,29 @@ fn (mut t Transformer) transform_in_expr(id flat.NodeId, node flat.Node) flat.No
result = lowered
} else {
// fixed array membership -> fixed_array_contains_int/string(arr, len, val)
new_lhs := t.transform_expr(lhs_id)
new_rhs := t.transform_expr(rhs_id)
// stabilize a side-effecting needle before a value-branch container hoists
new_lhs := if t.is_value_match_or_if_operand(rhs_id) {
t.snapshot_expr_for_reuse(lhs_id)
} else {
t.transform_expr(lhs_id)
}
new_rhs := t.transform_value_operand(rhs_id)
elem := fixed_array_elem_type(clean_rhs_type)
fn_name := fixed_array_contains_fn_name(elem)
len_expr := t.make_fixed_array_len_expr(clean_rhs_type)
result = t.make_call_typed(fn_name, arr3(new_rhs, len_expr, new_lhs), 'bool')
}
} else if clean_rhs_type == 'string' {
new_lhs := t.transform_expr(lhs_id)
new_rhs := t.transform_expr(rhs_id)
// If the container hoists a value branch — directly or nested inside a compound
// container (`... in wrap(match ...)`) — its materialization below hoists a
// prelude; stabilize a side-effecting needle first so it evaluates before it,
// e.g. `tr.needle() in (match n { First { tr.text_first(n)! } ... })`.
new_lhs := if t.operand_hoists_value_branch(rhs_id) {
t.snapshot_expr_for_reuse(lhs_id)
} else {
t.transform_expr(lhs_id)
}
new_rhs := t.transform_value_operand(rhs_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve LHS order when hoisting membership containers

When a string membership container is a value match/if and the needle has side effects, transform_value_operand(rhs_id) hoists the container's propagation prelude into pending_stmts, while new_lhs remains inline in the final helper call. For example, tr.needle() in (match n { First { tr.text_first(n)! } else { tr.text_second(n)! } }) executes the RHS text function before needle, reversing source order. Stabilize the LHS before materializing such a RHS and add an ordering regression.

AGENTS.md reference: AGENTS.md:L657-L658

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f378044. In the inline membership branches (dynamic-array, fixed-array, string, and the unknown fallback), a side-effecting needle is now stabilized to a temp before a value-branch container is materialized, so it evaluates first — preserving needle-before-container source order. For the dynamic-array branch (which transformed the container first) the needle is now evaluated before the container; the others simply gate new_lhs on is_value_match_or_if_operand(rhs_id). The semantic-array path (lower_array_membership_expr, receiver_first=false) already stabilized the needle before the container with drain_pending between, so it was unaffected.

Regression test select_value_string_membership_order: tr.needle_str() in (match node { First { tr.text_first(node)! } ... }) where both sides record into an order trace, encoded as 500 + order[0]*10 + order[1]512 ("lo" in "hello" true, order needle=1 then container=2). It fails on HEAD (produces 521 — reversed). No new regressions; review_cgen passes.

fn_name := if t.node_type(lhs_id) in ['u8', 'byte'] {
'string__contains_u8'
} else {
Expand All @@ -1977,8 +2025,13 @@ fn (mut t Transformer) transform_in_expr(id flat.NodeId, node flat.Node) flat.No
} else {
// Unknown containment is kept as in_expr so the backend can reject or
// handle genuinely unresolved cases.
new_lhs := t.transform_expr(lhs_id)
new_rhs := t.transform_expr(rhs_id)
// stabilize a side-effecting needle before a value-branch container hoists
new_lhs := if t.is_value_match_or_if_operand(rhs_id) {
t.snapshot_expr_for_reuse(lhs_id)
} else {
t.transform_expr(lhs_id)
}
new_rhs := t.transform_value_operand(rhs_id)
in_start := t.a.children.len
t.a.children << new_lhs
t.a.children << new_rhs
Expand Down Expand Up @@ -2066,7 +2119,11 @@ fn (mut t Transformer) lower_const_string_array_membership_expr(base_id flat.Nod
return none
}
}
needle := t.transform_expr(needle_id)
// Route the needle through typed value lowering (the container is a string array),
// so a value `match`/`if` needle materializes its propagating arm as a value instead
// of in a value-less statement context, e.g.
// `(match node { First { get_first(node)! } ... }) in allowed_words`.
needle := t.transform_expr_for_type(needle_id, 'string')
base_value := t.transform_expr(base_id)
base_data := t.make_cast('&string', t.make_selector(base_value, 'data', 'voidptr'), '&string')
len_expr := t.make_int_literal(expr.children_count)
Expand Down Expand Up @@ -2110,7 +2167,16 @@ fn (mut t Transformer) lower_type_pattern_membership(lhs_id flat.NodeId, rhs fla
if !t.is_sum_type_name(sum_name) {
return none
}
base := t.stable_expr_for_reuse(lhs_id)
// A value-branch subject (`(match node { First { make_foo(node)! } ... }) in [Foo1, Foo3]`)
// must be lowered as a typed value so its propagating arms are materialized into a temp;
// plain `stable_expr_for_reuse` would lower it with `transform_expr` in a value-less
// statement context and emit an empty expression.
base := if t.is_value_match_or_if_operand(lhs_id) {
t.stable_transformed_expr_for_reuse(t.transform_expr_for_type(lhs_id, sum_name), sum_name,
'in_lhs')
} else {
t.stable_expr_for_reuse(lhs_id)
}
// A non-trivial lhs is materialized as a value temp above. Use that temp's
// storage type for the tag checks; retaining the source pointer type here
// makes the generated checks dereference the value temp a second time.
Expand Down Expand Up @@ -2204,8 +2270,16 @@ fn (mut t Transformer) lower_array_membership_expr(base_id flat.NodeId, needle_i
elem_type, 'contains_needle')
t.drain_pending(mut prefix)
} else {
needle = t.stable_transformed_expr_for_reuse(t.transform_expr_for_type(needle_id, elem_type),
elem_type, 'contains_needle')
// `needle in container`: the needle is evaluated before the container in source order.
// If the container hoists a value branch whose prelude can mutate a syntactically stable
// needle (`x in (match node { First { change(mut x)! } ... })`), snapshot the needle's
// source-order value so the membership loop reads it before that prelude runs.
transformed_needle := t.transform_expr_for_type(needle_id, elem_type)
needle = if t.operand_hoists_value_branch(base_id) {
t.snapshot_transformed_expr_for_reuse(transformed_needle, elem_type, 'contains_needle')
} else {
t.stable_transformed_expr_for_reuse(transformed_needle, elem_type, 'contains_needle')
}
t.drain_pending(mut prefix)
base = t.stable_array_expr_for_membership(base_id, base_type, clean_base_type)
t.drain_pending(mut prefix)
Expand Down Expand Up @@ -2374,7 +2448,11 @@ fn (mut t Transformer) lower_array_last_index_expr(base_id flat.NodeId, needle_i
// stable_array_expr_for_membership
// supports helper handling in transform.
fn (mut t Transformer) stable_array_expr_for_membership(id flat.NodeId, raw_type string, clean_type string) flat.NodeId {
mut expr := t.transform_expr(id)
// Route a value `match`/`if` container through value lowering (e.g.
// `needle in (match node { ... get_values(node)! ... })`); otherwise the propagating
// arm tail is lowered in a value-less statement context and emits an empty expression.
// `transform_value_operand` is a no-op for the common non-branch containers.
mut expr := t.transform_value_operand(id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Snapshot array-membership needles before branch containers

When a propagating value-branch container mutates a syntactically stable needle, this materialization queues the container prelude after lower_array_membership_expr has left the needle identifier inline via stable_transformed_expr_for_reuse. For example, x in (match node { First { change(mut x)! } else { [1] } }) reads x in the generated membership loop only after change runs, rather than evaluating the needle before the container as the source requires. Snapshot the needle whenever the array container hoists a value branch, and add a propagation ordering regression.

AGENTS.md reference: AGENTS.md:L657-L658

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ee812dc. In x in container, lower_array_membership_expr lowers the needle before the container (needle-first path, receiver_first=false) but through stable_transformed_expr_for_reuse, which leaves a syntactically stable needle (ident/selector) inline. When the container is a value branch whose prelude mutates that needle, the container prelude drains after the needle is left inline, so the generated membership loop reads the needle only after the mutation.

The needle-first branch now snapshots the needle when the array container hoists a value branch — if t.operand_hoists_value_branch(base_id) { t.snapshot_transformed_expr_for_reuse(...) } — capturing its source-order value before the container prelude. The receiver-first path (arr.contains(x)) is intentionally unchanged: there the container is evaluated first in source order, so reading the needle after it is already correct.

Regression test select_value_membership_needle_snapshot: c.v in (match node { First { c.arr_first(node)! } … }) where the arm sets c.v = 1005 in [1, 5, 9] = true. On HEAD the mutated needle leaks in → 100 in [1, 5, 9] = false (verified: reverting the snapshot reproduces false). No new regressions; review_cgen passes.

if t.membership_container_is_pointer_array(raw_type) {
expr = t.make_prefix(.mul, expr)
}
Expand Down Expand Up @@ -3329,6 +3407,68 @@ fn (mut t Transformer) stable_transformed_expr_for_reuse(expr flat.NodeId, typ s
return t.make_ident(tmp_name)
}

// snapshot_expr_for_reuse materializes `id` into a temp holding its current value, unless it is
// a pure constant (which cannot change, so needs no snapshot). Ordering guards use it to capture
// the source-order value of an operand that precedes a value branch whose hoisted prelude might
// mutate that operand's storage. Unlike stable_expr_for_reuse it does snapshot value-bearing
// lvalues (idents/selectors/indexes) rather than leaving them inline.
fn (mut t Transformer) snapshot_expr_for_reuse(id flat.NodeId) flat.NodeId {
if t.is_ordering_snapshot_temp(id) {
return id
}
expr := if _ := t.generated_variant_access_type(id) {
id
} else {
t.transform_expr(id)
}
if t.is_pure_constant_expr(expr) || t.is_ordering_snapshot_temp(expr) {
return expr
}
tmp_name := t.new_temp('order_snapshot')
mut tmp_typ := t.node_type(expr)
if tmp_typ.len == 0 {
tmp_typ = t.node_type(id)
}
decl := t.make_decl_assign(tmp_name, expr)
if tmp_typ.len > 0 {
t.set_node_typ(int(decl), tmp_typ)
t.set_var_type(tmp_name, tmp_typ)
}
t.ordering_snapshot_names[tmp_name] = true
t.pending_stmts << decl
return t.make_ident(tmp_name)
}

// snapshot_transformed_expr_for_reuse is snapshot_expr_for_reuse for an already-transformed
// expression of known type.
fn (mut t Transformer) snapshot_transformed_expr_for_reuse(expr flat.NodeId, typ string, prefix string) flat.NodeId {
if t.is_pure_constant_expr(expr) || t.is_ordering_snapshot_temp(expr) {
return expr
}
tmp_name := t.new_temp(prefix)
t.ordering_snapshot_names[tmp_name] = true
t.pending_stmts << t.make_decl_assign_typed(tmp_name, expr, typ)
return t.make_ident(tmp_name)
}

// is_ordering_snapshot_temp reports whether `id` is an identifier naming a temp already created
// by a snapshot_*_for_reuse call. Such a temp holds a captured source-order value that no branch
// prelude mutates, so it must not be snapshotted again (which would recurse on a re-dispatch).
fn (t &Transformer) is_ordering_snapshot_temp(id flat.NodeId) bool {
if int(id) < 0 || int(id) >= t.a.nodes.len {
return false
}
node := t.a.nodes[int(id)]
return node.kind == .ident && node.value in t.ordering_snapshot_names
}

// operand_needs_ordering_snapshot reports whether a preceding operand must be snapshotted to
// preserve its source-order value before a later value branch's hoisted prelude runs: it is a
// value-bearing lvalue read (not a pure constant) and is not already a snapshot temp.
fn (t &Transformer) operand_needs_ordering_snapshot(id flat.NodeId) bool {
return !t.is_pure_constant_expr(id) && !t.is_ordering_snapshot_temp(id)
}

// is_stable_expr_for_reuse reports whether is stable expr for reuse applies in transform.
fn (t &Transformer) is_stable_expr_for_reuse(id flat.NodeId) bool {
if int(id) < 0 {
Expand Down Expand Up @@ -3372,6 +3512,42 @@ fn (t &Transformer) is_stable_expr_for_reuse(id flat.NodeId) bool {
}
}

// is_pure_constant_expr reports whether `id`'s value cannot be changed by a later mutation of
// any variable — a literal, enum value, `sizeof`/`typeof`, or a cast/paren/struct made only of
// such. Unlike is_stable_expr_for_reuse it returns false for value-bearing lvalue reads
// (idents, selectors, indexes): those are cheap to re-evaluate, but a hoisted branch prelude
// can mutate their storage, so reading them after the prelude yields a different value. Ordering
// guards use this to decide whether a preceding operand must be snapshotted to preserve its
// source-order value.
fn (t &Transformer) is_pure_constant_expr(id flat.NodeId) bool {
if int(id) < 0 {
return true
}
node := t.a.nodes[int(id)]
return match node.kind {
.int_literal, .float_literal, .bool_literal, .char_literal, .string_literal, .nil_literal,
.none_expr, .enum_val, .sizeof_expr, .typeof_expr {
true
}
.cast_expr, .paren {
node.children_count == 0 || t.is_pure_constant_expr(t.a.children[node.children_start])
}
.struct_init, .field_init {
mut pure := true
for i in 0 .. node.children_count {
if !t.is_pure_constant_expr(t.a.child(&node, i)) {
pure = false
break
}
}
pure
}
else {
false
}
}
}

// transform_fixed_array_len transforms transform fixed array len data for transform.
fn (mut t Transformer) transform_fixed_array_len(_id flat.NodeId, node flat.Node) ?flat.NodeId {
if node.value != 'len' || node.children_count == 0 {
Expand Down
Loading
Loading