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

Simplify impl_array_size macro and add a value #50

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

bifurcation
Copy link
Contributor

  • Derive the $len input to the macro from the $ty input
  • Allow an optional final comma on the input list
  • Add U1408 to the list of supported array sizes

$(
unsafe impl ArraySize for $ty {
type ArrayType<T> = [T; $len];
type ArrayType<T> = [T; <$ty as Unsigned>::USIZE];
Copy link
Member

Choose a reason for hiding this comment

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

👍

@tarcieri tarcieri merged commit 3215a05 into RustCrypto:master Feb 16, 2024
14 checks passed
@tarcieri
Copy link
Member

I noticed that, for whatever reason, this seems to have caused increased compile times. On my machine timing cargo build and cargo build --release (after clearing target for each run):

  • Before (7ac59a4): 1.0s debug, 0.85s release
  • After (3215a05): 2.77s debug, 2.17s release

Notably this matters for expanding the number extra sizes we define.

tarcieri added a commit that referenced this pull request Feb 25, 2024
Replaces the existing ad hoc array size selections with the following,
which cover all current known use cases:

- 0-256
- 272-1024 (multiples of 16)
- 1040-4096 (multiples of 32)

This also effectively reverts #50, since it was adding considerably to
compile times (now noted as a comment in that PR).
tarcieri added a commit that referenced this pull request Feb 25, 2024
Replaces the existing ad hoc array size selections with the following,
which cover all current known use cases:

- 0-256
- 272-1024 (multiples of 16)
- 1040-4096 (multiples of 32)

This also effectively reverts #50, since it was adding considerably to
compile times (now noted as a comment 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