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

Remove uses of coerceSize #107

Merged
merged 3 commits into from
Aug 30, 2024
Merged

Remove uses of coerceSize #107

merged 3 commits into from
Aug 30, 2024

Conversation

marsella
Copy link
Contributor

@marsella marsella commented Aug 5, 2024

Closes #101

This removes all the uses of coerceSize. It also remove a bunch of trailing whitespace, because my editor does that automatically, and adds a copyright notice.

This is on hold for now because as mentioned in the issue, it depends on a nightly-only feature of cryptol. Now ready for review after the release of Cryptol 3.2.0!

Common/ntt.cry Show resolved Hide resolved
- I misread `max 1 n - 1` when making the previous change. Removes the
  `max`s that are obsoleted by the numeric constraint guards I added.
- Adds docstrings for properties in `ntt` that I used to debug.
@marsella marsella marked this pull request as ready for review August 29, 2024 18:33
@@ -115,13 +116,14 @@ shuffle a =
/**
* Perform the butterfly operation.
*/
butterfly : {n} (fin n, n > 0) => Integer -> [n]Fld -> [n]Fld -> [2 * n]Fld
Copy link
Member

Choose a reason for hiding this comment

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

Why did we make the type of butterfly less general here (i.e., it seems looks like the function makes sense for any length > 0, rather than just powers of 2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh gosh, good question. I don't remember what I was thinking here, but probably I was just iterating on trying to understand the implementation and accidentally left this in. I'll change it back, thanks for catching this!

@marsella marsella merged commit 849123c into master Aug 30, 2024
2 checks passed
@marsella marsella deleted the 101-remove-coerceSize branch August 30, 2024 17:42
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.

Clean up uses of coerceSize
2 participants