Repository navigation
feat(opy): complete collection mutation and control flow - #335
Conversation
Teakowa
left a comment
There was a problem hiding this comment.
Blocking findings:
-
crates/opy-rs/src/lower/statements.rs/crates/opy-rs/src/compiler/lowering.rs— the newdeldepth boundary is one indexed level too strict. Pinned OverPy 9.7.10 rewrites the final deletion first, accepts a source target with four indexes through its three-dimensional reconstruction path, and only reachesCannot delete index of 4d arraywhen there is one additional nested__valueInArray__. This patch rejects every target withindexed_expr_depth >= 4/indices.len() > 3, so an upstream-accepted core form is rejected while the support row is promoted to Supported. The same upstream reconstruction also explicitly rejects random outer/middle indexes; the new native path has no equivalent boundary. Add pinned acceptance/rejection probes for these nested forms and match the 9.7.10 boundary before markingdelSupported. -
crates/opy-rs/src/compiler/lowering.rs—continuestill fails when aswitchis nested inside an enclosingfor/while/do ... while.contains_loop_continueonly descends throughStmt::If; a switch falls through tolower_switch_body, which sends its statements tolower_action(..., BreakTarget::Switch), whereStmt::Continueis rejected. Pinned OverPy'scontinue.tswalks parents through intervening structures until it reaches the enclosing loop, so this is an established accepted nesting form. Add an oracle-backed switch-in-loop case and preserve the enclosing-loop continue semantics before promoting the row to Supported. -
crates/opy-rs/src/compiler/lowering.rs/crates/opy-rs/src/compiler/tests/control_flow.rs— the new conditional dynamicgoto loc+...path is only protected by a native-output assertion. The pinned fixture covers an unconditional dynamic goto, notif condition: goto loc+expr. This matters becauseloc+is action-relative and pinned OverPy deliberately tracks variable gotos at rule scope to preserve surrounding control-flow/action positions. #328 requires independent upstream/differential evidence for completed behavior, so the implementation currently self-specifies this new path. Add a pinned 9.7.10 differential case with surrounding actions/structure and make the lowering match that action-distance behavior; do not rely on the single-Skip Ifassertion as completion evidence.
Teakowa
left a comment
There was a problem hiding this comment.
Follow-up: findings 2 and 3 are fixed, and the del depth/random-index cases now match the pinned oracle. One compatibility gap remains in finding 1: lower_delete checks randomness only in indices[..indices.len() - 1], but pinned OverPy 9.7.10 __del__.ts also calls astContainsRandom(content.args[0]) on the indexed array/root expression. That matters for player-variable targets because the parser permits an arbitrary player expression as the __playerVar__ receiver (for example a random player expression). A nested target such as del random.choice(getAllPlayers()).values[0][0][0] is therefore rejected upstream but is not rejected by this native guard. Include the root/player receiver in the nested-random check and add a pinned rejection probe for this form. After that, the previous review findings are otherwise resolved.
Summary
dellowering for global/player variables through three dimensionscontinue,goto RULE_START, and dynamicloc+control-flow formsValidation
cargo test -p opy-rsFixes #328