Infrastructure changes from PR 385#426
Merged
keyboardDrummer merged 1 commit intomainfrom Feb 26, 2026
Merged
Conversation
joehendrix
reviewed
Feb 16, 2026
joehendrix
reviewed
Feb 16, 2026
joehendrix
requested changes
Feb 16, 2026
Contributor
joehendrix
left a comment
There was a problem hiding this comment.
This PR bundles infrastructure improvements including a new NewlineSepBy separator, SMT encoding fixes for multi-argument functions and Map types, and a substFvarLifting utility for de Bruijn index handling.
Two issues were found (the second is minor):
- Precedence bug in Format.lean:321: Changing ≤ to < drops parentheses needed to preserve same-precedence grouping and non-default associativity — e.g., (true ==> false) ==> true formats as true ==> false ==> true, which parses as a different proposition.
- substFvarLifting limitation in LExprWF.lean:334: The liftBVars call uses default cutoff 0, which incorrectly shifts bvars in the replacement term that refer to binders shared with the target expression; the current SMT encoder call site is safe but the restriction should be documented or addressed with a cutoff parameter.
aqjune-aws
reviewed
Feb 16, 2026
Contributor
Author
|
Leaving the comments for @fabiomadge since I extracted this code from his PR. |
cc88a4c to
54897c3
Compare
9b0e6d0 to
a179ab3
Compare
Contributor
|
Rebased on current main and addressed all review feedback:
Tests added: Current state: 20 files, 394 insertions. All 406 tests pass. |
DDM changes: - Add NewlineSepBy separator and SyntaxDef.passthrough variant - Replace fromIonName?/toIonName with fromCategoryName? for category-based lookup - Add newline formatting case in ArgF.mformatM - Update Java/Lean codegen for new constructs - Comment parsing fix in Parser.lean Lambda/SMT changes: - Add liftBVars with cutoff parameter for correct de Bruijn index shifting - Add substFvarLifting/substFvarsLifting for substitution under binders - Fix multi-argument function SMT encoding (was hardcoded to unary) - Add Map type to SMT Array encoding Note: Laurel tests will fail until the Laurel grammar is updated to match the new DDM SyntaxDef structure (done in the follow-up Laurel API PR).
a179ab3 to
76ba04f
Compare
joehendrix
approved these changes
Feb 26, 2026
Contributor
joehendrix
left a comment
There was a problem hiding this comment.
This looks good to me.
aqjune-aws
approved these changes
Feb 26, 2026
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 26, 2026
…ons (#481) Part of the incremental split of #385. Can be merged independently of #426. ### Changes Converts Laurel's `Procedure` from a single precondition to a list of preconditions, and `Body.Abstract` from a single postcondition to a list. This enables multiple `requires`/`ensures` clauses per procedure, which is needed by the upcoming constrained types and contract features. **Laurel.lean:** - `precondition : WithMetadata StmtExpr` → `preconditions : List (WithMetadata StmtExpr)` - `Body.Abstract postcondition` → `Body.Abstract (postconditions : List ...)` - `Body.Opaque postcondition` → `Body.Opaque postconditions` (name only, already a list) - Reorder `Parameter` struct, add `Repr` instances **Adapted callers:** - `ConcreteToAbstractTreeTranslator` — parse `Option OptionalRequires` into list - `LaurelToCoreTranslator` — iterate over preconditions list, generate indexed labels for multiple preconditions - `HeapParameterization` — fold over preconditions/postconditions lists - `LaurelFormat` — format preconditions/postconditions lists, relocate `formatDeterminism` - `PythonToLaurel`, `Specs/ToLaurel` — adapt to new field names **LiftExpressionAssignments.lean:** - Process block-in-expression non-last elements left-to-right via `transformStmt` instead of right-to-left via `transformExprDiscarded` - Remove now-unused `transformExprDiscarded` ### Testing All existing Laurel, Python, and Core tests pass. No new tests needed — this is a structural refactor with no new features. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. --------- Co-authored-by: Andrew Wells <130512013+andrewmwells-amazon@users.noreply.github.com>
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.
Contains a subset of the changes from #385, rebased on current main.
DDM Infrastructure
NewlineSepByseparator andSyntaxDef.passthroughvariantfromIonName?/toIonNamewithfromCategoryName?for category-based lookupArgF.mformatMParser.leanLambda/SMT Bug Fixes
liftBVarswith cutoff parameter for correct de Bruijn index shiftingsubstFvarLifting/substFvarsLiftingfor substitution under binders (doc comment clarifies thatto's bvars must refer to binders outsidee)Testing
LExprWFTests.lean: tests forsubstFvarLiftingSMTEncoderTests.lean: updated for multi-arg encodingFunctions.lean: multi-argument function test + quantifier-in-body testReview feedback addressed
≤→<precedence change from the previous version has been removed (it was incorrect)substFvarLiftingper review feedbackBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.