Lift unlabeled block expressions emitted by Python-to-Laurel#1133
Lift unlabeled block expressions emitted by Python-to-Laurel#1133tautschnig wants to merge 3 commits into
Conversation
The Python-to-Laurel translator emits unlabeled Block expressions in
expression position for two common patterns:
* Modeled method calls with precondition tag checks:
{ asserts...; Call }
* Unmodeled method calls (PR #1019): havocs receiver + Any-typed args,
then yields a Hole:
{ havoc_stmt...; Hole }
The Laurel-to-Core translator rejects any Block in expression position
("block expression should have been lowered in a separate pass"), so
the LiftImperativeExpressions pass must eliminate these patterns.
The existing Block case in transformExpr kept the Block structure and
only transformed its sub-expressions. This change replaces the Block
structure with "prepend side-effects + return last element as value",
which is the natural semantics for these patterns.
Changes in LiftImperativeExpressions.lean:
1. transformExpr .Block case: for unlabeled blocks, recursively transform
each stmt (preserves SSA-snapshot semantics for assigns in expression
position), hoist asserts/declares via onlyKeepSideEffectStmtsAndLast,
then return the last element as the expression value rather than
wrapping back into a Block.
2. New helper containsAssignmentOutsideUnlabeledBlock: used by the
Assert/Assume cases in transformStmt. These cases previously bailed
out when a condition contained any assignment; they now proceed when
the assignment is inside an unlabeled block (since the .Block case
above will safely lift the side-effect).
The original containsAssignment is preserved for use by the ITE case,
which correctly isolates branch-local prepends when a branch contains
assignments.
Eliminates all "block expression should have been lowered" errors in
the internal benchmark suite.
Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Laurel liftExpressionAssignments pass so that unlabeled Block expressions occurring in expression position are lowered into “prepended side-effects + final value expression”, eliminating Block nodes that the Laurel-to-Core translator rejects. This targets Python-to-Laurel output patterns like { asserts; Call } and { havoc...; Hole }, and relaxes assert/assume-condition handling to allow assignments when they occur inside such unlabeled blocks.
Changes:
- Change
transformExpr’s.Blockhandling so unlabeled blocks are lowered by hoisting side effects and returning the last element as the value (instead of rebuilding aBlock). - Add
containsAssignmentOutsideUnlabeledBlockand use it intransformStmtfor.Assert/.Assumeso conditions can be transformed when assignments are confined to unlabeled blocks. - Extend lifting tests with an additional procedure to cover conditional assignment lifting behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Strata/Languages/Laurel/LiftImperativeExpressions.lean | Lowers unlabeled block expressions to prepends + value; adjusts assert/assume filtering for assignments inside unlabeled blocks. |
| StrataTest/Languages/Laurel/LiftExpressionAssignmentsTest.lean | Adds regression coverage for unlabeled blocks in expression contexts and ITE branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey, thanks for this PR. You're right that Laurel should support the code that PythonToLaurel is generating here. One thing though, the existing error "block expression should have been lowered" is phrased too strongly. Sorry about that. LaurelToCoreTranslator is intended to support some blocks in expressions, namely the ones that can be translated to Core expressions. The intention is for a block like Could we investigate making sure that only valid expression blocks make it to
Can we lift the asserts? The Laurel->Core translator will handle a singleton block
Can you say more about what Maybe there should be a separate case for a hole assignment in a block: |
Surgical follow-up addressing the two review threads on PR #1133: 1. Fix the bug Copilot flagged in transformExpr.Assign for .Declare targets without a substitution: the previous "return expr" early- returned the original assignment with the initializer un-transformed, so any unlabeled block / nested assignment / nondeterministic hole embedded in the initializer escaped lifting and reached the Laurel- to-Core translator unchanged. The fix recursively transforms the initializer with a fresh prepend stack, then reassembles state as outerPrepends ++ innerPrepends ++ [liftedDecl]. The outerPrepends-first ordering is important: when an already-processed (later in source) assignment introduced a snapshot substitution that this declaration's initializer now references, the snapshot declaration must precede the use. 2. Revert the wholesale .Block unwrapping that this PR previously added. keyboardDrummer pointed out that LaurelToCoreTranslator already natively handles singleton blocks (`{ e }`) and var-init-prefixed blocks (translating them as let-bindings via app(abs(name, body), value)), so collapsing the Block structure unconditionally was pessimizing those cases. Going back to the original two-line .Block case lets the translator see the structure it can handle. The actual benefit of this PR — relaxing assert/assume condition handling via containsAssignmentOutsideUnlabeledBlock — is preserved. 3. Add the regression test Copilot requested for nested-block-in-decl- initializer (`var y := { var t := { x := 1; x }; t + 1 }`). Without change (1) above, the inner block survives and the Core translator raises "block expression should have been lowered". Existing test expectations are updated to reflect the .Block revert (singleton `{ x }` / `{ y }` / `{ 0 }` / `{ t + 1 }` instead of unwrapped values). Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
|
Good point. In 977afc1 I reverted the wholesale Re: |
he Python-to-Laurel translator emits unlabeled
Blockexpressions in expression position for several patterns, including modeled method calls with precondition tag checks ({ asserts...; Call }) and unmodeled method calls (PR #1019:{ havocs...; Hole }). These patterns triggerblock expression should have been lowered in a separate passerrors inLaurelToCoreTranslatorfor nested cases, becauseLiftImperativeExpressionswas not fully lowering blocks/assignments embedded inside declare-with-init initializers.LaurelToCoreTranslatordoes already handle several block-in-expression patterns natively:.Block [single]— singleton blocks.Block (.Assign [.Declare ⟨name, ty⟩] init :: rest)— translated as a let-binding (app(abs(name, body), value))so this PR is now scoped narrowly to make sure only valid expression blocks reach the translator, rather than collapsing block structure unconditionally.
Changes in
LiftImperativeExpressions.lean:transformExpr.Assignfor.Declaretargets without substitution now recursively transforms the initializer with a fresh prepend stack and reassembles state asouterPrepends ++ innerPrepends ++ [liftedDecl], returning aVar(.Local name)reference as the expression's value. Previously the case didreturn exprearly, leaving inner blocks / assignments / nondeterministic holes in the initializer unlowered. TheouterPrepends-first ordering preserves correct execution order when a substitution-introducing snapshot decl (from a later-in-source assignment, processed earlier in the right-to-left traversal) is referenced from the transformed initializer.New helper
containsAssignmentOutsideUnlabeledBlockis used by the.Assert/.Assumecases oftransformStmt. These cases previously bailed out whenever the condition contained any assignment; they now proceed when the assignment is confined to an unlabeled block — since the.Blockcase oftransformExprtogether withonlyKeepSideEffectStmtsAndLastwill safely lift those side effects as prepends.The original
containsAssignmentis preserved for the ITE case, which correctly isolates branch-local prepends when a branch contains assignments.transformExpr.Blockis unchanged in its structural behavior: sub-expressions are recursively transformed, side effects are extracted viaonlyKeepSideEffectStmtsAndLast, and the result is wrapped back as a Block. We deliberately do not collapse Block structure here so that singleton blocks andvar x := init-prefixed blocks remain in shape for the translator's let-binding case.A regression test (
nestedBlockInDeclInit) covering the{ var t := { x := 1; x }; t + 1 }pattern is added toLiftExpressionAssignmentsTest.lean. Existing test expected outputs are kept consistent with this approach (singleton blocks{ x }/{ y }/{ 0 }/{ t + 1 }rather than unwrapped values).Eliminates all
block expression should have been lowerederrors in the internal benchmark suite.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.