v3: lower value-context match/if block operands with !/? propagation (fix #28000)
#28011
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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' { | ||
| // 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' { | ||
| 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 | ||
| } else if child.value == 'init' { | ||
| init_expr_id = t.a.child(child, 0) | ||
|
|
@@ -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)) | ||
| // 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 { | ||
|
|
@@ -1167,7 +1191,15 @@ fn (mut t Transformer) try_lower_array_append_stmt(id flat.NodeId) ?[]flat.NodeI | |
| } | ||
|
|
||
| mut result := []flat.NodeId{} | ||
| 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 { | ||
|
|
@@ -1196,7 +1228,12 @@ fn (mut t Transformer) try_lower_array_append_stmt(id flat.NodeId) ?[]flat.NodeI | |
| } | ||
| } | ||
| } else { | ||
| // 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) | ||
|
|
@@ -1368,6 +1405,21 @@ fn (mut t Transformer) try_lower_optional_array_append_stmt(_node flat.Node, lhs | |
| source) | ||
| result << t.make_if(not_ok, t.make_block(guard_stmts), t.make_empty()) | ||
|
|
||
| // If the RHS hoists a value branch whose prelude can reassign the optional source | ||
| // (`holder.values? << (match ... { holder.replace()! } ...)`), capture the optional's | ||
| // value-array address before lowering the RHS, so the append targets the storage selected in | ||
| // source order (consistent with the guard above) instead of re-reading the inline source | ||
| // after the RHS prelude. | ||
| mut captured_lhs_addr := flat.empty_node | ||
| mut has_captured_addr := false | ||
| 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') | ||
| has_captured_addr = true | ||
| t.drain_pending(mut result) | ||
| } | ||
|
|
||
| mut rhs := flat.empty_node | ||
| if !push_many { | ||
| if !rhs_is_sum_variant { | ||
|
|
@@ -1395,7 +1447,12 @@ fn (mut t Transformer) try_lower_optional_array_append_stmt(_node flat.Node, lhs | |
| } | ||
| } | ||
| } else { | ||
| // 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) | ||
|
There was a problem hiding this comment.
In the optional-LHS path, a push-many branch such as AGENTS.md reference: AGENTS.md:L657-L658 Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. 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 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 One transparency note on the regression. I built the branch and compared v3 against mainline The added regression |
||
| } | ||
| if !push_many { | ||
| rhs = t.coerce_transformed_expr_to_type(rhs, rhs_id, elem_type) | ||
|
|
@@ -1406,7 +1463,11 @@ fn (mut t Transformer) try_lower_optional_array_append_stmt(_node flat.Node, lhs | |
| push_many = t.array_append_rhs_is_push_many(lhs_id, rhs_id, rhs_type, elem_type) | ||
| } | ||
|
|
||
| lhs_addr := if has_captured_addr { | ||
| captured_lhs_addr | ||
| } else { | ||
| t.runtime_addr(t.make_selector(source, 'value', array_type), array_type) | ||
| } | ||
| if push_many { | ||
| call := if t.is_fixed_array_type(rhs_type) { | ||
| t.make_call_typed('array_push_many_ptr', arr3(lhs_addr, rhs, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
lenhas side effects andcapis a valuematch/ifwith propagation, transformingcapqueues its materialization beforenew_call, while the transformedlenexpression remains inline in that call. Thus[]int{len: trace_len(), cap: match node { ... trace_cap()! ... }}evaluatestrace_capbeforetrace_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 👍 / 👎.
There was a problem hiding this comment.
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_runtimenow finds the lastlen/capfield whose value is a value branch, and stabilizes an earlier side-effectinglen/capfield into a temp before that field hoists its materialization prelude — both are evaluated intonew_call, so this preserves their source order. (initis 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 (tcapran beforetlen). No new regressions;review_cgenpasses.