feat: prove Level cached-flag correctness - #27
Open
kim-em wants to merge 3 commits into
Open
Conversation
Owner
|
Yes I know it's false. This is what leanprover/lean4#8821 is for, maybe you can ask your human master to go poke at the FRO to stop sitting on year old PRs. |
Contributor
Author
Hi Mario (pure unadulterated human ignorance here, not LLM ignorance on top), what progress can we make in the meantime, without lean4#8821? Is this progress in the right direction? Is another reasonable alternative to be change |
Replace the `Level.hasParam_eq` and `Level.hasMVar_eq` axioms with proofs derived from `Level.mkData_eq`, and move them and the structural `hasParam'`/`hasMVar'` definitions out of `Verify/Axioms.lean` into `Verify/Level.lean`, as was done for the Expr cached flags in digama0#19. This also fixes the overflow branch of the `Level.mkData'` model, which was jointly inconsistent with the two flag equalities: see the new docstring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5K6isCtnCB86J8fRbVTKS
State the trust claim narrowly: `mkData_eq` is a chosen total specification for the opaque constant, sound for runs that do not abort, not a claim about what the implementation computes. Record that the `panic!` model was a copy of the pre-`@[extern]` Lean definition, i.e. of the bug fixed by leanprover/lean4#8559. Merge `mkData_hasParam` and `mkData_hasMVar` into a single `mkData_flags`, matching `Expr.mkData_flags`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5K6isCtnCB86J8fRbVTKS
leanprover/lean4#8554 is a closed PR, not an issue, and it proposed saturating; the merged fixes are leanprover/lean4#8559 for `Level` and leanprover/lean4#8560 for `Expr`, both of which abort. Rest the choice of total extension on the abort semantics alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5K6isCtnCB86J8fRbVTKS
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.
This PR replaces the
Level.hasParam_eqandLevel.hasMVar_eqaxioms with proofs derived fromLevel.mkData_eq, and moves them, along with the structuralhasParam'/hasMVar'definitions, out ofVerify/Axioms.leanintoVerify/Level.lean, mirroring what #19 (Prove Expr cached flag correctness) did for the Expr flags. Closes #24 (Prove Level cached-flag correctness).Please look closely at one part of this, because it goes beyond what the issue asked for and it changes what
mkData_eqasserts. As stated, the two equalities are not just unprovable frommkData_eq, they are refutable, and fixing that meant changing the overflow branch of themkData'model.The old model was a character-for-character copy of the Lean definition of
mkDatathat preceded the@[extern]one:That is, it was a copy of the bug:
panic!evaluates todefault = 0, so the branch drops both flag bits, which is exactly the non-conservativity reported in leanprover/lean4#8554 (fix: soundness bug in {Expr, Level}.data).Level.datafor.succ upassesu.data.depth.toNat + 1andData.depthreads 24 bits, so a level of cached depth2 ^ 24 - 1exists and its successor overflows. HenceLevel.succ (succ^[2 ^ 24 - 1] (.param x))hashasParam = falsebuthasParam' = true, andmkData_eqtogether with the oldhasParam_eqaxiom provesFalse. I checked that in Lean; the derivation is at the end of this description. The same goes forhasMVar_eqseparately.The kernel as it stands cannot reach that state.
Level.mkDataisopaquewith@[extern "lean_level_mk_data"], and since leanprover/lean4#8559 (fix: block adversarial exploit of non-abortingassert!) the C++ callslean_internal_panicon an overflowing depth, which prints to a saved stderr handle and callsexit(1). It never returns aLevel.Data. leanprover/lean4#8560 (fix: block potential adversarial exploit of non-abortingassert!) did the same forExpr.So on that branch there is no value to transcribe, and a total Lean model has to invent one. This PR invents the saturating one:
which keeps the flag bits and makes both equalities hold unconditionally. Two things to be clear about, both now recorded in the
mkData'docstring:panic!/0fails the second and is why the current axioms are inconsistent.A consequence is that
mkData_hasParamandmkData_hasMVarno longer need theird < 2 ^ 24hypotheses; they are now projections of a sharedmkData_flags, matchingExpr.mkData_flags.mkData_depthstill needs the hypothesis.The structural definitions and the downstream API are unchanged, and the axiom footprint matches the Expr precedent:
Not addressed here:
Expr.looseBVarRange_eqis still an axiom carrying the same stale "should now be provable" comment, and it is refutable against the currentExpr.mkData'for the same reason..bvar iwithi + 1 > 2 ^ 20 - 1fails theassert!, so the data is0andlooseBVarRange = 0whilelooseBVarRange' = i + 1. The Expr flags were unaffected, since0is conservative for them, which is why #19 went through.Derivation of `False` from the previous pair of axioms
Checked against the parent commit, 7842f38:
🤖 Prepared with Claude Code