Skip to content

Rollup of 6 pull requests #137446

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

Merged
merged 23 commits into from
Feb 23, 2025
Merged

Rollup of 6 pull requests #137446

merged 23 commits into from
Feb 23, 2025

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

bend-n and others added 23 commits February 17, 2025 10:07
Introduce an enum that represents the different possible sources for
dependencies, and use them where possible. This will enable more fine
grained control and provides better context than passing the `dep_root`
tuple.

Use this to ensure that injected crates always show up as private by
default.
I had to do a lot of debug by printing; having these `Debug` traits in
place made it easier. Additionally, add some more information to
existing `info!` statements.
This pair of fn was introduced to perform invariant checks for scalars.
Their current behavior doesn't mesh as well with checking SIMD types,
so change the name of the fn to reflect their actual use-case and
refactor the corresponding checks.

Also simplify the returns from Option<AbiAndPrefAlign> to Option<Align>,
because every site was mapping away the "preferred" alignment anyways.
Introduce a pair of functions that actually describe what they do,
because it wasn't clear the alignment is sometimes a forgery.
`compiler_builtins` is currently injected as `extern crate
compiler_builtins as _`. This has made gating via diagnostics difficult
because it appears in the crate graph as a non-private dependency, and
there isn't an easy way to differentiate between the injected AST and
user-specified `extern crate compiler_builtins`.

Resolve this by injecting `compiler_builtins` during postprocessing
rather than early in the AST. Most of the time this isn't even needed
because it shows up in `std` or `core`'s crate graph, but injection is
still needed to ensure `#![no_core]` works correctly.

A similar change was attempted at [1] but this encountered errors
building `proc_macro` and `rustc-std-workspace-std`. Similar failures
showed up while working on this patch, which were traced back to
`compiler_builtins` showing up in the graph twice (once via dependency
and once via injection). This is resolved by not injecting if a
`#![compiler_builtins]` crate already exists.

[1]: rust-lang#113634
Remove the portion of ed63539 that automatically sets crates private
based on whether they are dependencies of `std`. Instead, this is
controlled by dependency configuration in `Cargo.toml`.
In [1], most dependencies of `std` and other sysroot crates were marked
private, but this did not happen for `alloc` and `test`. Update these
here, marking public standard library crates as the only non-private
dependencies.

[1]: rust-lang#111076
Currently, marking a dependency private does not automatically make all
its child dependencies private. Resolve this by making its children
private by default as well.

This also resolves some FIXMEs for tests that are intended to fail but
previously passed.

[1]: rust-lang#135501 (comment)
The recent fixes to private dependencies exposed some cases in the UEFI
module where private dependencies are exposed in a public interface.
These do not need to be crate-public, so change them to `pub(crate)`.
…te, r=bjorn3

Inject `compiler_builtins` during postprocessing and ensure it is made private

Follow up of rust-lang#135278

Do the following:

* Inject `compiler_builtins` during postprocessing, rather than injecting `extern crate compiler_builtins as _` into the AST
* Do not make dependencies of `std` private by default (this was added in rust-lang#135278)
* Make sure sysroot crates correctly mark their dependencies private/public
* Ensure that marking a dependency private makes its dependents private by default as well, unless otherwise specified
* Do the `compiler_builtins` update that has been blocked on this

There is more detail in the commit messages. This includes the changes I was working on in rust-lang#136226.

try-job: test-various
try-job: x86_64-msvc-1
try-job: x86_64-msvc-2
try-job: i686-mingw-1
try-job: i686-mingw-2
stabilize `(const_)ptr_sub_ptr`

Tracking issue: rust-lang#95892
Closes rust-lang#95892
FCP Completed: rust-lang#95892 (comment)

r? ````@Noratrieb````
…-obk

Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes

There are a few intertwined problems with `sym fn` operands in both inline and global asm macros.

Specifically, unlike other anon consts, they may evaluate to a type with free regions in them without actually having an item-level type annotation to give them a "proper" type. This is in contrast to named constants, which always have an item-level type annotation, or unnamed constants which are constrained by their position (e.g. a const arg in a turbofish, or a const array length).

Today, we infer the type of the operand by looking at the HIR typeck results; however, those results are region-erased, so during borrowck we ICE since we don't expect to encounter erased regions. We can't just fill this type with something like `'static`, since we may want to use real (free) regions:

```rust
fn foo<'a>() {
  asm!("/* ... */", sym bar::<&'a ()>);
}
```

The first idea may be to represent `sym fn` operands using *inline* consts instead of anon consts. This makes sense, since inline consts can reference regions from the parent body (like the `'a` in the example above). However, this introduces a problem with `global_asm!`, which doesn't *have* a parent body; inline consts *must* be associated with a parent body since they are not a body owner of their own. In rust-lang#116087, I attempted to fix this by using two separate `sym` operands for global and inline asm. However, this led to a lot of confusion and also some unattractive code duplication.

In this PR, I adjust the lowering of `global_asm!` so that it's lowered in a "fake" HIR body. This body contains a single expression which is `ExprKind::InlineAsm`; we don't *use* this HIR body, but it's used in typeck and borrowck so that we can properly infer and validate the the lifetimes of `sym fn` operands.

I then adjust the lowering of `sym fn` to instead be represented with a HIR expression. This is both because it's no longer necessary to represent this operand as an anon const, since it's *just* a path expression, and also more importantly to sidestep yet another ICE (rust-lang#137179), which has to do with the existing code breaking an invariant of def-id creation and anon consts. Specifically, we are not allowed to synthesize a def-id for an anon const when that anon const contains expressions with def-ids whose parent is *not* that anon const. This is somewhat related to rust-lang#130443 (comment), which is also a place in the compiler where synthesizing anon consts leads to def-id parenting issue.

As a side-effect, this consolidates the type checking for inline and global asm, so it allows us to simplify `InlineAsmCtxt` a bit. It also allows us to delete a bit of hacky code from anon const `type_of` which was there to detect `sym fn` operands specifically. This also could be generalized to support `const` asm operands with types with lifetimes in them. Since we specifically reject these consts today, I'm not going to change the representation of those consts (but they'd just be turned into inline consts).

r? oli-obk -- mostly b/c you're patient and also understand the breadth of the code that this touches, please reassign if you don't want to review this.

Fixes rust-lang#111709
Fixes rust-lang#96304
Fixes rust-lang#137179
…-assumptions, r=bjorn3,RalfJung

compiler: untangle SIMD alignment assumptions

There were a number of puzzling assumptions being made about SIMD types and their layout that I have corrected in this diff. These are mostly no-op edits in actual fact, but they do subtly alter a pair of checks in our invariant-checking and union layout computation that rested on those peculiar assumptions. Those unfortunately stand in the way of any further actual fixes. I submit this for review, even though it's not clearly motivated without its followups, because it should still be possible to independently conclude whether this is correct.
…ultiple-of, r=Noratrieb

stabilize `unsigned_is_multiple_of`

tracking issue: rust-lang#128101
fcp completed in: rust-lang#128101 (comment)

### Public API

A version of this for all the unsigned types

```rust
fn is_multiple_of(lhs: u64, rhs: u64) -> bool {
    match rhs {
        // prevent division by zero
        0 => lhs == 0,
        _ => lhs % rhs == 0,
    }
}
```
…id-sugg, r=estebank

Remove invalid suggestion of into_iter for extern macro

Fixes rust-lang#137345

rust-lang#109082 is closed due to performance issue, do we have any other solution for this kind of issue?
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. rollup A PR which is a rollup labels Feb 22, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Collaborator

bors commented Feb 22, 2025

📌 Commit e780b89 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2025
@bors
Copy link
Collaborator

bors commented Feb 23, 2025

⌛ Testing commit e780b89 with merge bca5f37...

@bors
Copy link
Collaborator

bors commented Feb 23, 2025

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing bca5f37 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 23, 2025
@bors bors merged commit bca5f37 into rust-lang:master Feb 23, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 23, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#135501 Inject compiler_builtins during postprocessing and ensure… 394070965eb356dd500f957650ce9bae6de54f6b (link)
#137121 stabilize (const_)ptr_sub_ptr 31c561eb9973e9f59d921970287c3bd48367353a (link)
#137180 Give global_asm a fake body to store typeck results, repr… 32bafaae310522ec93773385b46e6b991b6531b6 (link)
#137256 compiler: untangle SIMD alignment assumptions 3ab1280b7f10c5768a50d6345b356221dc8c9c7b (link)
#137383 stabilize unsigned_is_multiple_of 2ace3af97c8fe542e6bd1d77cf0b21f29dfc5780 (link)
#137415 Remove invalid suggestion of into_iter for extern macro 1e94d55d19595f11e936d25f273c50e9c9a6fde1 (link)

previous master: 07697360ae

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (bca5f37): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This 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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 2
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -0.2%)

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.

mean range count
Regressions ❌
(primary)
0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.1% [-1.1%, -1.1%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.2% [-1.1%, 0.6%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 770.476s -> 770.304s (-0.02%)
Artifact size: 359.65 MiB -> 359.67 MiB (0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants