-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Make NonZero
constructors generic.
#120521
Make NonZero
constructors generic.
#120521
Conversation
This comment has been minimized.
This comment has been minimized.
@rust-lang/project-const-traits looks like a bug. Should this be relying on const traits in stable code anyways, are they ready and bug free enough for that? |
just a diagnostics bug, the error was correct.
nope, should definitely not use them in stable code. |
cf74423
to
a5042de
Compare
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.
impl lgtm. the main PR message needs updating.
cc #120257 for backlinking
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.
Thanks!
@bors r=dtolnay,oli-obk |
🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened. |
…ructors, r=dtolnay,oli-obk Make `NonZero` constructors generic. This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax. Tracking issue: rust-lang#120257 ~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~ ```rust 101 | if n == T::ZERO { | ^^^^^^^^^^^^ expected `host`, found `true` | = note: expected constant `host` found constant `true` ``` r? `@dtolnay`
…ructors, r=dtolnay,oli-obk Make `NonZero` constructors generic. This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax. Tracking issue: rust-lang#120257 ~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~ ```rust 101 | if n == T::ZERO { | ^^^^^^^^^^^^ expected `host`, found `true` | = note: expected constant `host` found constant `true` ``` r? ``@dtolnay``
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#120507 (Account for non-overlapping unmet trait bounds in suggestion) - rust-lang#120518 (riscv only supports split_debuginfo=off for now) - rust-lang#120521 (Make `NonZero` constructors generic.) - rust-lang#120527 (Switch OwnedStore handle count to AtomicU32) - rust-lang#120550 (Continue to borrowck even if there were previous errors) - rust-lang#120587 (miri: normalize struct tail in ABI compat check) - rust-lang#120590 (Remove unused args from functions) - rust-lang#120607 (fix rust-lang#120603 by adding a check in default_read_buf) Failed merges: - rust-lang#120575 (Simplify codegen diagnostic handling) r? `@ghost` `@rustbot` modify labels: rollup
…ructors, r=dtolnay,oli-obk Make `NonZero` constructors generic. This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax. Tracking issue: rust-lang#120257 ~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~ ```rust 101 | if n == T::ZERO { | ^^^^^^^^^^^^ expected `host`, found `true` | = note: expected constant `host` found constant `true` ``` r? ```@dtolnay```
💡 This pull request was already approved, no need to approve it again.
|
☀️ Test successful - checks-actions |
Finished benchmarking commit (384b02c): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 663.461s -> 663.578s (0.02%) |
pub const fn new(n: T) -> Option<Self> { | ||
// SAFETY: Memory layout optimization guarantees that `Option<NonZero<T>>` has | ||
// the same layout and size as `T`, with `0` representing `None`. | ||
unsafe { ptr::read(ptr::addr_of!(n).cast()) } |
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 looks like a reimplementation of either transmute_copy
or intrinsics::transmute_unchecked
?
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.
it used to be. I assume this was changed to be able to remove the #[rustc_allow_const_fn_unstable(const_refs_to_cell)]
, but then that wasn't removed 😆
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.
No, it was changed to fix the failing niche-filling.rs
test, caused by the assert!
call in transmute_copy
. Maybe transmute_unchecked
is what I was looking for.
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 tried removing #[rustc_allow_const_fn_unstable(const_refs_to_cell)]
as well, but it seems to be used by addr_of!
.
…ructors, r=Nilstrieb Use `transmute_unchecked` in `NonZero::new`. Tracking issue: rust-lang#120257 See rust-lang#120521 (comment).
…ructors, r=Nilstrieb Use `transmute_unchecked` in `NonZero::new`. Tracking issue: rust-lang#120257 See rust-lang#120521 (comment).
…ructors, r=Nilstrieb Use `transmute_unchecked` in `NonZero::new`. Tracking issue: rust-lang#120257 See rust-lang#120521 (comment).
…ructors, r=Nilstrieb Use `transmute_unchecked` in `NonZero::new`. Tracking issue: rust-lang#120257 See rust-lang#120521 (comment).
Rollup merge of rust-lang#120809 - reitermarkus:generic-nonzero-constructors, r=Nilstrieb Use `transmute_unchecked` in `NonZero::new`. Tracking issue: rust-lang#120257 See rust-lang#120521 (comment).
…r=Nilstrieb Use `transmute_unchecked` in `NonZero::new`. Tracking issue: rust-lang/rust#120257 See rust-lang/rust#120521 (comment).
…r=dtolnay Make `NonZero::get` generic. Tracking issue: rust-lang#120257 Depends on rust-lang#120521. r? `@dtolnay`
…r=dtolnay Make `NonZero::get` generic. Tracking issue: rust-lang#120257 Depends on rust-lang#120521. r? ``@dtolnay``
…r=dtolnay Make `NonZero::get` generic. Tracking issue: rust-lang#120257 Depends on rust-lang#120521. r? ```@dtolnay```
…dtolnay Make `NonZero::get` generic. Tracking issue: rust-lang#120257 Depends on rust-lang#120521. r? `@dtolnay`
…dtolnay Make `NonZero::get` generic. Tracking issue: rust-lang#120257 Depends on rust-lang#120521. r? `@dtolnay`
Make `NonZero::get` generic. Tracking issue: rust-lang/rust#120257 Depends on rust-lang/rust#120521. r? `@dtolnay`
Make `NonZero::get` generic. Tracking issue: rust-lang/rust#120257 Depends on rust-lang/rust#120521. r? `@dtolnay`
Make `NonZero::get` generic. Tracking issue: rust-lang/rust#120257 Depends on rust-lang/rust#120521. r? `@dtolnay`
This makes
NonZero
constructors generic, so thatNonZero::new
can be used without turbofish syntax.Tracking issue: #120257
I cannot figure out how to make this work withconst
traits. Not sure if I'm using it wrong or whether there's a bug:r? @dtolnay