use NonZeroU32 in newtype_index!macro, change syntax#53315
use NonZeroU32 in newtype_index!macro, change syntax#53315bors merged 12 commits intorust-lang:masterfrom
NonZeroU32 in newtype_index!macro, change syntax#53315Conversation
Mark-Simulacrum
left a comment
There was a problem hiding this comment.
r=me, unless you feel comments need addressing.
src/librustc_driver/test.rs
Outdated
There was a problem hiding this comment.
Why was this change necessary?
There was a problem hiding this comment.
This is because the get method of NonZeroU32 is not a const fn, so we cannot evaluate shifted_in as a const method anymore.
|
@bors r=Mark-Simulacrum |
|
📌 Commit fa86a854e679ff9833715ae5968f200a5d3ce09f has been approved by |
|
|
|
@bors try (let's do some benchmarking here first) |
|
⌛ Trying commit fa86a854e679ff9833715ae5968f200a5d3ce09f with merge 850f704f57dbfefaf6714cedb1a0d880529b9a5a... |
|
☀️ Test successful - status-travis |
|
@rust-timer build 850f704f57dbfefaf6714cedb1a0d880529b9a5a |
|
Insufficient permissions to issue commands to rust-timer. |
|
@rust-timer build 850f704f57dbfefaf6714cedb1a0d880529b9a5a |
|
Success: Queued 850f704f57dbfefaf6714cedb1a0d880529b9a5a with parent 3e05f80, comparison URL. |
|
@eddyb profiling sounds good, yes |
|
Perf says this is pretty painful. I wonder how simply not using the 0th slot in the various indexmaps would compare, instead of adding and subtracting 1. The searchability and ergro stuff is good though. |
|
Interesting. I'm quite surprised, actually. I can certainly roll back the |
|
That said, it seems like it's mostly a wash -- except for keccak, which is considerably slower (I wonder what makes it quite so different?). I've noticed the ctfe tests fluctuating 1-2% elsewhere. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
src/librustc_mir/lib.rs
Outdated
There was a problem hiding this comment.
min_const_fn should suffice I think?
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
There was a problem hiding this comment.
This isn't possible with min const fn. needs the const fn feature gate in this crate
There was a problem hiding this comment.
Or do the struct init directly without the unsafe fn call
|
☔ The latest upstream changes (presumably #52626) made this pull request unmergeable. Please resolve the merge conflicts. |
Also, adjust the MAX to be `u32::MAX - 1`, leaving room for `u32::MAX` to become a sentinel value in the future.
This reverts (part of) commit cb9a336ae2cf6a75fdcc130853286349cb424c96.
f19a7a4 to
ab43c1e
Compare
|
@bors r=Mark-Simulacrum |
|
📌 Commit ab43c1e has been approved by |
…Simulacrum use `NonZeroU32` in `newtype_index!`macro, change syntax Various improvements to the `newtype_index!` macro: - Use `NonZeroU32` so that `Option<T>` is cheap - More ergonomic helper method, no need to import `Idx` trait all the time - Improve syntax to use `struct` keyword so that ripgrep works to find type def'n Fixes rust-lang#50337 I'm curious to see if this passes tests =)
Rollup of 10 pull requests Successful merges: - #53315 (use `NonZeroU32` in `newtype_index!`macro, change syntax) - #53932 ([NLL] Remove base_place) - #53942 (Rewrite `precompute_borrows_out_of_scope` for fewer hash table lookups.) - #53973 (Have rust-lldb look for the rust-enabled lldb) - #53981 (Implement initializer() for FileDesc) - #53987 (rustbuild: allow configuring llvm version suffix) - #53993 (rustc_resolve: don't record uniform_paths canaries as reexports.) - #54007 (crates that provide a `panic_handler` are exempt from the `unused_extern_crates` lint) - #54040 (update books for next release) - #54050 (Update `petgraph` dependency to 0.4.13 to fix build with nightly)
|
👏 👏 👏 |
Various improvements to the
newtype_index!macro:NonZeroU32so thatOption<T>is cheapIdxtrait all the timestructkeyword so that ripgrep works to find type def'nFixes #50337
I'm curious to see if this passes tests =)
r? @oli-obk (picked because of reviewer suggestions...)