Skip to content
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

Rustup #14300

Merged
merged 20 commits into from
Feb 27, 2025
Merged

Rustup #14300

merged 20 commits into from
Feb 27, 2025

Conversation

flip1995
Copy link
Member

Out of cycle sync, as it is complicated and confusing to resolve merge conflicts on the Rust side. This needs review for eda3e64 and 4d8766c as well as the comment I'll leave below.

changelog: none

samueltardieu and others added 16 commits February 12, 2025 15:57
The `#[must_use]` attribute has no effect when applied to methods in
trait implementations.
Lint `#[must_use]` attributes applied to methods in trait impls

The `#[must_use]` attribute has no effect when applied to methods in trait implementations. This PR adds it to the unused `#[must_use]` lint, and cleans the extra attributes in portable-simd and Clippy.
`Postorder` has a `C: Customization<'tcx>` parameter, that gives it
flexibility about how it computes successors. But in practice, there are
only two `impls` of `Customization`, and one is for the unit type.

This commit simplifies things by removing the generic parameter and
replacing it with an `Option`.
As `unwrap_crate_local`, because it follows exactly the standard form of
an `unwrap` function.
Continuing the work from #137162.

Every method gains a `hir_` prefix.
…n, r=compiler-errors

Simplify `Postorder` customization.

`Postorder` has a `C: Customization<'tcx>` parameter, that gives it flexibility about how it computes successors. But in practice, there are only two `impls` of `Customization`, and one is for the unit type.

This commit simplifies things by removing the generic parameter and replacing it with an `Option`.

r? ````@saethlin````
Tweaks in and around `rustc_middle`

A bunch of tiny improvements I found while working on bigger things.

r? ```@lcnr```
Rollup of 10 pull requests

Successful merges:

 - #132876 (rustdoc book: acknowledge --document-hidden-items)
 - #136148 (Optionally add type names to `TypeId`s.)
 - #136609 (libcore/net: `IpAddr::as_octets()`)
 - #137336 (Stabilise `os_str_display`)
 - #137350 (Move methods from Map to TyCtxt, part 3.)
 - #137353 (Implement `read_buf` for WASI stdin)
 - #137361 (Refactor `OperandRef::extract_field` to prep for MCP838)
 - #137367 (Do not exempt nonexistent platforms from platform policy)
 - #137374 (Stacker now handles miri using a noop impl itself)
 - #137392 (remove few unused fields)

r? `@ghost`
`@rustbot` modify labels: rollup
More sophisticated span trimming for suggestions

Previously #136958 only cared about prefixes or suffixes. Now it detects more cases where a suggestion is "sandwiched" by unchanged code on the left or the right. Would be cool if we could detect several insertions, like `ACE` going to `ABCDE`, extracting `B` and `D`, but that seems unwieldy.

r? `@estebank`
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 #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/rust#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/rust#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 #111709
Fixes #96304
Fixes #137179
@rustbot
Copy link
Collaborator

rustbot commented Feb 26, 2025

r? @y21

rustbot has assigned @y21.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 26, 2025
@flip1995
Copy link
Member Author

@y21 See the PR description on why I requested a review on a Rustup PR. Only the linked commits and the file with a comment need review. I reviewed all other 98 files.

Copy link
Member

@y21 y21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@flip1995
Copy link
Member Author

Thanks for the review! Now this is blocked on rust-lang/rust#137640

This feature was stabilized, so the FormatArgs lints should check if the MSRV of
the stabilization is met, rather than checking if the feature is enabled.
With the attribute refactor in rustc, making this case machine applicable is not
easily possible anymore. This splits up the tests properly.
@flip1995 flip1995 added this pull request to the merge queue Feb 27, 2025
Merged via the queue into rust-lang:master with commit 715d3f9 Feb 27, 2025
11 checks passed
@flip1995 flip1995 deleted the rustup branch February 27, 2025 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants