Rollup of 7 pull requests #138611
Merged
Rollup of 7 pull requests #138611
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This changes the path from "licences" to "licenses" for consistency across the repo, including the usage directly around this line. This is a US/UK spelling difference, but I believe the US spelling is also more common in open source in general.
This avoids the need to unwrap an option after ensuring that it is some.
The existing method does some non-obvious extra work to collect user types and build user-type projections, which is specifically needed by `declare_bindings` and not by the other two callers.
This makes it clearer what it's actually used for, and makes it easier to think about modifying `Item::attributes`.
It has a single call site.
`LoweringContext::new_named_lifetime` lowers the `ident` passed in. Both of its call sites *also* lower `ident` *before* passing it in. I.e. both call sites cause the ident to be lowered twice. This commit removes the lowering at the two call sites, so the ident is only lowered once.
`lower_generic_bound_predicate` calls `lower_ident`, and then passes the lowered ident into `new_named_lifetime`, which lowers it again. This commit avoids the first lowering. This requires adding a `lower_ident` call on a path that doesn't involve `new_named_lifetime`.
Currently, when declaring an extern weak function in Rust, we use the
following syntax:
```rust
unsafe extern "C" {
#[linkage = "extern_weak"]
static FOO: Option<unsafe extern "C" fn() -> ()>;
}
```
This allows runtime-checking the extern weak symbol through the Option.
When emitting LLVM-IR, the Rust compiler currently emits this static
as an i8, and a pointer that is initialized with the value of the global
i8 and represents the nullabilty e.g.
```
@foo = extern_weak global i8
@_rust_extern_with_linkage_FOO = internal global ptr @foo
```
This approach does not work well with CFI, where we need to attach CFI
metadata to a concrete function declaration, which was pointed out in
rust-lang#115199.
This change switches to emitting a proper function declaration instead
of a global i8. This allows CFI to work for extern_weak functions.
We keep initializing the Rust internal symbol with the function
declaration, which preserves the correct behavior for runtime checking
the Option.
Co-authored-by: Jakob Koschel <jakobkoschel@google.com>
…rcote Stabilize `asm_goto` feature gate Stabilize `asm_goto` feature (tracked by rust-lang#119364). The issue will remain open and be updated to track `asm_goto_with_outputs`. Reference PR: rust-lang/reference#1693 # Stabilization Report This feature adds a `label <block>` operand type to `asm!`. `<block>` must be a block expression with type unit or never. The address of the block is substituted and the assembly may jump to the block. When block completes the `asm!` block returns and continues execution. The block starts a new safety context and unsafe operations within must have additional `unsafe`s; the effect of `unsafe` that surrounds `asm!` block is cancelled. See rust-lang#119364 (comment) and rust-lang#131544. It's currently forbidden to use `asm_goto` with output operands; that is still unstable under `asm_goto_with_outputs`. Example: ```rust unsafe { asm!( "jmp {}", label { println!("Jumped from asm!"); } ); } ``` Tests: - tests/ui/asm/x86_64/goto.rs - tests/ui/asm/x86_64/goto-block-safe.stderr - tests/ui/asm/x86_64/bad-options.rs - tests/codegen/asm/goto.rs
…anieu,lnicola Denote `ControlFlow` as `#[must_use]` I've repeatedly hit bugs in the compiler due to `ControlFlow` not being marked `#[must_use]`. There seems to be an accepted ACP to make the type `#[must_use]` (rust-lang/libs-team#444), so this PR implements that part of it. Most of the usages in the compiler that trigger this new warning are "root" usages (calling into an API that uses control-flow internally, but for which the callee doesn't really care) and have been suppressed by `let _ = ...`, but I did legitimately find one instance of a missing `?` and one for a never-used `ControlFlow` value in rust-lang#137448. Presumably this needs an FCP too, so I'm opening this and nominating it for T-libs-api. This PR also touches the tools (incl. rust-analyzer), but if this went into FCP, I'd split those out into separate PRs which can land before this one does. r? libs-api `@rustbot` label: T-libs-api I-libs-api-nominated
mir_build: Avoid some useless work when visiting "primary" bindings While looking over `visit_primary_bindings`, I noticed that it does a bunch of extra work to build up a collection of “user-type projections”, even though 2/3 of its call sites don't even use them. Those callers can get the same result via `thir::Pat::walk_always`. (And it turns out that doing so also avoids creating some redundant user-type entries in MIR for some binding constructs.) I also noticed that even when the user-type projections *are* used, the process of building them ends up eagerly cloning some nested vectors at every recursion step, even in cases where they won't be used because the current subpattern has no bindings. To avoid this, the visit method now assembles a linked list on the stack containing the information that *would* be needed to create projections, and only creates the concrete projections as needed when a primary binding is encountered. Some relevant prior PRs: - rust-lang#55274 - rust-lang@0bfe184 in rust-lang#55937 --- There should be no user-visible change in compiler output.
Emit function declarations for functions with `#[linkage="extern_weak"]`
Currently, when declaring an extern weak function in Rust, we use the following syntax:
```rust
unsafe extern "C" {
#[linkage = "extern_weak"]
static FOO: Option<unsafe extern "C" fn() -> ()>;
}
```
This allows runtime-checking the extern weak symbol through the Option.
When emitting LLVM-IR, the Rust compiler currently emits this static as an i8, and a pointer that is initialized with the value of the global i8 and represents the nullabilty e.g.
```
`@FOO` = extern_weak global i8
`@_rust_extern_with_linkage_FOO` = internal global ptr `@FOO`
```
This approach does not work well with CFI, where we need to attach CFI metadata to a concrete function declaration, which was pointed out in rust-lang#115199.
This change switches to emitting a proper function declaration instead of a global i8. This allows CFI to work for extern_weak functions. Example:
```
`@_rust_extern_with_linkage_FOO` = internal global ptr `@FOO`
...
declare !type !61 !type !62 !type !63 !type !64 extern_weak void `@FOO(double)` unnamed_addr rust-lang#6
```
We keep initializing the Rust internal symbol with the function declaration, which preserves the correct behavior for runtime checking the Option.
r? `@rcvalle`
cc `@jakos-sec`
try-job: test-various
Install licenses into `share/doc/rust/licenses` This changes the path from "licences" to "licenses" for consistency across the repo, including the usage directly around this line. This is a US/UK spelling difference, but I believe the US spelling is also more common in open source in general.
…ns, r=GuillaumeGomez rustdoc-json: Don't also include `#[deprecated]` in `Item::attrs` Closes rust-lang#138378 Not sure if this should bump `FORMAT_VERSION` or not. CC `@Enselic` `@LukeMathWalker` `@obi1kenobi` r? `@GuillaumeGomez,` best reviewed commit-by-commit
…nt, r=compiler-errors Avoid double lowering of idents It's easy to double lower idents and spans because they don't change type when lowered. r? `@cjgillot`
rustbot
added
A-rustdoc-json
rustbot
added
T-rustdoc
|
@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: 8279176ccd In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
|
This is an experimental post-merge analysis report. You can ignore it. Post-merge reportComparing 8279176 (base) -> 43a2e9d (this PR) Test differences
Additionally, 42 doctest diffs were found. These are ignored, as they are noisy. Job group index
|
|
Finished benchmarking commit (43a2e9d): 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 countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -3.3%)This 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResults (secondary -0.0%)This 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: 775.979s -> 774.655s (-0.17%) |
|
@rust-timer build a4e2389 |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (a4e2389): 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)Results (primary -3.1%)This 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.
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: 775.979s -> 774.566s (-0.18%) |
|
The regression is small enough that I don't think this is really worth looking into. @rustbot label: +perf-regression-triaged |
rustbot
added
the
perf-regression-triaged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
asm_gotofeature gate #133870 (Stabilizeasm_gotofeature gate)ControlFlowas#[must_use]#137449 (DenoteControlFlowas#[must_use])#[linkage="extern_weak"]#138349 (Emit function declarations for functions with#[linkage="extern_weak"])share/doc/rust/licenses#138412 (Install licenses intoshare/doc/rust/licenses)#[deprecated]inItem::attrs#138577 (rustdoc-json: Don't also include#[deprecated]inItem::attrs)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup