Rollup of 8 pull requests
Successful merges:
- #136356 (Docs for f16 and f128: correct a typo and add details)
- #136404 (Remove a footgun-y feature / relic of the past from the compiletest DSL)
- #136432 (LTA: Actually check where-clauses for well-formedness at the def site)
- #136438 (miri: improve error when offset_from preconditions are violated)
- #136441 ([`compiletest`-related cleanups 1/7] Cleanup `is_rustdoc` logic and remove a useless path join in rustdoc-json runtest logic)
- #136455 (Remove some `Clone` bounds and derives.)
- #136464 (Remove hook calling via `TyCtxtAt`.)
- #136467 (override default config profile on tarballs)
r? `@ghost`
`@rustbot` modify labels: rollup
Add a couple of missing `ensure_sufficient_stacks`
r? `@saethlin` I hope you didn't spend time on this already.
(I couldn't sleep, opened `check_tail_calls`, there was a single call where it could happen, might as well fix it)
This PR adds a couple of missing `ensure_sufficient_stack`s:
- one in `check_tail_calls` that prevented the #135709 backport on some targets.
- after that was fixed, the test still didn't pass starting at 4MB, so I also added one in `check_unsafety` and that made it pass.
I didn't add an `rmake` test purposefully limiting the min stack size on `issue-74564-if-expr-stack-overflow.rs`, but we could if we wanted to.
On `apple-aarch64-darwin`, this is enough to make `RUST_MIN_STACK=$((1024*1024*3)) ./x test tests/ui --test-args tests/ui/issues/issue-74564-if-expr-stack-overflow.rs` pass for me locally, and it does stack overflow otherwise.
When the word "cache" appears in the context of the query system, it often
isn't obvious whether that is referring to the in-memory query cache or the
on-disk incremental cache.
For these types, we can assure the reader that they are for in-memory caching.
Remove hook calling via `TyCtxtAt`.
All hooks receive a `TyCtxtAt` argument.
Currently hooks can be called through `TyCtxtAt` or `TyCtxt`. In the latter case, a `TyCtxtAt` is constructed with a dummy span and passed to the hook.
However, in practice hooks are never called through `TyCtxtAt`, and always receive a dummy span. (I confirmed this via code inspection, and double-checked it by temporarily making the `TyCtxtAt` code path panic and running all the tests.)
This commit removes all the `TyCtxtAt` machinery for hooks. All hooks now receive `TyCtxt` instead of `TyCtxtAt`. There are two existing hooks that use `TyCtxtAt::span`: `const_caller_location_provider` and `try_destructure_mir_constant_for_user_output`. For both hooks the span is always a dummy span, probably unintentionally. This dummy span use is now explicit. If a non-dummy span is needed for these two hooks it would be easy to add it as an extra argument because hooks are less constrained than queries.
r? `@oli-obk`
LTA: Actually check where-clauses for well-formedness at the def site
All of the added tests used to wrongfully pass.
r? oli-obk or types/compiler or reassign
Target modifiers (special marked options) are recorded in metainfo
Target modifiers (special marked options) are recorded in metainfo and compared to be equal in different linked crates.
PR for this RFC: https://github.com/rust-lang/rfcs/pull/3716
Option may be marked as `TARGET_MODIFIER`, example: `regparm: Option<u32> = (None, parse_opt_number, [TRACKED TARGET_MODIFIER]`.
If an TARGET_MODIFIER-marked option has non-default value, it will be recorded in crate metainfo as a `Vec<TargetModifier>`:
```
pub struct TargetModifier {
pub opt: OptionsTargetModifiers,
pub value_name: String,
}
```
OptionsTargetModifiers is a macro-generated enum.
Option value code (for comparison) is generated using `Debug` trait.
Error example:
```
error: mixing `-Zregparm` will cause an ABI mismatch in crate `incompatible_regparm`
--> $DIR/incompatible_regparm.rs:10:1
|
LL | #![crate_type = "lib"]
| ^
|
= help: the `-Zregparm` flag modifies the ABI so Rust crates compiled with different values of this flag cannot be used together safely
= note: `-Zregparm=1` in this crate is incompatible with `-Zregparm=2` in dependency `wrong_regparm`
= help: set `-Zregparm=2` in this crate or `-Zregparm=1` in `wrong_regparm`
= help: if you are sure this will not cause problems, use `-Cunsafe-allow-abi-mismatch=regparm` to silence this error
error: aborting due to 1 previous error
```
`-Cunsafe-allow-abi-mismatch=regparm,reg-struct-return` to disable list of flags.
All hooks receive a `TyCtxtAt` argument.
Currently hooks can be called through `TyCtxtAt` or `TyCtxt`. In the
latter case, a `TyCtxtAt` is constructed with a dummy span and passed to
the hook.
However, in practice hooks are never called through `TyCtxtAt`, and
always receive a dummy span. (I confirmed this via code inspection, and
double-checked it by temporarily making the `TyCtxtAt` code path panic
and running all the tests.)
This commit removes all the `TyCtxtAt` machinery for hooks. All hooks
now receive `TyCtxt` instead of `TyCtxtAt`. There are two existing hooks
that use `TyCtxtAt::span`: `const_caller_location_provider` and
`try_destructure_mir_constant_for_user_output`. For both hooks the span
is always a dummy span, probably unintentionally. This dummy span use is
now explicit. If a non-dummy span is needed for these two hooks it would
be easy to add it as an extra argument because hooks are less
constrained than queries.
This does mean that we have to resolve the list of arm IDs twice, but it's
unclear whether that even matters, whereas the cleaner signature is a nice
improvement.
Explain why we retroactively change a static initializer to have a different type
I keep getting confused about it and in turn confused `@GuillaumeGomez` while trying to explain it badly
Shorten error message for callable with wrong return type
```
error: expected `{closure@...}` to return `Ret`, but it returns `Other`
```
instead of
```
error: expected `{closure@...}` to be a closure that returns `Ret`, but it returns `Other`
```
Test validity of pattern types
r? `@RalfJung`
pulled out of #136006 so we don't have to rely on libcore types excercising this code path
There's nothing to fix. `rustc_layout_scalar_valid_range_start` structs just failed their validation on their value instead of their fields' value, causing a diff where moving to pattern types adds an additional `.0` field access to the validation error
```
error[E0369]: cannot add `((..., ..., ..., ...), ..., ..., ...)` to `((..., ..., ..., ...), ..., ..., ...)`
--> $DIR/binop.rs:9:7
|
LL | x + x;
| - ^ - ((..., ..., ..., ...), ..., ..., ...)
| |
| ((..., ..., ..., ...), ..., ..., ...)
|
= note: the full name for the type has been written to '$TEST_BUILD_DIR/$FILE.long-type-hash.txt'
= note: consider using `--verbose` to print the full type name to the console
```
```
error[E0600]: cannot apply unary operator `!` to type `(..., ..., ..., ...)`
--> $DIR/binop.rs:14:5
|
LL | !x;
| ^^ cannot apply unary operator `!`
|
= note: the full name for the type has been written to '$TEST_BUILD_DIR/$FILE.long-type-hash.txt'
= note: consider using `--verbose` to print the full type name to the console
```
CC #135919.
rustc_allowed_through_unstable_modules: require deprecation message
This changes the `#[rustc_allowed_through_unstable_modules]` attribute so that a deprecation message (ideally directing people towards the stable path) is required.
Convert two `rustc_middle::lint` functions to `Span` methods.
`rustc_middle` is a huge crate and it's always good to move stuff out of it. There are lots of similar methods already on `Span`, so these two functions, `in_external_macro` and `is_from_async_await`, fit right in. The diff is big because `in_external_macro` is used a lot by clippy lints.
r? ``@Noratrieb``
Highlight clarifying information in "expected/found" error
When the expected and found types have the same textual representation, we add clarifying in parentheses. We now visually highlight it in the output.
Detect a corner case where the clarifying information would be the same for both types and skip it, as it doesn't add anything useful.
diagnostics: fix borrowck suggestions for if/while let conditionals
This code detects the case where one of the borrows is inside the let init expr while the other end is not. If that happens, we don't want to suggest adding a semicolon, because it won't work.
Fixes#133941
Clean up MonoItem::instantiation_mode
More progress on cleaning up and documenting instantiation mode selection.
This should have no behavior changes at all, it just rearranges the code inside `MonoItem::instantiation_mode` to a more logical flow and I've tried to explain every choice the implementation is making.
Tweak fn pointer suggestion span
Use a more targeted span when suggesting casting an `fn` item to an `fn` pointer.
```
error[E0308]: cannot coerce functions which must be inlined to function pointers
--> $DIR/cast.rs:10:33
|
LL | let _: fn(isize) -> usize = callee;
| ------------------ ^^^^^^ cannot coerce functions which must be inlined to function pointers
| |
| expected due to this
|
= note: expected fn pointer `fn(_) -> _`
found fn item `fn(_) -> _ {callee}`
= note: fn items are distinct from fn pointers
help: consider casting to a fn pointer
|
LL | let _: fn(isize) -> usize = callee as fn(isize) -> usize;
| +++++++++++++++++++++
```
```
error[E0308]: mismatched types
--> $DIR/fn-pointer-mismatch.rs:42:30
|
LL | let d: &fn(u32) -> u32 = foo;
| --------------- ^^^ expected `&fn(u32) -> u32`, found fn item
| |
| expected due to this
|
= note: expected reference `&fn(_) -> _`
found fn item `fn(_) -> _ {foo}`
help: consider using a reference
|
LL | let d: &fn(u32) -> u32 = &foo;
| +
```
Previously we'd point at the whole expression for replacement, instead of marking what was being added.
We could also modify the suggestions for `&(name as fn())`, but for that we require storing more accurate spans than we have now.
Make comma separated lists of anything easier to make for errors
Provide a new function `listify`, meant to be used in cases similar to `pluralize!`. When you have a slice of arbitrary elements that need to be presented to the user, `listify` allows you to turn that into a list of comma separated strings.
This reduces a lot of redundant logic that happens often in diagnostics.
Rework "long type names" printing logic
Make it so more type-system types can be printed in a shortened version (like `Predicate`s).
Centralize printing the information about the "full type name path".
Make the "long type path" for the file where long types are written part of `Diag`, so that it becomes easier to keep track of it, and ensure it will always will be printed out last in the diagnostic by making its addition to the output implicit.
Tweak the shortening of types in "expected/found" labels.
Remove dead file `note.rs`.
Rename `tcx.ensure()` to `tcx.ensure_ok()`, and improve the associated docs
This is all based on my archaeology for https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/.60TyCtxtEnsure.60.
The main renamings are:
- `tcx.ensure()` → `tcx.ensure_ok()`
- `tcx.ensure_with_value()` → `tcx.ensure_done()`
- Query modifier `ensure_forwards_result_if_red` → `return_result_from_ensure_ok`
Hopefully these new names are a better fit for the *actual* function and purpose of these query call modes.
From `rustc_middle::infer` to `rustc_infer::infer`. Because everything
in it is only used within `rustc_infer`, and no longer needs to be
`pub`. Plus it's always good to make the huge `rustc_middle` crate
smaller.
`rustc_middle` is a huge crate and it's always good to move stuff out of
it. There are lots of similar methods already on `Span`, so these two
functions, `in_external_macro` and `is_from_async_await`, fit right in.
The diff is big because `in_external_macro` is used a lot by clippy
lints.
When the expected and found types have the same textual representation, we add clarifying in parentheses. We now visually highlight it in the output.
Detect a corner case where the clarifying information would be the same for both types and skip it, as it doesn't add anything useful.
```
error: expected `{closure@...}` to return `Ret`, but it returns `Other`
```
instead of
```
error: expected `{closure@...}` to be a closure that returns `Ret`, but it returns `Other`
```
Use a more targeted span when suggesting casting an `fn` item to an `fn` pointer.
```
error[E0308]: cannot coerce functions which must be inlined to function pointers
--> $DIR/cast.rs:10:33
|
LL | let _: fn(isize) -> usize = callee;
| ------------------ ^^^^^^ cannot coerce functions which must be inlined to function pointers
| |
| expected due to this
|
= note: expected fn pointer `fn(_) -> _`
found fn item `fn(_) -> _ {callee}`
= note: fn items are distinct from fn pointers
help: consider casting to a fn pointer
|
LL | let _: fn(isize) -> usize = callee as fn(isize) -> usize;
| +++++++++++++++++++++
```
```
error[E0308]: mismatched types
--> $DIR/fn-pointer-mismatch.rs:42:30
|
LL | let d: &fn(u32) -> u32 = foo;
| --------------- ^^^ expected `&fn(u32) -> u32`, found fn item
| |
| expected due to this
|
= note: expected reference `&fn(_) -> _`
found fn item `fn(_) -> _ {foo}`
help: consider using a reference
|
LL | let d: &fn(u32) -> u32 = &foo;
| +
```
Previously we'd point at the whole expression for replacement, instead of marking what was being added.
We could also modify the suggestions for `&(name as fn())`, but for that we require storing more accurate spans than we have now.
This improves performance on many-seed parallel (-Zthreads=32) miri
executions from managing to use ~8 cores to using 27-28 cores. That's
pretty reasonable scaling for the simplicity of this solution.
This code detects the case where one of the borrows is inside the
let init expr while the other end is not. If that happens, we don't
want to suggest adding a semicolon, because it won't work.
Implement MIR lowering for unsafe binders
This is the final bit of the unsafe binders puzzle. It implements MIR, CTFE, and codegen for unsafe binders, and enforces that (for now) they are `Copy`. Later on, I'll introduce a new trait that relaxes this requirement to being "is `Copy` or `ManuallyDrop<T>`" which more closely models how we treat union fields.
Namely, wrapping unsafe binders is now `Rvalue::WrapUnsafeBinder`, which acts much like an `Rvalue::Aggregate`. Unwrapping unsafe binders are implemented as a MIR projection `ProjectionElem::UnwrapUnsafeBinder`, which acts much like `ProjectionElem::Field`.
Tracking:
- https://github.com/rust-lang/rust/issues/130516
Use proper type when applying deref adjustment in const
When applying a deref adjustment to some type `Wrap<T>` which derefs to `T`, we were checking that `T: ~const Deref`, not `Wrap<T>: ~const Deref` like we should have been.
r? project-const-traits
Fixes#136273Fixes#135210 -- I just deleted the test since the regression test is uninteresting
Manually walk into WF obligations in `BestObligation` proof tree visitor
When we encounter a `WellFormed` obligation in the `BestObligation` proof tree visitor, ignore the proof tree and call `wf::unnormalized_obligations` to derive well-formed obligations with the correct cause codes. This is to avoid having to replicate the somewhat delicate logic that `wf.rs` does to set up its obligation causes... Don't see a better way to do this.
vibes?? r? lcnr
Make it so more type-system types can be printed in a shortened version (like `Predicate`s).
Centralize printing the information about the "full type name path".
Make the "long type path" for the file where long types are written part of `Diag`, so that it becomes easier to keep track of it, and ensure it will always will be printed out last in the diagnostic by making its addition to the output implicit.
Tweak the shortening of types in "expected/found" labels.
Remove dead file `note.rs`.
Provide a new function `listify`, meant to be used in cases similar to `pluralize!`. When you have a slice of arbitrary elements that need to be presented to the user, `listify` allows you to turn that into a list of comma separated strings.
This reduces a lot of redundant logic that happens often in diagnostics.
Insert null checks for pointer dereferences when debug assertions are enabled
Similar to how the alignment is already checked, this adds a check
for null pointer dereferences in debug mode. It is implemented similarly
to the alignment check as a `MirPass`.
This inserts checks in the same places as the `CheckAlignment` pass and additionally
also inserts checks for `Borrows`, so code like
```rust
let ptr: *const u32 = std::ptr::null();
let val: &u32 = unsafe { &*ptr };
```
will have a check inserted on dereference. This is done because null references
are UB. The alignment check doesn't cover these places, because in `&(*ptr).field`,
the exact requirement is that the final reference must be aligned. This is something to
consider further enhancements of the alignment check.
For now this is implemented as a separate `MirPass`, to make it easy to disable
this check if necessary.
This is related to a 2025H1 project goal for better UB checks in debug
mode: https://github.com/rust-lang/rust-project-goals/pull/177.
r? `@saethlin`
Replace our `LLVMRustDIBuilderRef` with LLVM-C's `LLVMDIBuilderRef`
Inspired by trying to split #134009 into smaller steps that are easier to review individually.
This makes it possible to start incrementally replacing our debuginfo bindings with the ones in the LLVM-C API, all of which operate on `LLVMDIBuilderRef`.
There should be no change to compiler behaviour.
Compiler: Finalize dyn compatibility renaming
Update the Reference link to use the new URL fragment from https://github.com/rust-lang/reference/pull/1666 (this change has finally hit stable). Fixes a FIXME.
Follow-up to #130826.
Part of #130852.
~~Blocking it on #133372.~~ (merged)
r? ghost
Similar to how the alignment is already checked, this adds a check
for null pointer dereferences in debug mode. It is implemented similarly
to the alignment check as a MirPass.
This is related to a 2025H1 project goal for better UB checks in debug
mode: https://github.com/rust-lang/rust-project-goals/pull/177.
Rollup of 8 pull requests
Successful merges:
- #135414 (Stabilize `const_black_box`)
- #136150 (ci: use windows 2025 for i686-mingw)
- #136258 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 11))
- #136270 (Remove `NamedVarMap`.)
- #136278 (add constraint graph to polonius MIR dump)
- #136287 (LLVM changed the nocapture attribute to captures(none))
- #136291 (some test suite cleanups)
- #136296 (float::min/max: mention the non-determinism around signed 0)
r? `@ghost`
`@rustbot` modify labels: rollup
Autodiff Upstreaming - rustc_codegen_ssa, rustc_middle
This PR should not be merged until the rustc_codegen_llvm part is merged.
I will also alter it a little based on what get's shaved off from the cg_llvm PR,
and address some of the feedback I received in the other PR (including cleanups).
I am putting it already up to
1) Discuss with `@jieyouxu` if there is more work needed to add tests to this and
2) Pray that there is someone reviewing who can tell me why some of my autodiff invocations get lost.
Re 1: My test require fat-lto. I also modify the compilation pipeline. So if there are any other llvm-ir tests in the same compilation unit then I will likely break them. Luckily there are two groups who currently have the same fat-lto requirement for their GPU code which I have for my autodiff code and both groups have some plans to enable support for thin-lto. Once either that work pans out, I'll copy it over for this feature. I will also work on not changing the optimization pipeline for functions not differentiated, but that will require some thoughts and engineering, so I think it would be good to be able to run the autodiff tests isolated from the rest for now. Can you guide me here please?
For context, here are some of my tests in the samples folder: https://github.com/EnzymeAD/rustbook
Re 2: This is a pretty serious issue, since it effectively prevents publishing libraries making use of autodiff: https://github.com/EnzymeAD/rust/issues/173. For some reason my dummy code persists till the end, so the code which calls autodiff, deletes the dummy, and inserts the code to compute the derivative never gets executed. To me it looks like the rustc_autodiff attribute just get's dropped, but I don't know WHY? Any help would be super appreciated, as rustc queries look a bit voodoo to me.
Tracking:
- https://github.com/rust-lang/rust/issues/124509
r? `@jieyouxu`
When encountering unexpected closure return type, point at return type/expression
```
error[E0271]: expected `{closure@fallback-closure-wrap.rs:18:40}` to be a closure that returns `()`, but it returns `!`
--> $DIR/fallback-closure-wrap.rs:19:9
|
LL | let error = Closure::wrap(Box::new(move || {
| -------
LL | panic!("Can't connect to server.");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `!`
|
= note: expected unit type `()`
found type `!`
= note: required for the cast from `Box<{closure@$DIR/fallback-closure-wrap.rs:18:40: 18:47}>` to `Box<dyn FnMut()>`
```
```
error[E0271]: expected `{closure@dont-ice-for-type-mismatch-in-closure-in-async.rs:6:10}` to be a closure that returns `bool`, but it returns `Option<()>`
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:6:16
|
LL | call(|| -> Option<()> {
| ---- ------^^^^^^^^^^
| | |
| | expected `bool`, found `Option<()>`
| required by a bound introduced by this call
|
= note: expected type `bool`
found enum `Option<()>`
note: required by a bound in `call`
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:3:25
|
LL | fn call(_: impl Fn() -> bool) {}
| ^^^^ required by this bound in `call`
```
```
error[E0271]: expected `{closure@f670.rs:28:13}` to be a closure that returns `Result<(), _>`, but it returns `!`
--> f670.rs:28:20
|
28 | let c = |e| -> ! {
| -------^
| |
| expected `Result<(), _>`, found `!`
...
32 | f().or_else(c);
| ------- required by a bound introduced by this call
-Ztrack-diagnostics: created at compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs:1433:28
|
= note: expected enum `Result<(), _>`
found type `!`
note: required by a bound in `Result::<T, E>::or_else`
--> /home/gh-estebank/rust/library/core/src/result.rs:1406:39
|
1406 | pub fn or_else<F, O: FnOnce(E) -> Result<T, F>>(self, op: O) -> Result<T, F> {
| ^^^^^^^^^^^^ required by this bound in `Result::<T, E>::or_else`
```
CC #111539.
add constraint graph to polonius MIR dump
Another easy one while I work on diagnostics. This PR adds a mermaid visualization of the polonius constraint graph to the polonius MIR dump.
Adding kills is left to a future PR (until they're encoded in edges directly or I set up recording debugging info in and out of the analysis), because right now they're only computed during traversal.
[Here's](https://gistpreview.github.io/?096b0131e8258f9a3125c55c7ac369bc) how that looks.
r? `@matthewjasper` but as always feel free to reroll.
Remove `NamedVarMap`.
`NamedVarMap` is extremely similar to `ResolveBoundVars`. The former contains two `UnordMap<ItemLocalId, T>` fields (obscured behind `ItemLocalMap` typedefs). The latter contains two
`SortedMap<ItemLocalId, T>` fields. We construct a `NamedVarMap` and then convert it into a `ResolveBoundVars` by sorting the `UnordMap`s, which is unnecessary busywork.
This commit removes `NamedVarMap` and constructs a `ResolveBoundVars` directly. `SortedMap` and `NamedVarMap` have slightly different perf characteristics during construction (e.g. speed of insertion) but this code isn't hot enough for that to matter.
A few details to note.
- A `FIXME` comment is removed.
- The detailed comments on the fields of `NamedVarMap` are copied to `ResolveBoundVars` (which has a single, incorrect comment).
- `BoundVarContext::map` is renamed.
- `ResolveBoundVars` gets a derived `Default` impl.
r? `@jackh726`
Stabilize `const_black_box`
This has been unstably const since #92226, but a tracking issue was never created. Per [discussion on Zulip][zulip], there should not be any blockers to making this const-stable. The function does not provide any functionality at compile time but does allow code reuse between const- and non-const functions, so stabilize it here.
[zulip]: https://rust-lang.zulipchat.com/#narrow/channel/146212-t-compiler.2Fconst-eval/topic/const_black_box
It's a function that prints numbers with underscores inserted for
readability (e.g. "1_234_567"), used by `-Zmeta-stats` and
`-Zinput-stats`. It's the only thing in `rustc_middle::util::common`,
which is a bizarre location for it.
This commit:
- moves it to `rustc_data_structures`, a more logical crate for it;
- puts it in a module `thousands`, like the similar crates.io crate;
- renames it `format_with_underscores`, which is a clearer name;
- rewrites it to be more concise;
- slightly improves the testing.
It's a function that does stuff with MIR and yet it weirdly has its own
module in `rustc_middle::util`. This commit moves it into
`rustc_middle::mir`, a more sensible home.
Fix deduplication mismatches in vtables leading to upcasting unsoundness
We currently have two cases where subtleties in supertraits can trigger disagreements in the vtable layout, e.g. leading to a different vtable layout being accessed at a callsite compared to what was prepared during unsizing. Namely:
### #135315
In this example, we were not normalizing supertraits when preparing vtables. In the example,
```
trait Supertrait<T> {
fn _print_numbers(&self, mem: &[usize; 100]) {
println!("{mem:?}");
}
}
impl<T> Supertrait<T> for () {}
trait Identity {
type Selff;
}
impl<Selff> Identity for Selff {
type Selff = Selff;
}
trait Middle<T>: Supertrait<()> + Supertrait<T> {
fn say_hello(&self, _: &usize) {
println!("Hello!");
}
}
impl<T> Middle<T> for () {}
trait Trait: Middle<<() as Identity>::Selff> {}
impl Trait for () {}
fn main() {
(&() as &dyn Trait as &dyn Middle<()>).say_hello(&0);
}
```
When we prepare `dyn Trait`, we see a supertrait of `Middle<<() as Identity>::Selff>`, which itself has two supertraits `Supertrait<()>` and `Supertrait<<() as Identity>::Selff>`. These two supertraits are identical, but they are not duplicated because we were using structural equality and *not* considering normalization. This leads to a vtable layout with two trait pointers.
When we upcast to `dyn Middle<()>`, those two supertraits are now the same, leading to a vtable layout with only one trait pointer. This leads to an offset error, and we call the wrong method.
### #135316
This one is a bit more interesting, and is the bulk of the changes in this PR. It's a bit similar, except it uses binder equality instead of normalization to make the compiler get confused about two vtable layouts. In the example,
```
trait Supertrait<T> {
fn _print_numbers(&self, mem: &[usize; 100]) {
println!("{mem:?}");
}
}
impl<T> Supertrait<T> for () {}
trait Trait<T, U>: Supertrait<T> + Supertrait<U> {
fn say_hello(&self, _: &usize) {
println!("Hello!");
}
}
impl<T, U> Trait<T, U> for () {}
fn main() {
(&() as &'static dyn for<'a> Trait<&'static (), &'a ()>
as &'static dyn Trait<&'static (), &'static ()>)
.say_hello(&0);
}
```
When we prepare the vtable for `dyn for<'a> Trait<&'static (), &'a ()>`, we currently consider the PolyTraitRef of the vtable as the key for a supertrait. This leads two two supertraits -- `Supertrait<&'static ()>` and `for<'a> Supertrait<&'a ()>`.
However, we can upcast[^up] without offsetting the vtable from `dyn for<'a> Trait<&'static (), &'a ()>` to `dyn Trait<&'static (), &'static ()>`. This is just instantiating the principal trait ref for a specific `'a = 'static`. However, when considering those supertraits, we now have only one distinct supertrait -- `Supertrait<&'static ()>` (which is deduplicated since there are two supertraits with the same substitutions). This leads to similar offsetting issues, leading to the wrong method being called.
[^up]: I say upcast but this is a cast that is allowed on stable, since it's not changing the vtable at all, just instantiating the binder of the principal trait ref for some lifetime.
The solution here is to recognize that a vtable isn't really meaningfully higher ranked, and to just treat a vtable as corresponding to a `TraitRef` so we can do this deduplication more faithfully. That is to say, the vtable for `dyn for<'a> Tr<'a>` and `dyn Tr<'x>` are always identical, since they both would correspond to a set of free regions on an impl... Do note that `Tr<for<'a> fn(&'a ())>` and `Tr<fn(&'static ())>` are still distinct.
----
There's a bit more that can be cleaned up. In codegen, we can stop using `PolyExistentialTraitRef` basically everywhere. We can also fix SMIR to stop storing `PolyExistentialTraitRef` in its vtable allocations.
As for testing, it's difficult to actually turn this into something that can be tested with `rustc_dump_vtable`, since having multiple supertraits that are identical is a recipe for ambiguity errors. Maybe someone else is more creative with getting that attr to work, since the tests I added being run-pass tests is a bit unsatisfying. Miri also doesn't help here, since it doesn't really generate vtables that are offset by an index in the same way as codegen.
r? `@lcnr` for the vibe check? Or reassign, idk. Maybe let's talk about whether this makes sense.
<sup>(I guess an alternative would also be to not do any deduplication of vtable supertraits (or only a really conservative subset) rather than trying to normalize and deduplicate more faithfully here. Not sure if that works and is sufficient tho.)</sup>
cc `@steffahn` -- ty for the minimizations
cc `@WaffleLapkin` -- since you're overseeing the feature stabilization :3
Fixes#135315Fixes#135316
This makes it possible to start incrementally replacing our debuginfo bindings
with the ones in the LLVM-C API, all of which operate on `LLVMDIBuilderRef`.
Instead re-export `rustc_hir_analysis::collect::suggest_impl_trait`,
which is the only thing from the module used in another crate. This
fixes a `FIXME` comment. Also adjust some visibilities to satisfy the
`unreachable_pub` lint.
This changes requires downgrading a link in a comment on `FnCtxt`
because `collect` is no longer public and rustdoc complains otherwise.
This is annoying but I can't see how to avoid it.
`delegation.rs` has three builders: `GenericsBuilder`,
`PredicatesBuilder`, and `GenericArgsBuilder`. The first two builders
have just two optional parameters, and the third one has zero. Each
builder is used within a single function. The code is over-engineered.
This commit removes the builders, replacing each with with a single
`build_*` function. This makes the code shorter and simpler.
There is a comment `Delegation to inherent methods is not yet
supported.` that appears three times mid-pattern and somehow inhibits
rustfmt from formatting the enclosing `match` statement. This commit
moves them to the top of the pattern, which enables more formatting.
This comment made sense when this crate was called `rustc_typeck`, but
makes less sense now that it's called `rustc_hir_analysis`. Especially
given that `check_drop_impl` is only called within the crate.
