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

Hide statements #5562

Merged
merged 94 commits into from
Jul 24, 2024
Merged

Conversation

keyboardDrummer
Copy link
Member

@keyboardDrummer keyboardDrummer commented Jun 15, 2024

Description

  • Introduce hide statements, which work together with the existing reveal statements. A function that has not been declared opaque, can still be hidden using hide. Hiding all functions can be done using hide *. Hide statements can be used as an alternative to making functions opaque, and unlike opaque, allow optimizing the performance of specific proofs in your codebase without affecting the remaining code.

How has this been tested?

  • Added several CLI tests related to hiding and revealing

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@keyboardDrummer keyboardDrummer added the run-integration-tests Forces running the CI for integration tests even if the deep tests fail label Jul 17, 2024
Copy link
Member

@atomb atomb left a comment

Choose a reason for hiding this comment

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

I love this feature. I noted one message that should be updated to account for the fact that a statement might be hide, and one potential documentation change.

s.IsGhost = s.ResolvedStatements.All(ss => ss.IsGhost);

} else if (stmt is HideRevealStmt hideRevealStmt) {
hideRevealStmt.ResolvedStatements.ForEach(ss => Visit(ss, true, "a reveal statement"));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hideRevealStmt.ResolvedStatements.ForEach(ss => Visit(ss, true, "a reveal statement"));
hideRevealStmt.ResolvedStatements.ForEach(ss => Visit(ss, true, "a hide or reveal statement"));

Copy link
Member

Choose a reason for hiding this comment

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

Or perhaps it could look at the mode and say either "hide" or "reveal" depending on which it is.

method Foo(x: nat) {
hide nat;
if (*) {
assert x > 0; // Fails because nat is hidden
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

var x := Bar();
if (*) {
reveal Bar;
assert x;
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little worried that the difference in behavior of hide on functions vs. methods could cause confusion. It makes sense in the "hide the things that are normally visible" interpretation, though, so I'm not too worried. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

The expect file for this input is:

revealMethods.dfy(8,11): Error: only functions and constants can be revealed
1 resolution/type errors detected in revealMethods.dfy

In the future I'll allow hide and revealing methods, and then a hidden method will also hide the ensures clause.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right! Okay, then I have no concern. :)

@@ -1032,9 +1031,6 @@ This list is not exhaustive but can definitely be useful to provide the next ste
<br><br>`assert A == B;`<br>`callLemma(x);`<br>`assert B == C;`<br> | [`calc == {`](#sec-calc-statement)<br>&nbsp;&nbsp;` A;`<br>&nbsp;&nbsp;` B;`<br>&nbsp;&nbsp;` { callLemma(x); }`<br>&nbsp;&nbsp;` C;`<br>`};`<br>`assert A == B;`<br>where the [`calc`](#sec-calc-statement) statement can be used to make intermediate computation steps explicit. Works with `<`, `>`, `<=`, `>=`, `==>`, `<==` and `<==>` for example.
<br><br><br>`assert A ==> B;` | `if A {`<br>&nbsp;&nbsp;` assert B;`<br>`};`<br>`assert A ==> B;`
<br><br>`assert A && B;` | `assert A;`<br>`assert B;`<br>`assert A && B;`
<br>`assert P(x);`<br>where `P` is an [`{:opaque}`](#sec-opaque) predicate | [`reveal P();`](#sec-reveal-statement)<br>`assert P(x);`<br><br>
`assert P(x);`<br>where `P` is an [`{:opaque}`](#sec-opaque) predicate<br><br> | [`assert P(x) by {`](#sec-assert-statement)<br>[&nbsp;&nbsp;` reveal P();`](#sec-reveal-statement)<br>`}`
`assert P(x);`<br>where `P` is not an [`{:opaque}`](#sec-opaque) predicate with a lot of `&&` in its body and is assumed | Make `P` [`{:opaque}`](#sec-opaque) so that if it's assumed, it can be proven more easily. You can always [reveal](#sec-reveal-statement) it when needed.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe change this to suggesting the use of hide?

Copy link
Member Author

Choose a reason for hiding this comment

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

On the left it could say:

hide P;
...
assert P(x);

and on the right then

hide P;
...
reveal P;
assert P(x);

But I can't imagine anyone would need this tip.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think you're probably right about that.

@keyboardDrummer keyboardDrummer merged commit 281922d into dafny-lang:master Jul 24, 2024
21 checks passed
@keyboardDrummer keyboardDrummer deleted the alwaysOpaque branch July 24, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-integration-tests Forces running the CI for integration tests even if the deep tests fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants