Commit graph

279922 commits

Author SHA1 Message Date
Matthias Krüger
1935bbfd18
Rollup merge of #136348 - RalfJung:miri-float-min-max, r=oli-obk
miri: make float min/max non-deterministic

This makes Miri match the documentation that landed in https://github.com/rust-lang/rust/pull/136296.

r? ``@oli-obk``
2025-02-01 01:19:22 +01:00
Matthias Krüger
47e6684e53
Rollup merge of #136314 - compiler-errors:const-deref-adj, r=fee1-dead
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 #136273
Fixes #135210 -- I just deleted the test since the regression test is uninteresting
2025-02-01 01:19:21 +01:00
Matthias Krüger
4b310be0d1
Rollup merge of #136266 - cyrgani:patch-1, r=Mark-Simulacrum
fix broken release notes id
2025-02-01 01:19:20 +01:00
Matthias Krüger
f90c321eb2
Rollup merge of #136163 - uellenberg:driftsort-off-by-one, r=Mark-Simulacrum
Fix off-by-one error causing slice::sort to abort the program

Fixes #136103.
Based on the analysis by ``@jonathan-gruber-jg`` and ``@orlp.``
2025-02-01 01:19:20 +01:00
Matthias Krüger
3c4b9122ec
Rollup merge of #135900 - compiler-errors:derive-wf, r=lcnr
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
2025-02-01 01:19:19 +01:00
Matthias Krüger
2460b280db
Rollup merge of #135840 - vayunbiyani:omit_intrinsic_unused_param_warning, r=oli-obk
omit unused args warnings for intrinsics without body

potential fix for https://github.com/rust-lang/rust/issues/135598
2025-02-01 01:19:19 +01:00
bors
854f22563c Auto merge of #136350 - matthiaskrgr:rollup-6eqfyvh, r=matthiaskrgr
Rollup of 9 pull requests

Successful merges:

 - #134531 ([rustdoc] Add `--extract-doctests` command-line flag)
 - #135860 (Compiler: Finalize dyn compatibility renaming)
 - #135992 (Improve documentation when adding a new target)
 - #136194 (Support clobber_abi in BPF inline assembly)
 - #136325 (Delay a bug when indexing unsized slices)
 - #136326 (Replace our `LLVMRustDIBuilderRef` with LLVM-C's `LLVMDIBuilderRef`)
 - #136330 (Remove unnecessary hooks)
 - #136336 (Overhaul `rustc_middle::util`)
 - #136341 (Remove myself from vacation)

