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

Introduce new trait for integers in field module #853

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

divergentdave
Copy link
Contributor

This introduces a new Integer trait that accompanies FieldElementWithInteger. Most of the trait bounds on the integer type are moved to be super-traits of Integer, related error associated types are moved into the trait, and the zero_integer() and one_integer() static methods are renamed and moved in as well. This is a follow-up to #819, to make breaking changes I deferred then.

@divergentdave divergentdave requested a review from a team as a code owner November 28, 2023 22:46
type TryIntoU64Error: std::error::Error;

/// Returns zero.
fn zero() -> Self;
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW some boilerplate can be elided with a default implementation like so

    fn zero() -> Self {
        0.try_into().unwrap()
    }

but that doesn't really seem like much of an improvement. I might feel better about it if we could also kill the type TryInto... boilerplate with default associated types, but that's an unstable feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The associated types for errors aren't strictly necessary, it would be possible for users to write something like <F::Integer as TryFrom<usize>>::Error instead of F::TryFromUsizeError, but the associated types as pseudo-aliases makes things easier to read, so I'm okay with keeping them around. I think I'd prefer to keep zero() and one() as required methods, and just use literals in each impl block, since it feels less hacky.

@divergentdave divergentdave merged commit 711aec2 into main Dec 4, 2023
6 checks passed
@divergentdave divergentdave deleted the david/integer-trait branch December 4, 2023 18:30
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.

3 participants