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

Use model to simplify report #479

Merged
merged 3 commits into from
Aug 10, 2024
Merged

Use model to simplify report #479

merged 3 commits into from
Aug 10, 2024

Conversation

yav
Copy link
Collaborator

@yav yav commented Aug 9, 2024

This PR does the following:

  • Adds a .mli file and some docs to the Explain and Report modules
  • Modifies Explain to do simplify things based on the current model.
    • For constraints, we eliminate disjunctions, implications, and if-then-else, by eliminating the alternatives that are not viable in the model
    • For the resource hint, we eliminate constraints that are true in the model

yav added 3 commits August 9, 2024 11:18
Type `state_entry` does not seem to be used for anything.
In particular, we eliminate disjunctions, implications,
and if-then-else.
@cp526
Copy link
Collaborator

cp526 commented Aug 10, 2024

Thanks! That looks like it should be very useful.

I wonder whether we might want to set it up so there's a switch in the top bar of the html that toggles between two views:
(1) Showing the state, including constraints and resources just as CN "knows" them, not specialised to the counterexample.
(2) The state specialised to the counterexample, including specialised versions of constraints and resources.

That could be quite easy to implement, by just always recording two versions of some of the components in Report.report, and letting the html switch between these.

The UI should (eventually) be useful for two purposes:

(a) For debugging a verification failure. Here I imagine (2) would often be really useful, to reduce the noise in what's displayed and to make it easier to understand the gap in the proof. One probably still wants to sometimes also see (1). For instance when the verification requires a new lemma, if that's an inductive fact, it might be useful to see (1) to work out how to generalise the current counterexample.

(b) A new CN user should be able to use the interactive UI to develop an intuition for how CN works: for instance by stepping through some (verified or unverified) example to see what CN "knows" at any point along the symbolic execution path. For this (1) is more useful than (2), because the point is not to understand a concrete failure.

Separately: it's probably useful to also be able to switch between showing specialised and non-specialised versions of the unproved constraint or missing resource causing the error (the extras component in Explain).

@cp526 cp526 merged commit 89a1278 into master Aug 10, 2024
2 checks passed
| None -> [ mk (ITE (go1 cond, go1 ifT, go1 ifF)) ])
| _ -> [ mk term ]
in
match lct with LC.Forall _ -> [ lct ] | LC.T ct -> List.map (fun x -> LC.T x) (go ct)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to extend go to know about binders (and not try to evaluate those), so we can apply the evaluation to Forall and to iterated resources.

@PeterSewell
Copy link
Contributor

PeterSewell commented Aug 11, 2024 via email

Comment on lines +31 to +34
{ where : where_report; (** Location information *)
resources : Pp.document list * Pp.document list; (** Resources *)
constraints : Pp.document list * Pp.document list; (** Constraints *)
terms : term_entry list * term_entry list (** Term values *)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for future reference, my preference is that comments should be there to explain why rather than what and variable names should be self explanatory as far as possible. If the variable name would end up being too long, then a short name plus a comment (e.g. as in lines 39-43) is good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants