pure/loops: skip output reordering when the ok-tuple is not a permutation of the loop outputs - #25
Open
karthikbhargavan wants to merge 6 commits into
Open
pure/loops: skip output reordering when the ok-tuple is not a permutation of the loop outputs#25karthikbhargavan wants to merge 6 commits into
karthikbhargavan wants to merge 6 commits into
Conversation
Co-authored-by: Alexander Bentkamp <alexander@cryspen.com>
Co-authored-by: Alexander Bentkamp <alexander@cryspen.com>
Co-authored-by: Alexander Bentkamp <alexander@cryspen.com>
…tion of the loop outputs `compute_outputs_indices_if_followed_by_ok` assumed that the arguments of the `ok (...)`/`break (...)` tuple following a loop are a permutation of the variables bound by the loop-output pattern, and mapped each argument to its position in the pattern. This assumption is wrong: the ok-tuple may repeat an fvar when the forward result coincides with a given-back value. For instance, `return *x` inside a `loop` over `x: &mut u32` makes SymbolicToPure produce `let v = loop ... in ok (v, v)`. The computed index list then has the wrong length, the sanity check in `reorder_loop_outputs.update_and_close_loop_body.upd` fires, and the uncaught `CFailure` escapes the per-function error recovery and aborts translation of the entire crate. Had the lengths happened to match, duplicated indices would instead have silently duplicated break outputs, so the guard is a correctness fix as well as a crash fix. Fix: return `None` (i.e., skip the reordering optimization for that loop) unless the ok-tuple args and the loop-output pattern have equal length and the computed index list is duplicate-free, i.e., a true permutation. Validated by translating the issue's repro (now succeeds, with the loop result correctly used for both outputs) and its control, and by running the full test suite: all backend outputs are byte-identical to the committed ones. Fixes #22 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
maximebuyse
self-requested a review
July 30, 2026 11:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Translating a function that
returns a borrowed value from inside aloop— e.g.crashed Aeneas with an uncaught internal error (
CFailure) that escaped the per-function error recovery and aborted translation of the entire crate.Root cause: when the forward result coincides with a given-back value,
SymbolicToPureproduceslet v = loop ... in ok (v, v)— the ok-tuple repeats the same fvar.compute_outputs_indices_if_followed_by_ok(src/pure/PureMicroPassesLoops.ml) assumed the ok-tuple args are a permutation of the loop-output pattern; with a duplicated fvar the computed index list has the wrong length and the break-rewriter's sanity check fires (PureMicroPassesLoops.ml:1818-1819). Had the lengths happened to match, duplicated indices would instead have silently duplicated break outputs, so the guard is a correctness fix as well as a crash fix.Fix: in
compute_outputs_indices_if_followed_by_ok, returnNoneunless (a) the ok-tuple args and the loop-output pattern have equal length and (b) the computed index list is duplicate-free (a true permutation).Nonemerely skips the reordering optimization for that loop.Fixes #22
Validation
All with the pinned charon (v0.1.223,
charon-pin=cb50ff16); the repro llbc were regenerated with it.devbuild onf4_return_in_loop.rs→[Error] Internal error, backtrace throughPureMicroPassesLoops.ml, exit 2, no output.F4.leangenerated; the loop function is as expected, with the single loop result used for both outputs:control_ok.rs(distinct returned value) still translates, exit 0.tests/srcand ran both the patched and an unpatcheddevbinary with-backend leanon every fixture: zero exit-code differences; the only 6 non-zero fixtures (borrow_check_negative,loops_borrow_check_negative,raw_pointers,higher_ranked_implied_bounds_{borrow,regions,types}) fail identically on unpatcheddev(expected-failure tests). The fullextract-testsrun (all backends,-checks) passed and left every committed generated output byte-identical (git statusclean apart from the source change).dune buildanddune build @fmtboth pass.🤖 Generated with Claude Code