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

CheckedAdd should probably not extend Add #274

Open
popematt opened this issue Jun 30, 2023 · 3 comments
Open

CheckedAdd should probably not extend Add #274

popematt opened this issue Jun 30, 2023 · 3 comments

Comments

@popematt
Copy link

popematt commented Jun 30, 2023

...and more generally, checked operation traits should not extend their unchecked counterparts.

I'd like to contribute a PR to implement CheckedAdd for the NonZero___ types in std::num, but it is not possible since they do not implement core::ops::Add (for good reason). The way to unblock my desired change is to make CheckedAdd not extend Add.

I suspect that removing the relationship between CheckedAdd and Add might be a breaking change, so I figured I'd propose the idea before I start a PR for it.

The presence of an unchecked add implies that a checked add is possible, but a checked add does not imply that an unchecked add is possible. I think that, if anything, Add should extend CheckedAdd (though I realize the practical difficulties of that). Given the relationship between them, I wonder if the real "right" solution also involves getting checked operations into core::ops. If it is, then I am willing to pursue that course of action instead of making changes to num-traits.

@cuviper
Copy link
Member

cuviper commented Jun 30, 2023

I suspect that removing the relationship between CheckedAdd and Add might be a breaking change,

Yes, that is a breaking change, sorry.

I wonder if the real "right" solution also involves getting checked operations into core::ops.

Those traits are all related to built-in language operators, so you can impl Add to get a working + operator on your own types. It's not impossible to think of having core traits for more numeric methods, but there's not any current precedent for that. Most of these num traits were in the standard library before Rust 1.0, and were kicked out to live here instead.

@popematt
Copy link
Author

Most of these num traits were in the standard library before Rust 1.0, and were kicked out to live here instead.

Do you know where I can find any of the history of that decision?

@cuviper
Copy link
Member

cuviper commented Jun 30, 2023

I found the commit history, but not much discussion. For this particular trait:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants