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

Tracking issue for core::ops::*<T> on Saturating<T> / Wrapping<T> type #91586

Open
4 of 11 tasks
kellerkindt opened this issue Dec 6, 2021 · 0 comments
Open
4 of 11 tasks
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@kellerkindt
Copy link
Contributor

kellerkindt commented Dec 6, 2021

Relevant RFC "928" (rust-lang/rfcs#928).

Public API

The Wrapping and Saturating types enforce that a certain behaviour for all operations on the underlying integer is performed - .wrapping_* or .saturating_*. When doing the "simplified" operations, .add always becomes .wrapping_add or .saturating_add, .sub always becomes .wrapping_sub / .saturating_sub and so on.

The fundamental ideas seem to be:

  1. Never perform the an integer operation with the wrong behaviour by forgetting to write .wrapping_* or .saturating_*
  2. Reduce boiler-plate code / text by being able to omit writing .wrapping_* or .saturating_*

But number 2 seems to fall short in reality. The traits in core::ops:: are only implemented for Wrapping and Saturating where the other type is Wrapping or Saturating as well (impl core::ops::*<Wrapping<T>> for Wrapping<T>¹). So, instead of writing a integer addition like this:

    let mut value = 17u8;
    // ...
    value += 8;
    value *= 7;
    // ...
    let other: u8 = value / 3;

for Wrapping and Saturating types, it looks likes this:

    let mut value = Wrapping(17u8);
    // ...
    value += Wrapping(8);
    value *= Wrapping(7);
    // ...
    let other: Wrapping<u8> = value / Wrapping(3);

This introduces the need for additional mentions of Wrapping / Saturating in every code location where an integer operation is performed - and in my opinion, adds unnecessary noise to the code. In my opinion, having the integer being wrapped in either Wrapping or Saturating in the declaration / initialization should be all that is needed.

I propose that - in addition to impl core::ops::*<Wrapping<T>> for Wrapping<T>¹ - the integer operations are also implemented using the raw integer type itself (impl core::ops::*<T> for Wrapping<T>¹), and therefore the following is being made possible:

    let mut value = Wrapping(17u8);
    // ...
    value += 8;
    value *= 7;
    // ...
    let other: Wrapping<u8> = value / 3;

¹ as well as for Saturating

Steps / History

Unresolved Questions

@kellerkindt kellerkindt added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 6, 2021
@dtolnay dtolnay changed the title Proposal: Implement core::ops::*<T> on Saturating<T> / Wrapping<T> type Tracking issue for core::ops::*<T> on Saturating<T> / Wrapping<T> type Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

1 participant