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 Saturating type #87920

Closed
2 of 6 tasks
kellerkindt opened this issue Aug 10, 2021 · 26 comments · Fixed by #115477
Closed
2 of 6 tasks

Tracking Issue for Saturating type #87920

kellerkindt opened this issue Aug 10, 2021 · 26 comments · Fixed by #115477
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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 Aug 10, 2021

This is a tracking issue for the RFC "928" (rust-lang/rfcs#928).
The feature gate for the issue is #![feature(saturating_int_impl)].

About tracking issues

Tracks progress for Saturating type analogue to #32463

Unresolved Questions

Steps

Implementation history

@kellerkindt kellerkindt added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Aug 10, 2021
@the8472 the8472 changed the title Tracking Issue for Starutating type Tracking Issue for Saturating type Aug 10, 2021
@kennytm kennytm added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Aug 28, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 28, 2021
Add Saturating type (based on Wrapping type)

Tracking rust-lang#87920

### Unresolved Questions
<!--
Include any open questions that need to be answered before the feature can be
stabilised.
-->

 - [x] ~`impl Div for Saturating<T>` falls back on inner integer division - which seems alright?~
    - [x] add `saturating_div`? (to respect division by `-1`)
 - [x] There is no `::saturating_shl` and `::saturating_shr`. (How to) implement `Shl`, `ShlAssign`, `Shr` and `ShrAssign`?
   - [naively](3f7d2ce)
 - [x] ~`saturating_neg` is only implemented on [signed integer types](https://doc.rust-lang.org/std/?search=saturating_n)~
 - [x] Is the implementation copied over from the `Wrapping`-type correct for `Saturating`?
   - [x] `Saturating::rotate_left`
   - [x] `Saturating::rotate_right`
   - [x] `Not`
   - [x] `BitXorOr` and `BitXorOrAssign`
   - [x] `BitOr` and `BitOrAssign`
   - [x] `BitAnd` and `BitAndAssign`
   - [x] `Saturating::swap_bytes`
   - [x] `Saturating::reverse_bits`
@joshtriplett
Copy link
Member

FCP for stabilizing saturating_div:
@rfcbot merge

@rfcbot
Copy link

rfcbot commented Sep 4, 2021

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 4, 2021
@Amanieu
Copy link
Member

Amanieu commented Sep 4, 2021

What about the remaining unresolved question regarding the correct behavior of Shl?

@kellerkindt
Copy link
Contributor Author

@kennytm outlined possible implementations for that (#87921 (comment)), but I am not sure what seems to be the correct implementation and I would be happy for any input in that regards.

@Amanieu
Copy link
Member

Amanieu commented Sep 4, 2021

I would like to see the situation with saturating shift clarified first before we stabilize this. There is an unsubmited RFC about this (cc @avitex) which has been discussed a bit in Zulip (1 2) but without coming to a conclusion.

@rfcbot concern saturating shifts

@kellerkindt
Copy link
Contributor Author

kellerkindt commented Sep 4, 2021

Hmm, it might be a bit confusing, but I initially intended to only stabilize saturating_div (#88624) and not the whole Saturating type (yet).

@joshtriplett also mentioned it that way here?

@Amanieu
Copy link
Member

Amanieu commented Sep 4, 2021

Oh right, I missed that message. TBF it's a bit confusing to have two very different features sharing the same tracking issue.

@rfcbot resove saturating shift

@kellerkindt
Copy link
Contributor Author

kellerkindt commented Sep 4, 2021

Oh right, I missed that message. TBF it's a bit confusing to have two very different features sharing the same tracking issue.

Yes, you are right about that, sorry 😅🙈

@avitex I would appreciate your implementation for shl and shr for the number types to use it in the Saturating type 🙂👍

@Amanieu You seem to have a typo in your second rfcbot command?

@Amanieu
Copy link
Member

Amanieu commented Sep 6, 2021

@rfcbot resolve saturating shifts

@yaahc
Copy link
Member

yaahc commented Sep 29, 2021

Oh right, I missed that message. TBF it's a bit confusing to have two very different features sharing the same tracking issue.

@rfcbot resove saturating shift

Should we go ahead and split this tracking issue?

@kellerkindt
Copy link
Contributor Author

That's fine with me, I had the wrong impression that the rfcbot would be run on #88624. What effect does it have on the running poll?

@yaahc
Copy link
Member

yaahc commented Sep 29, 2021

That's fine with me, I had the wrong impression that the rfcbot would be run on #88624. What effect does it have on the running poll?

We would close the poll on this tracking issue and re-open it on the new one and check off the boxes for those who've already approved it here, so effectively none.

@kellerkindt
Copy link
Contributor Author

kellerkindt commented Sep 29, 2021

Okay, I have just created a new tracking issue: #89381

@yaahc
Copy link
Member

yaahc commented Sep 29, 2021

@rfcbot cancel

@rfcbot
Copy link

rfcbot commented Sep 29, 2021

@yaahc proposal cancelled.

@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 29, 2021
@Rudxain
Copy link
Contributor

Rudxain commented Oct 23, 2022

I have a question. Why was the name saturating chosen? I had no idea what it was until I read the docs and realized it's just clamped arithmetic. Since it's still a nightly feature, can it be renamed to clamping?

@kellerkindt
Copy link
Contributor Author

kellerkindt commented Oct 23, 2022

I have a question. Why was the name saturating chosen? I had no idea what it was until I read the docs and realized it's just clamped arithmetic. Since it's still a nightly feature, can it be renamed to clamping?

Because all/most the underlying calls on the Integer are also already prefixed with saturating_* :)

https://doc.rust-lang.org/stable/std/intrinsics/fn.saturating_add.html

@Amanieu
Copy link
Member

Amanieu commented Aug 13, 2023

The only contentious part of Saturating was the shifts, which are explicitly excluded. The most likely future plan for those is to expose them as methods with informative names.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Aug 13, 2023

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 13, 2023
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Aug 22, 2023
@rfcbot
Copy link

rfcbot commented Aug 22, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@Amanieu Amanieu removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Aug 29, 2023
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Sep 1, 2023
@rfcbot
Copy link

rfcbot commented Sep 1, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

kellerkindt added a commit to kellerkindt/rust that referenced this issue Sep 2, 2023
Also stabilizes saturating_int_assign_impl, rust-langgh-92354.

And also make pub fns const where the underlying saturating_*
fns became const in the meantime since the Saturating type was
created.
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 7, 2023
@bors bors closed this as completed in 6011fd4 Sep 17, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 17, 2023
Rollup merge of rust-lang#115477 - kellerkindt:stabilized_int_impl, r=dtolnay

Stabilize the `Saturating` type

Closes rust-lang#87920
Closes rust-lang#92354

Stabilization report rust-lang#87920 (comment)
FCP rust-lang#87920 (comment)
stlankes pushed a commit to hermit-os/rust that referenced this issue Sep 18, 2023
Also stabilizes saturating_int_assign_impl, rust-langgh-92354.

And also make pub fns const where the underlying saturating_*
fns became const in the meantime since the Saturating type was
created.
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Sep 25, 2023
Stabilize the `Saturating` type

Closes #87920
Closes #92354

Stabilization report rust-lang/rust#87920 (comment)
FCP rust-lang/rust#87920 (comment)
flip1995 pushed a commit to flip1995/rust that referenced this issue Sep 25, 2023
Also stabilizes saturating_int_assign_impl, rust-langgh-92354.

And also make pub fns const where the underlying saturating_*
fns became const in the meantime since the Saturating type was
created.
mauricelam added a commit to mauricelam/rust-arithmetic-mode that referenced this issue Nov 3, 2023
The implementation in stdlib has been removed as part of stabilizing
rust-lang/rust#87920. Support for saturating
shifts will be added back once rust-lang/libs-team#230
is fixed.
bitboom added a commit to islet-project/islet that referenced this issue Apr 19, 2024
This patch resolves below:

```
   Compiling serde v1.0.198
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /home/sangwan/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.198/src/lib.rs:279:13
    |
279 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes t
```

Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
bitboom added a commit to islet-project/islet that referenced this issue Apr 19, 2024
This patch resolves below:

```
   Compiling serde v1.0.198
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /home/sangwan/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.198/src/lib.rs:279:13
    |
279 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes t
```

Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
bitboom added a commit to islet-project/islet that referenced this issue Apr 19, 2024
This patch resolves below:

```
   Compiling serde v1.0.198
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /home/sangwan/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.198/src/lib.rs:279:13
    |
279 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes t
```

Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
zpzigi754 added a commit to zpzigi754/islet that referenced this issue May 13, 2024
This fix is for resolving the below error in AnalysisHub.

```
$ ./scripts/deps/rust.sh
...
   Compiling tempfile v3.10.1
   Compiling serde_derive v1.0.201
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.201/src/lib.rs:280:13
    |
280 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes to enable
...
```

Signed-off-by: Changho Choi <ch754.choi@samsung.com>
zpzigi754 added a commit to zpzigi754/islet that referenced this issue May 13, 2024
This update will fix the below error in AnalysisHub.

```
$ ./scripts/deps/rust.sh
...
   Compiling tempfile v3.10.1
   Compiling serde_derive v1.0.201
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.201/src/lib.rs:280:13
    |
280 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes to enable
...
```
zpzigi754 added a commit to zpzigi754/islet that referenced this issue May 13, 2024
This update will fix the below error in AnalysisHub.

```
$ ./scripts/deps/rust.sh
...
   Compiling tempfile v3.10.1
   Compiling serde_derive v1.0.201
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.201/src/lib.rs:280:13
    |
280 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes to enable
...
```

Signed-off-by: Changho Choi <ch754.choi@samsung.com>
zpzigi754 added a commit to islet-project/islet that referenced this issue May 13, 2024
This fix is for resolving the below error in AnalysisHub.

```
$ ./scripts/deps/rust.sh
...
   Compiling tempfile v3.10.1
   Compiling serde_derive v1.0.201
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.201/src/lib.rs:280:13
    |
280 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes to enable
...
```

Signed-off-by: Changho Choi <ch754.choi@samsung.com>
zpzigi754 added a commit to zpzigi754/islet that referenced this issue May 13, 2024
This update will fix the below error in AnalysisHub.

```
$ ./scripts/deps/rust.sh
...
   Compiling tempfile v3.10.1
   Compiling serde_derive v1.0.201
error[E0658]: use of unstable library feature 'saturating_int_impl'
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.201/src/lib.rs:280:13
    |
280 |     pub use self::core::num::Saturating;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #87920 <rust-lang/rust#87920> for more information
    = help: add `#![feature(saturating_int_impl)]` to the crate attributes to enable
...
```

Signed-off-by: Changho Choi <ch754.choi@samsung.com>
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 disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants