Skip to content

Fix Boole tests after merge#1209

Merged
atomb merged 1 commit into
merge-with-main-2026-05-20from
josh/fix-boole-tests
May 22, 2026
Merged

Fix Boole tests after merge#1209
atomb merged 1 commit into
merge-with-main-2026-05-20from
josh/fix-boole-tests

Conversation

@joscoh
Copy link
Copy Markdown
Contributor

@joscoh joscoh commented May 21, 2026

This is a temporary PR to fix the Boole tests after the merge with main. This PR does 3 main things:

  1. Makes the sequence functions in sha256_compact_indexed monomorphic, since the SMT backend does not support polymorphism (resulting from the newly produced preconditions for sequence indexing)
  2. Adds invariants and preconditions to allow the SHA256 example to verify
  3. Fixes a bug where the type of Sequence.empty was lost in the translation to Core, which also resulted in polymorphic precondition proof obligations that hit errors. For example, v:=Sequence.select(Sequence.empty_int, 0); generated 0 < Sequence.length (Sequence.empty), which lost the information that Sequence.empty was instantiated at type int.

@github-actions github-actions Bot added the Core label May 21, 2026
@atomb atomb marked this pull request as ready for review May 22, 2026 15:32
@atomb atomb requested a review from a team as a code owner May 22, 2026 15:32
Copy link
Copy Markdown
Contributor

@PROgram52bc PROgram52bc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some hand-written comments based on agent feedbacks.

| .seq_of_bv8 _ ⟨_, vs⟩ | .seq_of_bv16 _ ⟨_, vs⟩ | .seq_of_bv32 _ ⟨_, vs⟩
| .seq_of_bv64 _ ⟨_, vs⟩ | .seq_of_int _ ⟨_, vs⟩ => do
let vals ← vs.toList.mapM toCoreExpr
return vals.foldl (fun acc v => mkCoreApp Core.seqBuildOp [acc, v]) Core.seqEmptyOp
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here it still uses the default Core.seqEmptyOp as an initial value, and could lead to missing type information if vals is empty.

function Seq_lib_filter (s : Sequence bv32, p : bv32 -> bool) : Sequence bv32;
function Seq_lib_sort_by (s : Sequence bv32, less : bv32 -> bv32 -> bool) : Sequence bv32;
function Seq_lib_to_set (s : Sequence bv32) : Set bv32;
function Set_finite (s : Set bv32) : bool;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since they are no longer polymorphic, maybe renaming to something like Set_finite_bv32 (and similarly for the above) makes more sense?

@atomb
Copy link
Copy Markdown
Contributor

atomb commented May 22, 2026

Since @joscoh is out today and owns this PR, we agreed to merge this one first and then address the two comments above either as an amendment to #1205 or a second PR right after it.

@atomb atomb merged commit 7346fe8 into merge-with-main-2026-05-20 May 22, 2026
17 checks passed
@atomb atomb deleted the josh/fix-boole-tests branch May 22, 2026 18:28
PROgram52bc pushed a commit that referenced this pull request May 22, 2026
…oding

Resolves conflict in StrataTest/Languages/Boole/FeatureRequests/sha256_compact_indexed.lean.

Base (#1209) independently fixed the seqEmpty type-loss in Verify.lean / Factory.lean
and made the SHA256 example monomorphic with extra invariants/preconditions, so its
expected #guard_msgs output is the post-fix version (all pass). With the program now
monomorphic and concretely typed, this PR's callSiteTypeSubst / SMTEncoder ftvar
fallback are no-ops on this test, and base's expected output is the correct merged
result. Took base's `/-- info:` block verbatim; build verified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants