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

[RPC] Clean up to_cryptol interface in cryptoltypes.py #1312

Merged
merged 5 commits into from
Dec 11, 2021

Conversation

m-yac
Copy link
Contributor

@m-yac m-yac commented Dec 9, 2021

Currently, cryptoltypes.py has the functions to_cryptol, CryptolType.from_python, and CryptolType.convert, all of which do approximately the same thing. Only the first is used in any current code in this repo or in saw-script, so this PR removes the latter two.

Currently, __to_cryptol__ has a CryptolType parameter that does nothing, and to_cryptol has a CryptolType parameter that only matters in the following two cases:

to_cryptol(x, Bitvector(Num(w))) == to_cryptol(BV(w, x)) # if x is an int
to_cryptol(x, Bitvector(Num(w))) == to_cryptol(BV.from_bytes(x, size=w)) # if x is a bytearray or bytes

Otherwise every Python value has a unique CryptolType, so the parameter is unused. These CryptolType parameters are never actually used in any current code in this repo or in saw-script, and I prefer the expressions on the RHS of the equalities above anyway, so this PR also removes these parameters.

NB: This could potentially break someone's code if they use one of the deleted functions or parameters.

@m-yac m-yac requested a review from pnwamk December 9, 2021 23:30
@RyanGlScott
Copy link
Contributor

NB: This could potentially break someone's code if they use one of the deleted functions or parameters.

In that case, do mention these changes in the CHANGELOG.

Copy link
Contributor

@pnwamk pnwamk left a comment

Choose a reason for hiding this comment

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

LGTM module the one comment regarding possibly keeping the methods on CryptolType

@m-yac m-yac merged commit 07c689a into master Dec 11, 2021
@m-yac m-yac deleted the rpc/clean-cryptoltypes-1 branch December 11, 2021 00:01
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