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

Elephant in room: changes for stable const-generics? #180

Closed
jon-chuang opened this issue Jan 18, 2021 · 1 comment · Fixed by #379
Closed

Elephant in room: changes for stable const-generics? #180

jon-chuang opened this issue Jan 18, 2021 · 1 comment · Fixed by #379

Comments

@jon-chuang
Copy link
Contributor

jon-chuang commented Jan 18, 2021

I'm surprised this hasn't been brought up, since min_const_generics was merged to master 22 days ago rust-lang/rust#79135 but I suppose that there are probably other more pressing issues. Nonetheless, a truly significant amount of ugly code can be rid off by replacing BigInteger trait with struct BigInteger<const N: usize>([u64; N]). It is unclear at this point if we still want a BigInteger trait or to have the type parameter BigInteger<const N: usize> (this entails moving const param bound to other trait/datatype, hence I think keeping datatype unspecific trait where no significant gains exist is likely path).

I wish at this point that we had collated all the nice things that could be done once we migrate to suitable new impl.

Obvious things:

  • although retroactively annoying to change imports, looking forward, importing just BigInteger rather than tiresomely specifying BigInteger256 etc is nice

Other things:

  • For some somewhat obscure reasons, it is sometimes desirable to treat BigInteger types transparently as arrays.

Issues:

  • Our current method of unrolling loops at the preprocessing stage stops working once we move from literal parametrisation in macro instantiations to constants. So, this is a problem. https://www.reddit.com/r/rust/comments/6ou9qo/crunchy_unroll_constant_loops/. My proposed solution is work around for the const fn in the Fp impl, while for the mul_assign etc which are more performance critical, we impl using a literal for each of N=1..16 using match... and have slow generic impl for N > 16??? It's not very pretty and very code bloaty. But I suppose in terms of pure macro expanded code bloat, it is better than before, when we have separate macro impl for every single num_limbs desired. Since we are creating generic types, compilation time for algebra itself is expected to go down, while on the other hand the individual curves only impl a handful of concrete types.
  • Something that does become very nice on the other hand is that one can now access the const-generic impls of the const-fn versions of functions. So one might be able to dedup a bit. So no, we could also do this previously.

Feel free to add to the list.

Oh yes, of course, these changes only take place in stable 1.51, which is two patches away.

Oh wait sorry, so there's issue #46

@Pratyush
Copy link
Member

This is fixed with #379

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 a pull request may close this issue.

2 participants