-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proof conventions 2 #1134
Proof conventions 2 #1134
Conversation
85a38f8
to
f39fd01
Compare
96c3688
to
8995741
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like these changes. They clean up a lot of long-standing cruft. I left a few comments, but they're all really just comments, not suggestions to change anything as part of this PR.
SatMulti stats vals -> do | ||
res <- runProofScript tactic proofgoal | ||
case res of | ||
ValidProof stats _thm -> return stats -- TODO, do something with these theorems! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be valuable to store theorems in MethodSpec
s? We'd want to check that it doesn't lead to significantly increased memory use, though, and it doesn't have to happen as part of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I discuss this a bit in #1136
InvalidProof _ _ _ -> return False | ||
UnfinishedProof _ -> | ||
-- TODO? is this the right behavior? | ||
do printOutLnTop Warn "Could not determine if preconditions are vacuous" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like reasonable behavior to me, at least unless we see evidence to the contrary.
, "Prefer to use 'admit', this command will eventually be removed." | ||
] | ||
|
||
, prim "admit" "String -> ProofScript ()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change! Having an explanation for admitting a proof will help generate useful verification summaries, among other benefits.
, "if '<prop>' is false, where '<example>' is a counter example." | ||
, "If '<prop>' is a curried function, then '<example>' will be a tuple." | ||
, "If the proof of <prop> was not finished, but we did not find a counterexample," | ||
, "the example will run '<false> {{ () }}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will likely lead to a type error in an argument designed to process a counter-example, but perhaps that's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I wasn't entirely sure what else to do here.
It now accepts a `ProofGoal` and computes a `ProofResult`, which is either a completed proof, a counterexample to a subgoal of the proof, or an incomplete proof. All the primitives accepting `ProofScript` arguments are now work independently of what the proof script action computes. In addition, verification methods are more careful to check that all proof obligations are discharged, whereas previously they were not.
to return () instead of SatResult.
This takes a string message from the user describing why this goal is being admitted. We gently suggest in the help text to use `admit` instead of `assume_unsat` or `assume_valid` to start laying the groundwork for deprecating the latter.
to take a `Theorem` argument. Update the help documenation for `caseProofResult` and `caseSatResult` to indicate what happens in case of an "unknown" or "unfinished" result.
We strengthen the invariants on Prop to require that they have type saw-core type `Prop`, which is now possible because we fixed a bug in `scTypeOf`. Implement `boolToProp` to treat boolean terms as propositions.
8995741
to
eed686f
Compare
Round 2 of updating proof conventions. This mostly focuses on fixing #9 and #1099.