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

Consider promoting Field::{zero, one} to associated constants #87

Closed
tarcieri opened this issue Jul 31, 2022 · 2 comments · Fixed by #94
Closed

Consider promoting Field::{zero, one} to associated constants #87

tarcieri opened this issue Jul 31, 2022 · 2 comments · Fixed by #94

Comments

@tarcieri
Copy link
Contributor

tarcieri commented Jul 31, 2022

Right now Field::zero() and Field::one() are static methods of the Field trait.

If they were instead associated constants, e.g. Field::ZERO/Field::ONE, they can be used in const contexts such as defining other constants:

pub struct ProjectivePoint<Fe: Field> { x: Fe, y: Fe, z: Fe }

impl<Fe: Field> ProjectivePoint<Fe> {
    pub const IDENTITY: Self = Self {
        x: Fe::ZERO,
        y: Fe::ONE,
        z: Fe::ZERO,
    };
}
@str4d
Copy link
Member

str4d commented Oct 28, 2022

I vaguely recall that at the time the traits were first written, there were limitations on how the associated constants could be either constructed or used? I don't recall the specifics, but the course of action depends on several things:

  • If all trait implementors will always be able to instantiate the associated constants, then we can add them.
  • If all trait users will be able to use the associated constants everywhere they can use the trait methods (which definitely seems like it should be true for the versions of Rust we support), then we can remove the trait methods.

str4d added a commit that referenced this issue Oct 28, 2022
We now require that the type implementing `Field`, and its particular
values for these constants, can be constructed in a const context. Once
upon a time this might have been onerous, but it should now be a
reasonable requirement given our MSRV of 1.56.0.

Closes #87.
@str4d
Copy link
Member

str4d commented Oct 28, 2022

I've opened #94 so we can test the first bullet point.

str4d added a commit that referenced this issue Nov 2, 2022
We now require that the type implementing `Field`, and its particular
values for these constants, can be constructed in a const context. Once
upon a time this might have been onerous, but it should now be a
reasonable requirement given our MSRV of 1.56.0.

Closes #87.
str4d added a commit that referenced this issue Nov 2, 2022
We now require that the type implementing `Field`, and its particular
values for these constants, can be constructed in a const context. Once
upon a time this might have been onerous, but it should now be a
reasonable requirement given our MSRV of 1.56.0.

Closes #87.
@str4d str4d closed this as completed in #94 Nov 2, 2022
@str4d str4d closed this as completed in 6bf93ee Nov 2, 2022
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