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

Improvements to multiset, map pervasives #752

Merged
merged 3 commits into from
Aug 16, 2023
Merged

Conversation

jonhnet
Copy link
Collaborator

@jonhnet jonhnet commented Aug 16, 2023

multiset: removed finite assumption (and axiom!), replaced with comment explaining that multiset can have infinite support, but not infinite multiplicity. Based::new comprehension on a function (consistent with other pervasive types), reworked other definitions to use it, and removed now-unnecessary axioms.

map: moved values from map.rs (should be just _t axioms) to map_lib (should be _v).

@jonhnet
Copy link
Collaborator Author

jonhnet commented Aug 16, 2023

(sorry map_lib diff is noisy -- I cleaned up a bunch of end-of-line whitespace and a broken end-of-file linefeed along the way. Here's to vargo fmt...)

@jonhnet jonhnet requested review from tjhance and utaal and removed request for tjhance August 16, 2023 16:23
@jonhnet
Copy link
Collaborator Author

jonhnet commented Aug 16, 2023

(removing tjhance as I forgot he was on vacation)

Copy link
Collaborator

@utaal utaal left a comment

Choose a reason for hiding this comment

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

This is technically a breaking change, but these have existed for only a few days, so I'm okay with landing without annoucements/deprecation.

Please squash when merging.

@jonhnet jonhnet merged commit ed64233 into main Aug 16, 2023
4 checks passed
@jonhnet jonhnet deleted the jonh-pervasives-for-splinter branch August 16, 2023 18:15
@tjhance
Copy link
Collaborator

tjhance commented Aug 16, 2023

as I said in the slack, the change to multiset is unsound unless #[verifier::accept_recursive_types(V)] is changed

@utaal
Copy link
Collaborator

utaal commented Aug 16, 2023

as I said in the slack, the change to multiset is unsound unless #[verifier::accept_recursive_types(V)] is changed

Thank you for catching this (and sorry for missing it). It's reverted for now: 56edc01

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.

3 participants