r? `@ghost`
`@rustbot` modify labels: rollup
2025-01-31 20:16:46 +00:00
Michael Goulet
304b3cfcb2 Manually walk into WF obligations in BestObligation proof tree visitor 2025-01-31 18:21:58 +00:00
Michael Goulet
d8b176f683 Move fulfillment error derivation into new module 2025-01-31 18:16:02 +00:00
bors
aa4cfd0809 Auto merge of #134424 - 1c3t3a:null-checks, r=saethlin
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`
2025-01-31 15:56:53 +00:00
Matthias Krüger
fb67f3a2bf
Rollup merge of #136341 - jieyouxu:unvac, r=jieyouxu
Remove myself from vacation

r? `@ghost`
2025-01-31 12:28:20 +01:00
Matthias Krüger
95f746d2ef
Rollup merge of #136336 - nnethercote:overhaul-rustc_middle-util, r=jieyouxu
Overhaul `rustc_middle::util`

It's an odd module with some odd stuff in it.

r? `@Noratrieb`
2025-01-31 12:28:20 +01:00
Matthias Krüger
417aa6c1f6
Rollup merge of #136330 - nnethercote:rm-unnecessary-hooks, r=oli-obk
Remove unnecessary hooks

Some hooks can be downgraded to vanilla functions.

r? `@oli-obk`
2025-01-31 12:28:19 +01:00
Matthias Krüger
a5a005febe
Rollup merge of #136326 - Zalathar:llvm-di-builder-ref, r=nikic
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.
2025-01-31 12:28:18 +01:00
Matthias Krüger
f1daf9e2c4
Rollup merge of #136325 - compiler-errors:indirectly, r=RalfJung
Delay a bug when indexing unsized slices

Fixes #136298

r? RalfJung or reassign
2025-01-31 12:28:18 +01:00
Matthias Krüger
12a7f06e3c
Rollup merge of #136194 - taiki-e:bpf-clobber-abi, r=amanieu
Support clobber_abi in BPF inline assembly

This supports [`clobber_abi`](https://doc.rust-lang.org/nightly/reference/inline-assembly.html#abi-clobbers) which is one of the requirements of stabilization mentioned in the tracking Issue for `asm_experimental_arch` (#93335).

Refs: [Section 1.1 "Registers and calling convention" in BPF ABI Recommended Conventions and Guidelines v1.0](https://github.com/torvalds/linux/blob/v6.13/Documentation/bpf/standardization/abi.rst#11registers-and-calling-convention)
> R0 - R5 are scratch registers and BPF programs needs to spill/fill them if necessary across calls.

cc `@alessandrod` `@dave-tucker` `@tamird` `@vadorovsky` (target maintainers mentioned in platform support document which will be added by https://github.com/rust-lang/rust/pull/135107)

r? `@Amanieu`

`@rustbot` label +O-eBPF +A-inline-assembly
2025-01-31 12:28:17 +01:00
Matthias Krüger
f818842ce2
Rollup merge of #135992 - madsmtm:new-target-docs, r=jieyouxu
Improve documentation when adding a new target

https://github.com/rust-lang/rust/pull/133631#issuecomment-2607877936 shows that it can be a bit difficult process-wise to add a new target.

I've added a bit of text to the docs, suggesting that users add the target defintion/spec first, and later work on `std` support.

I also found that we have two places where we document how to add a new target. I've linked these for now, but they should probably be merged somehow in the future.

`@rustbot` label A-docs
r? compiler
CC `@workingjubilee` who's worked a lot on target specs IIRC.
2025-01-31 12:28:16 +01:00
Matthias Krüger
308ea7120b
Rollup merge of #135860 - fmease:compiler-mv-obj-save-dyn-compat-ii, r=jieyouxu
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
2025-01-31 12:28:15 +01:00
Matthias Krüger
86595e4c88
Rollup merge of #134531 - GuillaumeGomez:extract-doctests, r=notriddle,aDotInTheVoid
[rustdoc] Add `--extract-doctests` command-line flag

Part of https://github.com/rust-lang/rust/issues/134529.

It was discussed with the Rust-for-Linux project recently that they needed a way to extract doctests so they can modify them and then run them more easily (look for "a way to extract doctests" [here](https://github.com/Rust-for-Linux/linux/issues/2)).

For now, I output most of `ScrapedDoctest` fields in JSON format with `serde_json`. So it outputs the following information:

 * filename
 * line
 * langstr
 * text

cc `@ojeda`
r? `@notriddle`
2025-01-31 12:28:15 +01:00
Bastian Kersting
b151b513ba 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 is related to a 2025H1 project goal for better UB checks in debug
mode: https://github.com/rust-lang/rust-project-goals/pull/177.
2025-01-31 11:13:34 +00:00
Ralf Jung
d98270b07a miri: make float min/max non-deterministic 2025-01-31 11:56:02 +01:00
bors
7f36543a48 Auto merge of #136332 - jhpratt:rollup-aa69d0e, r=jhpratt
Rollup of 9 pull requests

Successful merges:

 - #132156 (When encountering unexpected closure return type, point at return type/expression)
 - #133429 (Autodiff Upstreaming - rustc_codegen_ssa, rustc_middle)
 - #136281 (`rustc_hir_analysis` cleanups)
 - #136297 (Fix a typo in profile-guided-optimization.md)
 - #136300 (atomic: extend compare_and_swap migration docs)
 - #136310 (normalize `*.long-type.txt` paths for compare-mode tests)
 - #136312 (Disable `overflow_delimited_expr` in edition 2024)
 - #136313 (Filter out RPITITs when suggesting unconstrained assoc type on too many generics)
 - #136323 (Fix a typo in conventions.md)

r? `@ghost`
`@rustbot` modify labels: rollup
2025-01-31 09:42:28 +00:00
Mads Marquart
7df38d94a1 Recommend adding target spec first, then std support later
Co-Authored-By: Jieyou Xu <jieyouxu@outlook.com>
2025-01-31 09:41:59 +01:00
bors
25a16572a3 Auto merge of #136331 - jhpratt:rollup-curo1f4, r=jhpratt
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
2025-01-31 06:55:04 +00:00
许杰友 Jieyou Xu (Joe)
780cd71d2f triagebot: remove myself from vacation 2025-01-31 14:46:13 +08:00
Jacob Pratt
e429adaeb1
Rollup merge of #136323 - etaoins:fix-typo-in-conventions-md, r=tgross35
Fix a typo in conventions.md

Introduced in #135950
2025-01-31 00:26:35 -05:00
Jacob Pratt
e6285b8ff1
Rollup merge of #136313 - compiler-errors:too-many-args, r=lqd
Filter out RPITITs when suggesting unconstrained assoc type on too many generics

Fixes #136233
2025-01-31 00:26:34 -05:00
Jacob Pratt
15ef406f9d
Rollup merge of #136312 - compiler-errors:overflow_delimited_expr-2024, r=ytmimi
Disable `overflow_delimited_expr` in edition 2024

This reverts the style guide changes and sets the default to "false" in rustfmt for style edition 2024.

r? `@ytmimi`

cc `@rust-lang/style` `@rust-lang/rustfmt`
2025-01-31 00:26:33 -05:00
Jacob Pratt
4e64180917
Rollup merge of #136310 - lqd:long-types, r=compiler-errors
normalize `*.long-type.txt` paths for compare-mode tests

When using a compare mode, the name of the test + compare-mode is embedded in some of rustc's output, like the location where a long type `bla.long-type(-some-hash)?.txt` is written to. That generally makes these tests fail under all compare-modes.

This PR fixes this by normalizing the compare-mode suffix away in the stderr output. We can also see some remnants of the long-removed `nll` compare mode being normalized away ^^.

I did this to fix some failures with `--compare-mode next-solver` (but it also fixes them with e.g. `--compare-mode polonius` of course):
- it makes 9 new tests pass with the new solver
- however, 3 tests I changed here still don't pass with the new solver (IIRC there were 2 ICEs, and some duplicate errors for the 3rd one)

(There was also one that triggered slowness in the new solver while triggering the long type failure, I'll mention this on zulip. )
2025-01-31 00:26:33 -05:00
Jacob Pratt
e2a73ab7ad
Rollup merge of #136300 - RalfJung:compare-and-swap, r=joboet
atomic: extend compare_and_swap migration docs

Fixes https://github.com/rust-lang/rust/issues/80486
2025-01-31 00:26:32 -05:00
Jacob Pratt
fc25599f4e
Rollup merge of #136297 - zeenix:patch-1, r=lqd
Fix a typo in profile-guided-optimization.md

It's either "profile-guided" or "profiled-guided" and I think it'sw the former. :)
2025-01-31 00:26:31 -05:00
Jacob Pratt
940b45f27c
Rollup merge of #136281 - nnethercote:rustc_hir_analysis, r=lcnr
`rustc_hir_analysis` cleanups

Just some improvements I found while looking through this code.

r? `@lcnr`
2025-01-31 00:26:31 -05:00
Jacob Pratt
c19c4b91f5
Rollup merge of #133429 - EnzymeAD:autodiff-middle, r=oli-obk
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`
2025-01-31 00:26:30 -05:00
Jacob Pratt
ae9dbf169f
Rollup merge of #132156 - estebank:closure-return, r=Nadrieril,compiler-errors
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.
2025-01-31 00:26:29 -05:00
Jacob Pratt
08dc8c931f
Rollup merge of #136296 - RalfJung:float-min-max, r=tgross35
float::min/max: mention the non-determinism around signed 0

