fix: extract inclusive for-loop ranges with their final iteration - #310
Open
asterite wants to merge 1 commit into
Open
fix: extract inclusive for-loop ranges with their final iteration#310asterite wants to merge 1 commit into
asterite wants to merge 1 commit into
Conversation
The extractor ignored the `inclusive` flag on `HirForStatement`, so a Noir loop `for i in start..=end` was emitted as the half-open Lean loop `for i in start .. end`, silently dropping the final iteration. Any assert or state mutation in that iteration was absent from the model, making it diverge from the program's real semantics (found while verifying noir-lang/sha256, where the dropped iteration carried the assert constraining the last word of a message block). Emit `end + 1` as the upper bound when the range is inclusive, mirroring the rewrite Noir's own SSA generation performs for constant bounds. When `end` is the maximum value of the index type this add overflows and the model treats the execution as failing, whereas the real program iterates through `end` — modelling that corner faithfully would need an inclusive-loop primitive in the semantics; the divergence is documented at the emission site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
asterite
added a commit
to noir-lang/sha256
that referenced
this pull request
Jul 27, 2026
The extractor dropped the final iteration of Noir's inclusive `for k in msg_start..=msg_end` loop in build_msg_block, so the Lean model never constrained the last word of a message block, making the block spec unprovable. Re-extracted with the fix proposed in reilabs/lampe#310; the only change is the loop's upper bound becoming `msg_end + 1`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 tasks
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.
Hi! I'm continuing the work in noir-lang/sha256#59 and I found that inclusive ranges (
..=) didn't work well in Lampe, so this is a PR to fix that.Below is the description that Claude created for the PR.
Problem
The extractor ignores the
inclusiveflag onHirForStatement(and the monomorphizedast::For), so a Noir loopis emitted as the half-open Lean loop
for i in start .. end, silently dropping the final iteration for every inclusive loop.This makes the Lean model diverge from the program's real semantics in both directions:
assert, the model is weaker than the circuit, so correctness specs against a reference become unprovable — and one could wrongly conclude a circuit under-constrains a value.We found this while formally verifying noir-lang/sha256: its
build_msg_blockusesfor k in msg_start..=msg_end, where the final iteration carries theassert_eqconstraining the last word of the message block. In the extracted model that word was never constrained, making the block-correctness spec unprovable.Fix
When
fors.inclusiveis set, emitend + 1(an add of the numeric literal1at the loop-index type) as the loop's upper bound. This mirrors the rewrite Noir's own SSA generation performs for constant bounds below the type's maximum (seecodegen_forinnoirc_evaluator/src/ssa/ssa_gen/mod.rs).Known caveat:
endat the type's maximumWhen
endis the maximum value of the index type,end + 1overflows; the Lampe model treats that as failure, whereas the real program iterates up to and includingend(Noir generates abreak-based loop for that case rather than computingend + 1). So for that corner the model considers executions failing that really succeed, andSTHoaretriples about them hold vacuously.Modelling this faithfully would require either an inclusive-loop primitive in the
Omnisemantics or desugaring to an exclusive loop plus a guarded trailing iteration (if start ≤ end { body(end) }), both of which are heavier changes with proof-ergonomics implications. Given the status quo drops an iteration from every inclusive loop, this PR takes the simple fix and documents the divergence at the emission site — happy to follow up with the faithful treatment if you have a preference between the two.Testing
inclusive_range.nrtoExtractionTestswith its extracted golden showing theend + 1bound.build_msg_blockspec (including the final word's constraint) is fully provable against the extraction; without it, it is not.🤖 Generated with Claude Code