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

Add ConstZero and ConstOne traits #303

Merged
merged 6 commits into from Feb 8, 2024
Merged

Conversation

tarcieri
Copy link
Contributor

@tarcieri tarcieri commented Dec 14, 2023

Adds traits which are alternatives to the more dynamic Zero/One traits which are useful for stack-allocated types where it's possible to define constant values for zero/one.

ZeroConstant ConstZero bounds on Zero as a supertrait, and OneConstant ConstOne on One, allowing them to be used as drop-in replacements.

When a type also impls PartialEq, then ZeroConstant also provides a blanket impl of Zero, and likewise for OneConstant/One, making it simple for stack-allocated integers to impl these traits as an alternative to Zero/One while still remaining fully compatible. (Edit: removed)

The internal impls of Zero/One on the numeric primitive types have been changed to use these traits, which should be a fully backwards-compatible change. (Edit: removed)

Alternative to #276.

@cuviper
Copy link
Member

cuviper commented Dec 15, 2023

Do you also want to re-export these in the root? I think most people don't use the module hierarchy from here, which is honestly kind of a mess...

When a type also impls PartialEq, then ZeroConstant also provides a blanket impl of Zero, and likewise for OneConstant/One, making it simple for stack-allocated integers to impl these traits as an alternative to Zero/One while still remaining fully compatible.

The problem is that these are not perfectly overlapping, especially with generics, like:

impl<T: Zero> Zero for Complex<T> { ... }
impl<T: ZeroConstant> ZeroConstant for Complex<T> { ... }

The first would include non-const types, but a blanket impl<T: ZeroConstant> Zero for T makes these conflict.

It's similar to Niko's old post about wanting impl<T: Copy> Clone for T:
https://smallcultfollowing.com/babysteps//blog/2016/09/24/intersection-impls/

@tarcieri
Copy link
Contributor Author

I have removed the blanket impls, and in their place impl'd ZeroConstant for Wrapping instead.

Sidebar: would names like ZeroConst and OneConst be better?

@cuviper
Copy link
Member

cuviper commented Dec 29, 2023

Sidebar: would names like ZeroConst and OneConst be better?

Probably yes, or ConstZero and ConstOne. There's not a lot of precedent that I can find, but some unstable APIs in std do use an abbreviated prefix, like ConstParamTy.

@tarcieri tarcieri changed the title Add ZeroConstant and OneConstant traits Add ConstZero and ConstOne traits Dec 31, 2023
src/identities.rs Outdated Show resolved Hide resolved
src/identities.rs Outdated Show resolved Hide resolved
tarcieri added a commit to RustCrypto/crypto-bigint that referenced this pull request Jan 9, 2024
Renames the trait to match the prospective name it might potentially
receive upstream in `num-traits`: rust-num/num-traits#303
tarcieri added a commit to RustCrypto/crypto-bigint that referenced this pull request Jan 9, 2024
Renames the trait to match the prospective name it might potentially
receive upstream in `num-traits`: rust-num/num-traits#303
Adds traits which are alternatives to the more dynamic `Zero`/`One`
traits which are useful for stack-allocated types where it's possible to
define constant values for zero/one.

`ZeroConstant` bounds on `Zero` as a supertrait, and `OneConstant` on
`One`, allowing them to be used as drop-in replacements.

When a type also impls `PartialEq`, then `ZeroConstant` also provides a
blanket impl of `Zero`, and likewise for `OneConstant`/`One`, making
it simple for stack-allocated integers to impl these traits as an
alternative to `Zero`/`One` while still remaining fully compatible.

The internal impls of `Zero`/`One` on the numeric primitive types have
been changed to use these traits, which should be a fully
backwards-compatible change.
Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

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

Thanks! I rebased to pick up CI changes, and added a couple small fixes.

@cuviper cuviper added this pull request to the merge queue Feb 8, 2024
Merged via the queue into rust-num:master with commit a095b70 Feb 8, 2024
4 checks passed
@tarcieri tarcieri deleted the constant-traits branch February 8, 2024 01:11
tarcieri added a commit to RustCrypto/crypto-bigint that referenced this pull request Feb 8, 2024
tarcieri added a commit to RustCrypto/crypto-bigint that referenced this pull request Feb 8, 2024
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.

None yet

2 participants