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

Explicit stride #1227

Merged
merged 17 commits into from
Jul 20, 2021
Merged

Explicit stride #1227

merged 17 commits into from
Jul 20, 2021

Conversation

robdockins
Copy link
Contributor

This PR adds new syntactic forms for enumerations with explicit stride values (CF #1087).

The currently-implemented syntax steals the identifiers by and down as new keywords; we should discuss if this is acceptable, or if new syntax should be designed.

@robdockins robdockins marked this pull request as ready for review June 30, 2021 16:27
@robdockins
Copy link
Contributor Author

Documentation updates are still needed, but I wanted to make sure we are happy with the syntax choices before doing that.

@yav
Copy link
Member

yav commented Jun 30, 2021

@robdockins the notation makes sense to me. The only improvement I can think of is to clarify in the comment-docs for the "inclusive" constructs (the non < ones) that the end point might not actually be in the final result (as in [ 4 .. 1 down by 2] is [4,2], which didn't get to the 1)

Copy link
Member

@yav yav 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 there are a few more changes that we need for the solver, to match the more relaxed constraints on /^ and /%.

Otherwise it looks good to me, with the caveat that I didn't go over the evaluation semantics in detail

src/Cryptol/TypeCheck/Solver/InfNat.hs Show resolved Hide resolved
src/Cryptol/TypeCheck/Solver/InfNat.hs Show resolved Hide resolved
@robdockins robdockins requested a review from yav July 1, 2021 00:41
@robdockins
Copy link
Contributor Author

AFAICT, the remaining failures are not related to this PR.

@robdockins
Copy link
Contributor Author

@yav, @brianhuffman, any additional thoughts about this?

lib/Cryptol.cry Show resolved Hide resolved
lib/Cryptol.cry Show resolved Hide resolved
Copy link
Member

@yav yav left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@robdockins robdockins merged commit 20b8b82 into master Jul 20, 2021
robdockins added a commit to GaloisInc/saw-script that referenced this pull request Jul 21, 2021
Add stub implementations of the new enumeration primitives
from GaloisInc/cryptol#1227, and fix up the type of `ecFromTo`,
which also changed slightly in that PR.
robdockins added a commit to GaloisInc/saw-script that referenced this pull request Jul 28, 2021
Add stub implementations of the new enumeration primitives
from GaloisInc/cryptol#1227, and fix up the type of `ecFromTo`,
which also changed slightly in that PR.
robdockins added a commit to GaloisInc/saw-script that referenced this pull request Jul 29, 2021
Add stub implementations of the new enumeration primitives
from GaloisInc/cryptol#1227, and fix up the type of `ecFromTo`,
which also changed slightly in that PR.
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.

2 participants