Rollup of 3 pull requests #149462
Rollup of 3 pull requests #149462
Conversation
…o a separate function
…o a separate method
Existing but private bindings - break, everything else - continue.
…for non-glob and glob bindings. In preparation for introducing `Scope::Module(Non)Globs` and `ScopeSet::Module`.
…avidtwco const validation: remove check for mutable refs in final value of const This check rejects code that is not necessarily UB, e.g. a mutable ref to a `static mut` that is very carefully used correctly. That led to us having to describe it in the Reference, which uncovered just how ad-hoc this check is (rust-lang/reference#2074). Even without this check, we still reject things like ```rust const C: &mut i32 = &mut 0; ``` This is rejected by const checking -- the part of the frontend that looks at the source code and says whether it is allowed in const context. In the Reference, this restriction is explained [here](https://doc.rust-lang.org/nightly/reference/const_eval.html#r-const-eval.const-expr.borrows). So, the check during validation is just a safety net. And it is already a safety net with gaping holes since we only check `&mut T`, not `&UnsafeCell<T>`, due to the fact that we promote some immutable values that have `!Freeze` type so `&!Freeze` actually can occur in the final value of a const. So... it may be time for me to acknowledge that the "mutable ref in final value of const" check is a cure that's worth than the disease. Nobody asked for that check, I just added it because I was worried about soundness issues when we allow mutable references in constants. Originally it was much stricter, but I had to slowly relax it to its current form to prevent t from firing on code we intend to allow. In the end there are only 3 tests left that trigger this error, and they are all just constants containing references to mutable statics -- not the safest code in the world, but also not so bad that we have to spend a lot of time devising a core language limitation and associated Reference wording to prevent it from ever happening. So... `@rust-lang/wg-const-eval` `@rust-lang/lang` I propose that we allow code like this ```rust static mut S: i32 = 3; const C2: &'static mut i32 = unsafe { &mut * &raw mut S }; ``` `@theemathas` would be great if you could try to poke a hole into this. ;)
std: split up the `thread` module Almost all functionality in `std::thread` is currently implemented in `thread/mod.rs`, resulting in a *huge* file with more than 2000 lines and multiple, interoperating `unsafe` sections. This PR splits the file up into multiple different private modules, each implementing mostly independent parts of the functionality. The only remaining `unsafe` interplay is that of the `lifecycle` and `scope` modules, the `spawn_scoped` implementation relies on the live thread count being updated correctly by the `lifecycle` module. This PR contains no functional changes and only moves code around for the most part, with a few notable exceptions: * `with_current_name` is moved to the already existing `current` module and now uses the `name` method instead of calculating the name from private fields. The old code was just a reimplementation of that method anyway. * The private `JoinInner` type used to implement both join handles now has some more methods (`is_finished`, `thread` and the `AsInner`/`IntoInner` implementations) to avoid having to expose private fields and their invariants. * The private `spawn_unchecked_` (note the underscore) method of `Builder` is now a freestanding function in `lifecycle`. The rest of the changes are just visibility annotations. I realise this PR ended up quite large – let me know if there is anyway I can aid the review process. Edit: I've simplified the diff by adding an intermediate commit that creates all the new files by duplicating `mod.rs`. The actual changes in the second commit thus appear to delete the non-relevant parts from the respective file.
resolve: Identifier resolution refactorings Mostly splitting large functions into smaller functions, including some parts from rust-lang#144131, there should be no functional changes. See individual commits.
rustbot
added
S-waiting-on-review
|
@bors r+ rollup=never p=5 |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR:
previous master: 8a3a6bdb68 In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 8a3a6bd (parent) -> 3ff30e7 (this PR) Test differencesShow 191 test diffsStage 1
Stage 2
Additionally, 189 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 3ff30e7eafc1da7104c3960187d17939172428ed --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (3ff30e7): comparison URL. Overall result: ❌ regressions - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -3.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResults (primary 0.2%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 471.041s -> 470.301s (-0.16%) |
|
@rust-timer build fad09d2 |
This comment has been minimized.
This comment has been minimized.
|
@rust-timer build fa3c1be |
|
(You can't schedule more than a single parallel build per PR. It mught be a bit more practical to do the runs on the rolled up PRs directly). |
|
Finished benchmarking commit (fad09d2): comparison URL. Overall result: ❌ regressions - please read the text belowInstruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 471.041s -> 468.72s (-0.49%) |
|
@rust-timer build fa3c1be |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (fa3c1be): comparison URL. Overall result: no relevant changes - no action neededInstruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 471.041s -> 471.472s (0.09%) |
|
cc @petrochenkov looks like there are a bunch of tiny perf degradations in #149454 🤔 |
|
@matthiaskrgr This is from the last commit - c91b6ca (can revert and benchmark it to be sure), because we now look up a name in a module ~twice. It can be potentially optimized, and would be good to optimize it, but that code is a bit of a minefield, with any change, including bugfixes, being able to cause breakage in user code. |
8 participants
Successful merges:
threadmodule #148765 (std: split up thethreadmodule)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup