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

SizeT: make SizeT.t a new type #3285

Merged
merged 2 commits into from
May 14, 2024
Merged

SizeT: make SizeT.t a new type #3285

merged 2 commits into from
May 14, 2024

Conversation

mtzguido
Copy link
Member

@mtzguido mtzguido commented May 3, 2024

This PR improves responsiveness whenever we use a SizeT.t in a context where it is wrong to do so, like if the expected type is int (and we forgot the v). Instead of trying to prove int == SizeT.t by SMT, F* will now just fail during typechecking/unification. All other integer types already use new.

To do this, we must redefine SizeT.t as a fresh inductive, otherwise we cannot claim that it is new. I wonder if there's a reason against this, since I remember that this module had some custom extraction rules. Maybe @R1kM or @msprotz know?

@msprotz
Copy link
Collaborator

msprotz commented May 3, 2024

I think the custom extraction rules are in FStar.Extraction.Krml.fs and as long as you can still pattern-match just like before, it should be good.

I think before the new keyword existed, you had to do something like:

type t = t'
type t' = | T: ...

and then sometimes in reduction you would see one or another which complicated the custom extraction. But that was ages ago, and I'm not even sure I remember this correctly.

This allows to fail faster (without SMT) when we mistakenly use a SizeT
where an int is needed, and similar errors.
@mtzguido
Copy link
Member Author

I got an everest green locally, so I think this should be fine. Also HACL* dist had no diff, but I'm not sure if it's still regenerated on every make? I'm merging, please report if you find anything weird.

@mtzguido mtzguido merged commit 23d4ec1 into FStarLang:master May 14, 2024
2 checks passed
@mtzguido mtzguido deleted the sizet branch May 14, 2024 21:54
@msprotz
Copy link
Collaborator

msprotz commented May 14, 2024

sadly HACL* has not been switched over to use sizet so I'm not surprised there's no diff there -- ping @cmovcc though for the starmalloc diff

@cmovcc
Copy link
Contributor

cmovcc commented May 16, 2024

Can confirm that StarMalloc's dist/ is left unchanged by this PR. Thanks for the ping Jonathan!

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.

None yet

3 participants