Turns out this can actually produce different results on different machines [in practice](https://github.com/rust-lang/rust/issues/83984#issuecomment-2623859230); that seems worth documenting. I assume LLVM will happily const-fold these operations so so there could be different results for the same input even on the same machine, depending on whether things get const-folded or not.

`@nikic` I remember there was an LLVM soundness fix regarding scalar evolution for loops that had to recognize certain operations as non-deterministic... it seems to me that pass would also have to avoid predicting the result of `llvm.{min,max}num`, for the same reason?

r? `@tgross35`
Cc `@rust-lang/libs-api`

If this lands we should also make Miri non-deterministic here.

Fixes https://github.com/rust-lang/rust/issues/83984
2025-01-31 00:25:38 -05:00
Jacob Pratt
55512db4aa
Rollup merge of #136291 - lcnr:compare-mode, r=oli-obk
some test suite cleanups

found while checking `compare-mode next-solver`

r? `@oli-obk`

<sub> [lcnr](https://github.com/rust-lang/rust/commits?author=lcnr) authored and JJ_EMPTY_STRING committed </sub>
2025-01-31 00:25:38 -05:00
Jacob Pratt
b87df231c5
Rollup merge of #136287 - zmodem:nocapture, r=nikic
LLVM changed the nocapture attribute to captures(none)

This updates RustWrapper.cpp and tests after
https://github.com/llvm/llvm-project/pull/123181
2025-01-31 00:25:37 -05:00
Jacob Pratt
8b409e44da
Rollup merge of #136278 - lqd:polonius-debugger-episode-3, r=matthewjasper
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.
2025-01-31 00:25:36 -05:00
Jacob Pratt
dcf1134386
Rollup merge of #136270 - nnethercote:rm-NamedVarMap, r=jackh726
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`
2025-01-31 00:25:36 -05:00
Jacob Pratt
1a5ebc9e52
Rollup merge of #136258 - notriddle:notriddle/issue-d, r=fmease
rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 11)

Follow up

* https://github.com/rust-lang/rust/pull/134053
* https://github.com/rust-lang/rust/pull/130287

et al

As always, it's easier to review the commits one at a time. Don't use the Files Changed tab. It's confusing.
2025-01-31 00:25:35 -05:00
Jacob Pratt
7f05c7e50c
Rollup merge of #136150 - marcoieni:windows-25-i686-mingw, r=Kobzol
ci: use windows 2025 for i686-mingw

try-job: i686-mingw
2025-01-31 00:25:35 -05:00
Jacob Pratt
b249760c51
Rollup merge of #135414 - tgross35:stabilize-const_black_box, r=dtolnay
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
2025-01-31 00:25:34 -05:00
Nicholas Nethercote
0c47091006 Overhaul to_readable_str.
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.
2025-01-31 16:04:13 +11:00
Nicholas Nethercote
4ced93ed35 Don't export the Trivial* macros.
They're only used within the crate.
2025-01-31 16:04:13 +11:00
Nicholas Nethercote
140817380c Move find_self_call.
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.
2025-01-31 16:04:13 +11:00
Nicholas Nethercote
3f5e218581 Give a better explanation for having bug_fmt and span_bug_fmt. 2025-01-31 16:04:07 +11:00
Nicholas Nethercote
1d2cb611f7 Remove the mir_build hook.
It was downgraded from a query in #122721 but it can just be a vanilla
function because it's not called in `rustc_middle`.
2025-01-31 15:15:01 +11:00
Nicholas Nethercote
4b025ca083 Remove the thir_{tree,flat} hooks.
They were downgraded from queries in #123995 but they can just be
vanilla functions because they are not called in `rustc_middle`.
2025-01-31 15:13:51 +11:00
Nicholas Nethercote
be1aa7bb2a Add/clarify comments about hooks.
Explaining things that weren't clear to me at first.
2025-01-31 15:11:09 +11:00
bors
c37fbd873a Auto merge of #135318 - compiler-errors:vtable-fixes, r=lcnr
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 #135315
Fixes #135316
2025-01-31 04:09:11 +00:00