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

Newtype fix #1396

Merged
merged 4 commits into from
Jul 30, 2021
Merged

Newtype fix #1396

merged 4 commits into from
Jul 30, 2021

Conversation

robdockins
Copy link
Contributor

Fix support for record selectors applied to newtypes. Fixes #1256.

@robdockins robdockins requested a review from atomb July 23, 2021 20:32
Copy link
Contributor

@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 think this looks good, as long as it passes all the tests. It seems to also update the code to handle translation of the new Cryptol enumeration functions even though the description doesn't mention that. Was that intentional?

@robdockins
Copy link
Contributor Author

Oops, not really. I just accidentally branched from #1391 instead of master.

@atomb
Copy link
Contributor

atomb commented Jul 23, 2021

Oops, not really. I just accidentally branched from #1391 instead of master.

Ah, yeah, Brian's probably the right one to review that code, anyway. :)

@robdockins robdockins added the PR: submodule bump Pull requests that include a submodule bump label Jul 23, 2021
Update simple-smt version and enable `-threaded` runtime.
This tracks the changes from GaloisInc/cryptol#1225

The threaded runtime is necessary to keep `waitForProcess`
from stalling all threads in simple-smt.
Use sites are no longer expected to push selectors down into
functions and sequences; that has been taken care of inside
the Cryptol typechecker now for some time.

Add a case for selectors applied to newtypes.  Newtype values
are currently translated exactly as their underlying records, so
we can emit essentially the same code, we just need to look up
the field index in the newtype definition.
@robdockins robdockins added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Jul 29, 2021
@robdockins
Copy link
Contributor Author

Remaining failures seem unrelated, as best I can tell.

@robdockins robdockins merged commit 65d764b into master Jul 30, 2021
@mergify mergify bot deleted the newtype-fix branch July 30, 2021 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run PR: submodule bump Pull requests that include a submodule bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for newtypes
2 participants