[go: nahoru, domu]

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

Field Trait API UX #41

Open
gluax opened this issue Feb 10, 2023 · 3 comments
Open

Field Trait API UX #41

gluax opened this issue Feb 10, 2023 · 3 comments

Comments

@gluax
Copy link
gluax commented Feb 10, 2023

I think that for the Field trait, rather than defining methods like add, sub, mult, inv, etc., they should leverage the core::ops::Add and others.

If a trait for a method does not exist, it should be defined in a similar style to one from core::ops.

NOTE:
The use of core instead of std here, and I would recommend using core for all things where possible as it would make supporting no_std easier.
They point to the same implementations, but core is specifically for no_std environments.

@schouhy
Copy link
Contributor
schouhy commented Feb 10, 2023

leveraging Add and its friends was our approach too at first. We hit a snag when we implemented complex formulas for field extensions and elliptic curves. We noticed that unless the operations are defined for all combinations of (&F, &F), (F, &F), (&F, F), (F, F), the formulas end up being very unreadable because of explosions of parenthesis and & symbols. The syntax to add all those trait bounds seemed very convoluted, specially for the (&T, &T) case where we need to ask for a type T to have implemented Add for &T. Also, for fields, all the operations only make sense when OutputType equal to F, which made the syntax also way more verbose.
We came up with this solution that in our opinion made things simple for the trait code and for the users of the trait to implement it. And formulas can be read easily.
All of the std::ops are implemented off the operations in the trait now. I wonder if changing those to core::ops would suffice to have support for no_std easier. But I'm not familiar with that.

@unbalancedparentheses
Copy link
Member

I wouldn't suffice but it would make things simpler. Let's create an issue to migrate to core.

@gluax
Copy link
Author
gluax commented Feb 22, 2023

Regarding the complexity around Add for the different combinations, it can be complex, but rust has tools to make it easier.

We could leverage macro_rules like most libraries, including how rust implements the trait for its core types(u8, u16, etc.). Or, if we are particularly inspired, we could write a proc macro for something like #[derive(Add)] at the expense of compile time.

Despite the extra work, it's better in the long term if we ever decide to layer traits. For example, let's say we want to define a Hash2Curve trait which requires some mathematical operations on Field types. Without it having the operations on the Field trait itself, we'd have to define Hash2Curve in a way like:

pub trait Hash2Curve {
  type Field: Field + Add<Rhs = Self::Field> + ...;

  ...
}

We could not define those trait bounds on the trait, but then type inference within the compiler or specific use cases would deny the usage of those traits. This means we could leverage the .add method defined on the Field trait, but ergonomically this leaves much to be desired.

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

No branches or pull requests

3 participants