Rollup of 10 pull requests#156834
Conversation
…ulacrum [Debug Info] Gracefully handle invalid `String`/`Vec` Somewhat related to rust-lang#150392. Currently the handling can throw an exception, which we should absolutely not do. It causes issues with debugger adapters (e.g. CodeLLDB will hang forever. Trying to stop the debugger via vscode's interface causes a CodeLLDB to leak memory constantly until RAM is depleted and the OS starts killing processes). The exception has been replaced with a printed error message and a placeholder value. Additionally, if a String/Vec is in an "invalid" state due to niche optimization (`capacity >= (1 << 63)`, common with `Option<String>`/`Option<Vec<T>>`), the pointer and length values will be meaningless, but are not guaranteed to be 0'd. The debugger will happily proceed as if they are useful values, and often do things like \<try to read multiple GB of data from the debugee\>. I added simple checks to ensure that the capacity and length are within bounds, and that the pointer is non-null. If any check fails, the string/vec just acts as if it's empty. Eventually this problem will be solved on LLDB's end via llvm/llvm-project#188487 or similar, but preventing issues on our end in the short term will help a lot. --- try-job: aarch64-apple
…clubby789 Install additional LLVM DLL on Windows Fixes rust-lang#155268 The problem is that with shared linking to LLVM lib we have binaries requiring it in `$sysroot/bin` and `$sysroot/lib/rustlib/triple/bin`, but place the DLL only in the first location. On Unix systems rpath would take care of that, but on Windows the required DLL must be next to the binary or be found in PATH. This PR puts another copy of the library into that second location. The size overhead of such copy compared to static linking is much lower than I anticipated even though I couldn't manage to make bootstrap create hard links. The sizes without and with `llvm-tools` component: ``` ❯ du --summarize -h .rustup/toolchains/{additional-dll,static}* 638M .rustup/toolchains/additional-dll 800M .rustup/toolchains/additional-dll-tools 608M .rustup/toolchains/static-link 1.1G .rustup/toolchains/static-link-tools ``` Statically linked LLD sits at 139M while dynamically linked one is 7.5M + 150M for the libLLVM, so the difference is minor. Alternatively, we could require that on Windows, binaries located in `$sysroot/lib/rustlib/triple/bin` must always be called with `$sysroot/bin` in the PATH. However, that sounds like something that would require an announcement and some grace period.
…feature_hard_error_option, r=dingxiangfei2009 Allow forbidden target features to be hard errors Sometimes when adding a new target features to the list of known target features, we want to completely forbid them from being used because they are ABI affecting and should be target modifiers instead. As such, we want to issue an error rather than a warning since there are no future compatibility concerns . r? @workingjubilee since you're familiar with the motivating case in rust-lang#136597 If I rebase that PR on top of this locally, then we get the following error messages when trying to activate the SLS target feature (note that they are *errors* not *warnings* and we've omitted the usual note about future compatibility): ``` error: target feature `harden-sls-ijmp` cannot be enabled with `-Ctarget-feature`: use `harden-sls` compiler flag instead error: target feature `harden-sls-ret` cannot be enabled with `-Ctarget-feature`: use `harden-sls` compiler flag instead error: aborting due to 2 previous errors ```
…-compiler-doc-comments, r=jackh726 compiler: fix duplicated "the" in two doc-comments Two one-line typo fixes for duplicated "the" in compiler doc-comments: - `compiler/rustc_type_ir/src/solve/mod.rs` — "only if any of the the opaques in the opaque type storage has a hidden type" → "...any of the opaques in..." - `compiler/rustc_borrowck/src/diagnostics/var_name.rs` — "/// Find the the name and span of the variable corresponding to the given region." → "/// Find the name and span of the variable corresponding to the given region." No code/behavior change.
…diagnostic, r=jackh726 Remove stale RTN FIXME for assoc item constraint fallback The old FIXME suggested that this diagnostic path should emit a `return_type_notation` feature-gate diagnostic. That is no longer correct: RTN associated item constraints now use `(..)`, so a form like `Trait<method(): Bound>` is not made valid by enabling `return_type_notation`. This PR removes the stale FIXME without changing the diagnostic behavior.
…contract, r=jackh726 Fix reborrow ICE in MIR place lowering ## Summary Fixes rust-lang#156482. This fixes an internal THIR/MIR contract inconsistency for generic reborrow expressions. `ExprKind::Reborrow` is lowered into MIR as `Rvalue::Reborrow`, but MIR expression categorization treated it as place-like while `expr_as_place` treated evaluating it as a place as unreachable. That caused the rust-lang#156482 ICE, where the compiler panicked with `entered unreachable code` in `expr_as_place` while building MIR for a generic reborrow expression. The fix makes the MIR builder contract explicit: a reborrow expression is rvalue-producing. Its source may be a place, but the reborrow expression itself is not an assignable place. ## What changed - Updated MIR expression categorization so `ExprKind::Reborrow` is categorized as rvalue-producing rather than place-like. - Updated place lowering so reborrow follows the normal rvalue temporary-materialization path if a place is needed. - Added a targeted UI regression test covering the exact rust-lang#156482 repro. cc @aapoalas
…aits, r=ytmimi rustfmt: format const trait impls to `const impl` for syntax transition Because this concerns rustc development (we want this in next beta) so that we can ensure by the next beta rustc can remove the parser support for `impl const Trait`, this PR is made against the r-l/r repo :)
…enkov Privacy: enqueue type alias fixes rust-lang#156778 r? @petrochenkov
…ided-infer-lt-rib, r=petrochenkov delegation: visit body under elided-infer lifetime rib Fixes rust-lang#156806. Part of rust-lang#118212. r? @petrochenkov
…nnethercote Turn `lint_index` from `Option<u16>` to `u16` for LintExpectationId While doing lint attributes PR, I realised that the field `lint_index` didn't need to be Optional (this PR is part of a series of PRs that will make the lint attrs PR more easily reviewable) r? @nnethercote because you did rust-lang#156596 recently
|
@bors r+ rollup=never p=5 |
|
@bors p=10 |
This comment has been minimized.
This comment has been minimized.
…uwer Rollup of 10 pull requests Successful merges: - #155509 ([Debug Info] Gracefully handle invalid `String`/`Vec`) - #156229 (Install additional LLVM DLL on Windows) - #152821 (Allow forbidden target features to be hard errors) - #156560 (compiler: fix duplicated "the" in two doc-comments) - #156725 (Remove stale RTN FIXME for assoc item constraint fallback) - #156803 (Fix reborrow ICE in MIR place lowering) - #156815 (rustfmt: format const trait impls to `const impl` for syntax transition) - #156818 (Privacy: enqueue type alias) - #156820 (delegation: visit body under elided-infer lifetime rib) - #156829 (Turn `lint_index` from `Option<u16>` to `u16` for LintExpectationId)
|
💔 Test for 1a251dc failed: CI. Failed job:
|
|
Hm.. |
This comment has been minimized.
This comment has been minimized.
|
@bors treeopen |
|
Tree is now open for merging. |
|
📌 Perf builds for each rolled up PR:
previous master: 62f36da19c 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 62f36da (parent) -> b52edc2 (this PR) Test differencesShow 17 test diffsStage 1
Stage 2
Additionally, 12 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 b52edc25bfbaa955b4b83c10f998e5224c3478b2 --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 (b52edc2): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (primary 1.6%, secondary 0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.1%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 510.755s -> 513.181s (0.47%) |
Successful merges:
String/Vec#155509 ([Debug Info] Gracefully handle invalidString/Vec)const implfor syntax transition #156815 (rustfmt: format const trait impls toconst implfor syntax transition)lint_indexfromOption<u16>tou16for LintExpectationId #156829 (Turnlint_indexfromOption<u16>tou16for LintExpectationId)r? @ghost
Create a similar rollup