-
Notifications
You must be signed in to change notification settings - Fork 23
Modes for controlling irrelevant axiom removal #301
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi, while reviewing this PR I became to want to assure the natures of properties Strata want to reason about. If we want to detect malformed API calls whose inputs cannot be right, would aggressively removing axioms affect the result? If the axioms about List membership is correctly encoded, Strata will correctly indicate that |
Axiom removal is sound, as long as we're relying on an Does this address your concern? |
| (fun (k, v) => f!"{k}: [{Std.Format.joinSep (v.map Std.format) ", "}]") | ||
| f!"{Std.Format.joinSep entries ", \n"}" | ||
| /-- Fixed-point computation for axiom relevance. -/ | ||
| private partial def computeRelevantAxioms (prog : Program) (cg : CallGraph) (fmap : FuncAxMap) |
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.
In principle this could be proved terminating by looking at number axioms in prog - |discoveredAxioms| but maybe that is not necessary for now.
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.
Indeed -- I shied away from that proof for now. I expect we won't be using axiom removal too much after datatypes are in. This is mostly for a rainy day.
| assume [assume_maybe_except_none]: (ExceptOrNone_tag(maybe_except) == EN_NONE_TAG); | ||
| }; | ||
|
|
||
| axiom [__end_of_prelude_marker]: (true); |
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.
Somewhere, it should probably be documented that this must occur at the end of the prelude. And what happens if there are multiple preludes/libraries being used?
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.
Totally geared towards a single prelude for now. It's a stopgap, which I acknowledge in Strata/Languages/Boogie/Verifier.lean.
To record - we had a separate chat about this and I recommended that the codebase explicitly mentions the Aggressive mode may change unsat to sat. |
I think Precise can also change unsat to sat. E.g. if you have |
|
Ah, right. I was implicitly assuming that existing axioms were not false. |
Just checking. Yes, so you can do this when you are proving assertions. But not if you are checking coverage/reachability. |
Indeed, and that was June's point too. We don't have coverage checking in Strata yet (imminent), but when we do, we won't run this axiom removal bit on it. |
…ation; turn off axiom removal in StrataMain
Pull request was converted to draft
|
I'll re-open this PR after #314 is merged. |
Description of changes:
Make the detection of relevant axioms for a given function more precise. See test in
irrelevantAxiomsTestPgm2inStrata/StrataTest/Languages/Boogie/Examples/RemoveIrrelevantAxioms.leanfor details.There are three modes to choose from for detecting and removing irrelevant axioms:
Off: do not remove any axioms.Aggressive: Only the functions in the consequentQof a goalP ==> Qwill be taken into account for relevant axiom analysis. This means that axioms relevant to some function inPmay be removed. This is sound, but may yield unknown results from the solver.Precise: Functions in bothPandQof a goalP ==> Qwill be considered for relevant axiom analysis. A caveat is that only the antecedents that appear after a marker called__end_of_prelude_markerwill be considered as the real assumptions here.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.