Skip to content
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 #1102

Merged
merged 18 commits into from
Mar 15, 2021
Merged

Proof conventions #1102

merged 18 commits into from
Mar 15, 2021

Conversation

robdockins
Copy link
Contributor

Start refactoring some of the core SAWScript proof primitives. This strengthens the abstractions surrounding these soundness-critical operations and paves the way to rework some of the underlying data representations in a safe way.

@robdockins robdockins requested a review from brianhuffman March 3, 2021 23:45
@robdockins
Copy link
Contributor Author

@brianhuffman, this PR isn't really done yet, but there are a number of changes I'm working on to the proof state management and such that I think it would be worthwhile to have your early input on.

@robdockins robdockins force-pushed the proof-conventions branch 5 times, most recently from 8598350 to f4f53d7 Compare March 9, 2021 19:46
@robdockins robdockins force-pushed the proof-conventions branch 2 times, most recently from 37681e4 to 7344a1b Compare March 11, 2021 19:43
@robdockins robdockins marked this pull request as ready for review March 11, 2021 19:48
@robdockins
Copy link
Contributor Author

I think this PR is a cohesive unit of work now, and hopefully passes CI. I don't know that it does everything we want to do on this front yet, but I think it's better to get it reviewed and merged before it gets even bigger.

src/SAWScript/Builtins.hs Outdated Show resolved Hide resolved
src/SAWScript/Proof.hs Outdated Show resolved Hide resolved
do p' <- unfoldProp sc vars p
check hyps e' p'

RewriteEvidence ss e' ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this rule provides a bit of a soundness loophole. Local assumptions can be added to simpsets, and we currently don't have any way to track their provenance, so they might be used out of their scope. To fix this we will need to add some additional bookeeping to the Simpset type, which might be a bit awkward.

{ thmProp :: Prop
, thmStats :: SolverStats
{ _thmProp :: Prop
, _thmStats :: SolverStats
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this field redundant? It seems like you could extract SolverStats from the Evidence field. Is it just used to cache the value so we don't have to recompute it multiple times?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, it feels a bit redundant, and it probably makes sense to remove it at some point. However, at this stage I wanted to minimize observable changes in behavior, so I retained the SolverStats alongside the new Evidence type.

, psTimeout :: Maybe Integer
{ _psGoals :: [ProofGoal]
, _psConcl :: ProofGoal
, _psStats :: SolverStats
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar question here as with the Theorem type: SolverStats seems like a more limited view of Evidence; is it redundant?

Make `Prop` an opaque newtype and make all operations that manipulate
it local to the Proof module.
The `Theorem` and `ProofState` data types are now abstract,
and changes in proof state are mostly mediated by the
new `Tactic` concept.  Theorems now contain `Evidence`
which explains the reasoning for why we accept a particular
proposition as true. Proof states now explicitly construct
evidence as part of tactic execution, and evidence is checked
against the proposition to be proved when finishing a proof.

Forms of evidence we accept are:

1) Explicit proof terms, according to types-as-propositions
2) Black-box propositions either directly admitted,
   random checked via quickcheck or proved by solvers
3) Rewriting, unfolding and conversion steps on goals
4) Sequent calculus reasoning rules (theorem application, cut,
   introduction of local hypotheses, etc).

This isolates the core soundness concepts to the "SAWScript.Proof"
module, where the key invariants can be maintained relatively
easily.
Add documentation, especially for the `Evidence` datatype.
Use the SATQuery type for the "offline" proof exporters, and
a variety of other tweaks.
@robdockins
Copy link
Contributor Author

I think this is ready to go, except perhaps the discussion regarding what to do with SolverStats vs Evidence. I'm inclined to leave things as-is for now and do more cleanup in a later stage, with more of a focus on how that will affect the proof summary code, etc.

@robdockins robdockins added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Mar 15, 2021
@mergify mergify bot merged commit c107866 into master Mar 15, 2021
@mergify mergify bot deleted the proof-conventions branch March 15, 2021 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants