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

add orDefault lemmas #2946

Merged
merged 8 commits into from
Oct 5, 2022
Merged

add orDefault lemmas #2946

merged 8 commits into from
Oct 5, 2022

Conversation

dwightguth
Copy link
Collaborator

Previously we were missing some lemmas for lookup or default that were present for lookup. We add some lemmas here to the MAP-SYMBOLIC module so that reasoning about the orDefault version of lookup can work correctly.

@rv-jenkins rv-jenkins changed the base branch from master to develop October 3, 2022 19:09
@dwightguth
Copy link
Collaborator Author

@ehildenb what tests would you suggest for this PR?

@dwightguth dwightguth marked this pull request as ready for review October 4, 2022 15:12
@ehildenb
Copy link
Member

ehildenb commented Oct 4, 2022

@dwightguth add tests to k-distribution/tests/regression-new/map-symbolic. You should make sure that each lemma here is exercised by some test. Do not specify the RHS of the tests (similar to how the existing tests work), instead use the exact output of the proof run and compare to a golden file. This is a more robust way tto measure "did the exact simplifications I wanted to apply actually apply?"

Comment on lines +47 to +51
claim <k> lookup ( (X:MyId |-> 3 y |-> 4) [ x <- 5 ] [ X ] orDefault 0 ) => . </k> requires X =/=K x
claim <k> lookup ( (X:MyId |-> 3 y |-> 4) [ x <- 5 ] [ X ] orDefault 0 ) => . </k> requires X =/=K y
claim <k> lookup ( ( (y |-> 4) [ x ] orDefault 0 ) ==K 4 ) => . </k>
claim <k> lookup ( (Y:MyId |-> 1 X:MyId |-> 2) [ Y:MyId ] orDefault 0 ) => . </k>
claim <k> lookup ( (X:MyId |-> 1 Y:MyId |-> 2) [ Z:MyId ] orDefault 0 ) => . </k> requires Z =/=K X andBool Z =/=K Y
Copy link
Member

Choose a reason for hiding this comment

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

We should add a tests which have the in_keys predicate in the side-condition:

claim <k> lookup( M:Map [ X <- 1 ] [ Y ] orDefault 0 ) => . </k> requires COND1 andBool COND2

Where COND1 ranges over [X ==K Y, X =/=K Y], and COND2 ranges over [Y in_keys(M), notBool Y in_keys(M)].

As it stands, we only have one test that exercises the orDefault behavior, and only with a single-element map.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can add these tests, and will, but I wanted to note that, in fact, test 51 also exercises several of the default cases.

Comment on lines +8 to +11

claim <k> lookup ( ( (y |-> 4) [ x ] orDefault 0 ) ==K 4 ) => . </k>

endmodule
Copy link
Member

Choose a reason for hiding this comment

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

If we instead just have (y |-> 4) [ x] orDefault 0 directly, the output will be exactly 0, which is a bit stronger of a test than this one.

But if we add the other 4 tests I mentioned then we should be covering the behaviors of orDefault.

@dwightguth
Copy link
Collaborator Author

I added the tests you requested, but I want to note that none of them actually simplify to 0. The only one that ought in theory to be simplifiable to 0 is the one where X =/= Y and Y is not in M. But the simplification rules I added aren't actually powerful enough to determine that that means the lookup ought to be simplifiable to 0. We can always add more simplification rules later to do so, but I would prefer to add the minimum amount necessary to bring orDefault up to parity with regular lookup while also being sufficient to solve the issue I was having in the C semantics. The lemmas don't need to be complete; we can add more when we discover more things we want to be able to prove.

rule (K1 |-> _V M:Map) [ K2 ] orDefault D => M [K2] orDefault D requires K1 =/=K K2 ensures notBool (K1 in_keys(M)) [simplification]
rule (_MAP:Map [ K <- V1 ]) [ K ] orDefault _ => V1 [simplification]
rule ( MAP:Map [ K1 <- _V1 ]) [ K2 ] orDefault D => MAP [ K2 ] orDefault D requires K1 =/=K K2 [simplification]
rule .Map [ _ ] orDefault D => D [simplification]
Copy link
Member

Choose a reason for hiding this comment

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

One small generalization of this rule, which should enable both this simpliifcation and many others (as well as actually exercising some of the new tests), without being considered an "extension" of the previous lemmass because there is no analogue in the previous lemmsa for this:

Suggested change
rule .Map [ _ ] orDefault D => D [simplification]
rule M [ K ] orDefault D => D requires notBool K in_keys(M) [simplification]

This would let it simplify out more, at the expense of triggering more frequent in_keys(...) checks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Per discussion with Everett over slack we decided to leave this lemma as-is for now since it's current form suffices for the C semantics example I was working with. We can revisit Everett's suggestion if we discover terms that we want to simplify that these lemmas are not powerful enough to handle.

@rv-jenkins rv-jenkins merged commit fff1da8 into develop Oct 5, 2022
@rv-jenkins rv-jenkins deleted the orDefault branch October 5, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants