-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Stabilize std::num #23549
Stabilize std::num #23549
Conversation
Note:
|
|
||
#[inline(always)] | ||
fn shl(self, other: $f) -> Wrapping<$t> { | ||
Wrapping(self.0 << other) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like these aren't actually doing the wrapping operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I suppose they are, nvm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: #22020
Update: I've resolved the staging issue. There appears to be a bad interaction between int fallback and inherent impls. In this case adding a single type annotation was enough to solve the problem. This should probably be investigated separately in a minimal test case. |
Minimal test case here: #23545 (comment) |
@@ -2655,6 +2656,7 @@ pub fn range_step<A: Int>(start: A, stop: A, step: A) -> RangeStep<A> { | |||
} | |||
|
|||
#[stable(feature = "rust1", since = "1.0.0")] | |||
#[allow(deprecated)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems somewhat worrisome to use these deprecated bounds in stable implementations. I think we had a problem in the past, for example, about an impl of Iterator
for each concrete primitive type as a type parameter causes type inference to go awry. I forget the details, but we may want to deal with this sort of fallout sooner rather than later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree; see #23549 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only worry is that this sort of usage of Int
is not warned about in the unstable lint, so we could alter this functionality of code using 100% stable code and we could still break them. This is pretty minor breakage though so it's not necessarily that bad.
Could this also handle #23454? (it seems they should both be removed?) |
☔ The latest upstream changes (presumably #23512) made this pull request unmergeable. Please resolve the merge conflicts. |
1fa1851
to
ccc8ad1
Compare
Updated, adding @alexcrichton I believe this addresses your concerns. |
f5d7e07
to
911f6cd
Compare
☔ The latest upstream changes (presumably #23654) made this pull request unmergeable. Please resolve the merge conflicts. |
Rebased. |
let length = (self.end - self.start).to_usize(); | ||
(length.unwrap_or(0), length) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may end up having unfortunate perf side effects, but we may be able to regain it later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. There are various possibilities for regaining it (including something like ToPrimitive
, or perhaps based on our new conversion traits).
That said, do you think we should worry about the hit now? This PR could be tweaked in that direction...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have a solution on hand I think we should implement it now, but otherwise I think we'll just have to hold off and wait.
A few nits here and there, but otherwise this all looks great to me! r=me with nits updated |
|
⛄ The build was interrupted to prioritize another pull request. |
⌛ Testing commit ea14886 with merge 4344f2a... |
@bors: r- |
This commit stabilizes the `std::num` module: * The `Int` and `Float` traits are deprecated in favor of (1) the newly-added inherent methods and (2) the generic traits available in rust-lang/num. * The `Zero` and `One` traits are reintroduced in `std::num`, which together with various other traits allow you to recover the most common forms of generic programming. * The `FromStrRadix` trait, and associated free function, is deprecated in favor of inherent implementations. * A wide range of methods and constants for both integers and floating point numbers are now `#[stable]`, having been adjusted for integer guidelines. * `is_positive` and `is_negative` are renamed to `is_sign_positive` and `is_sign_negative`, in order to address rust-lang#22985 * The `Wrapping` type is moved to `std::num` and stabilized; `WrappingOps` is deprecated in favor of inherent methods on the integer types, and direct implementation of operations on `Wrapping<X>` for each concrete integer type `X`. Closes rust-lang#22985 Closes rust-lang#21069 [breaking-change]
⌛ Testing commit 232424d with merge 80bf31d... |
This commit stabilizes the `std::num` module: * The `Int` and `Float` traits are deprecated in favor of (1) the newly-added inherent methods and (2) the generic traits available in rust-lang/num. * The `Zero` and `One` traits are reintroduced in `std::num`, which together with various other traits allow you to recover the most common forms of generic programming. * The `FromStrRadix` trait, and associated free function, is deprecated in favor of inherent implementations. * A wide range of methods and constants for both integers and floating point numbers are now `#[stable]`, having been adjusted for integer guidelines. * `is_positive` and `is_negative` are renamed to `is_sign_positive` and `is_sign_negative`, in order to address #22985 * The `Wrapping` type is moved to `std::num` and stabilized; `WrappingOps` is deprecated in favor of inherent methods on the integer types, and direct implementation of operations on `Wrapping<X>` for each concrete integer type `X`. Closes #22985 Closes #21069 [breaking-change] r? @alexcrichton
This commit stabilizes the `std::num` module: * The `Int` and `Float` traits are deprecated in favor of (1) the newly-added inherent methods and (2) the generic traits available in rust-lang/num. * The `Zero` and `One` traits are reintroduced in `std::num`, which together with various other traits allow you to recover the most common forms of generic programming. * The `FromStrRadix` trait, and associated free function, is deprecated in favor of inherent implementations. * A wide range of methods and constants for both integers and floating point numbers are now `#[stable]`, having been adjusted for integer guidelines. * `is_positive` and `is_negative` are renamed to `is_sign_positive` and `is_sign_negative`, in order to address rust-lang#22985 * The `Wrapping` type is moved to `std::num` and stabilized; `WrappingOps` is deprecated in favor of inherent methods on the integer types, and direct implementation of operations on `Wrapping<X>` for each concrete integer type `X`. Closes rust-lang#22985 Closes rust-lang#21069 [breaking-change] r? @alexcrichton
Swap order of input and output parameters of ptr::copy_nonoverlapping (rust-lang/rust#23866), remove usage of std::num::Int (rust-lang/rust#23549), change (rust-lang/rust#23860).
Swap order of input and output parameters of ptr::copy_nonoverlapping (rust-lang/rust#23866), remove usage of std::num::Int (rust-lang/rust#23549), change (rust-lang/rust#23860).
This commit stabilizes the
std::num
module:Int
andFloat
traits are deprecated in favor of (1) thenewly-added inherent methods and (2) the generic traits available in
rust-lang/num.
Zero
andOne
traits are reintroduced instd::num
, whichtogether with various other traits allow you to recover the most
common forms of generic programming.
FromStrRadix
trait, and associated free function, is deprecatedin favor of inherent implementations.
point numbers are now
#[stable]
, having been adjusted for integerguidelines.
is_positive
andis_negative
are renamed tois_sign_positive
andis_sign_negative
, in order to address Maybe Float::{is_positive, is_negative} should return false for 0's? #22985Wrapping
type is moved tostd::num
and stabilized;WrappingOps
is deprecated in favor of inherent methods on theinteger types, and direct implementation of operations on
Wrapping<X>
for each concrete integer typeX
.Closes #22985
Closes #21069
[breaking-change]
r? @alexcrichton