add `TypingMode::Borrowck` #138785

lcnr · 2025-03-21T13:19:00Z

Shares the first commit with #138499, doesn't really matter which PR to land first 😊 😁

Introduces TypingMode::Borrowck which unlike TypingMode::Analysis, uses the hidden type computed by HIR typeck as the initial value of opaques instead of an unconstrained infer var. This is a part of rust-lang/types-team#129.

Using this new TypingMode is unfortunately a breaking change for now, see tests/ui/impl-trait/non-defining-uses/as-projection-term.rs. Using an inference variable as the initial value results in non-defining uses in the defining scope. We therefore only enable it if with -Znext-solver=globally or -Ztyping-mode-borrowck

To do that the PR contains the following changes:

TypeckResults::concrete_opaque_type are already mapped to the definition of the opaque type
- writeback now checks that the non-lifetime parameters of the opaque are universal
- for this, fn check_opaque_type_parameter_valid is moved from rustc_borrowck to rustc_trait_selection
we add a new query type_of_opaque_hir_typeck which, using the same visitors as MIR typeck, attempts to merge the hidden types from HIR typeck from all defining scopes
- done by adding a DefiningScopeKind flag to toggle between using borrowck and HIR typeck
- the visitors stop checking that the MIR type matches the HIR type. This is trivial as the HIR type are now used as the initial hidden types of the opaque. This check is useful as a safeguard when not using TypingMode::Borrowck, but adding it to the new structure is annoying and it's not soundness critical, so I intend to not add it back.
add a TypingMode::Borrowck which behaves just like TypingMode::Analysis except when normalizing opaque types
- it uses type_of_opaque_hir_typeck(opaque) as the initial value after replacing its regions with new inference vars
- it uses structural lookup in the new solver

fixes #112201, fixes #132335, fixes #137751

r? @compiler-errors @oli-obk

add `TypingMode::Borrowck` Still not quite ready Based on rust-lang#138492 and rust-lang#138719 r? `@compiler-errors` `@oli-obk`

increment depth of nested obligations properly fixes the root cause of rust-lang#109268. While we didn't get hangs here before, I ended up encountering its root cause again with rust-lang#138785. r? types

compiler-errors · 2025-04-03T00:25:59Z

tests/ui/generic-associated-types/issue-87258_a.stderr

+LL |     fn foo<'a>() -> Self::FooFuture<'a> {
+   |        ^^^
+   |
+   = note: consider removing `#[define_opaque]` or adding an empty `#[define_opaque()]`


This note will need tweaking

Yeah, that's existing though and the suggestiono to add an empty #[define_opaque()] is applicable for ATPIT afaik? Or at least it should be

Yes, the "consider removing" part should be dependent on the existence of the attr, tho.

compiler-errors · 2025-04-03T00:27:01Z

tests/ui/impl-trait/issue-55872.rs

        //~^ ERROR type parameter `T` is part of concrete type but not used in parameter list for the `impl Trait` type alias
+        || ()


OK, yeah that seems like the major fallout of this PR (along w/ the bad spans for mismatched hidden types).

I guess most of these will not be a problem w/ RPIT.

compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs

we already collect opaque types from nested items during `mir_borrowck` of the root, checking that they are consistent this way.

oli-obk

makes all sense to me

oli-obk · 2025-04-03T09:55:13Z

compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs

            } else {
-                hir_opaque_ty = Some(concrete_type);
+                let hir_ty = tcx.type_of_opaque_hir_typeck(def_id).instantiate_identity();


When do we go down this route? If borrowck was tainted, we don't get here. If borrowck didn't find the opaque constraint because it was only in typeck, that's allowed for RPIT, but shouldn't we have gotten the HIR type in borrowck now that all the HIR types are seeding the borrowck table?

Well, this is reachable if neither -Ztyping-mode-borrowck or -Znext-solver=globally are enabled, right?

We're not seeding the borrowck table anymore, we only seed the default value when encountering an opaque. If the only uses of the opaque are in dead code, we still don't have any opaque in the opaque type storage in the old solver. cc #112417

I think the new solver may actually never reach this as we should try to eagerly normalize the fn sig which defines the opaque? But for now having only uses in dead code does reach this

compiler-errors

r=me,oli-obk

compiler-errors · 2025-04-04T18:06:08Z

compiler/rustc_middle/src/ty/typeck_results.rs

@@ -158,7 +158,7 @@ pub struct TypeckResults<'tcx> {
    /// We also store the type here, so that the compiler can use it as a hint


Update comment slightly?

actually just fold this into ur next pr, there's no reason to block this pr

compiler/rustc_pattern_analysis/src/rustc.rs

compiler-errors · 2025-04-04T18:10:32Z

@bors r=compiler-errors,oli-obk rollup=never

bors · 2025-04-04T18:10:34Z

📌 Commit 509a144 has been approved by compiler-errors,oli-obk

It is now in the queue for this repository.

bors · 2025-04-04T19:54:46Z

⌛ Testing commit 509a144 with merge 17ffbc81a30c094193836a5d7f90dff273b5df93...

bors · 2025-04-04T23:02:49Z

☀️ Test successful - checks-actions
Approved by: compiler-errors,oli-obk
Pushing 17ffbc8 to master...

github-actions · 2025-04-04T23:04:22Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 5337252 (parent) -> 17ffbc8 (this PR)

Test differences

Show 158 test diffs

Stage 1

[crashes] tests/crashes/112201.rs: pass -> [missing] (J1)
[crashes] tests/crashes/132335.rs: pass -> [missing] (J1)
[crashes] tests/crashes/137751.rs: pass -> [missing] (J1)
[ui] tests/ui/impl-trait/issues/fuzzer-ice-issue-112201.rs: [missing] -> pass (J1)
[ui] tests/ui/impl-trait/non-defining-uses/as-projection-term.rs#current: [missing] -> pass (J1)
[ui] tests/ui/impl-trait/non-defining-uses/as-projection-term.rs#next: [missing] -> pass (J1)
[ui] tests/ui/traits/next-solver/opaques/ambig-in-mir-typeck.rs: [missing] -> pass (J1)
[ui] tests/ui/traits/next-solver/opaques/revealing-use-in-nested-body.rs: [missing] -> pass (J1)
errors::verify_trait_selection_opaque_type_non_generic_param_23: [missing] -> pass (J2)
session_diagnostics::verify_borrowck_opaque_type_lifetime_mismatch_6: [missing] -> pass (J2)
session_diagnostics::verify_borrowck_opaque_type_lifetime_mismatch_7: pass -> [missing] (J2)
session_diagnostics::verify_borrowck_opaque_type_non_generic_param_6: pass -> [missing] (J2)
session_diagnostics::verify_borrowck_simd_intrinsic_arg_const_7: [missing] -> pass (J2)
session_diagnostics::verify_borrowck_simd_intrinsic_arg_const_8: pass -> [missing] (J2)
session_diagnostics::verify_borrowck_tail_expr_drop_order_8: [missing] -> pass (J2)
session_diagnostics::verify_borrowck_tail_expr_drop_order_9: pass -> [missing] (J2)

Stage 2

[ui] tests/ui/impl-trait/issues/fuzzer-ice-issue-112201.rs: [missing] -> pass (J0)
[ui] tests/ui/impl-trait/non-defining-uses/as-projection-term.rs#current: [missing] -> pass (J0)
[ui] tests/ui/impl-trait/non-defining-uses/as-projection-term.rs#next: [missing] -> pass (J0)
[ui] tests/ui/traits/next-solver/opaques/ambig-in-mir-typeck.rs: [missing] -> pass (J0)
[ui] tests/ui/traits/next-solver/opaques/revealing-use-in-nested-body.rs: [missing] -> pass (J0)
[crashes] tests/crashes/112201.rs: pass -> [missing] (J3)
[crashes] tests/crashes/132335.rs: pass -> [missing] (J3)
[crashes] tests/crashes/137751.rs: pass -> [missing] (J3)

Additionally, 134 doctest diffs were found. These are ignored, as they are noisy.

Job group index

J0: aarch64-apple, aarch64-gnu, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-apple-2, x86_64-gnu, x86_64-gnu-llvm-18-1, x86_64-gnu-llvm-18-2, x86_64-gnu-llvm-19-1, x86_64-gnu-llvm-19-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
J1: x86_64-gnu-llvm-18-3, x86_64-gnu-llvm-19-3
J2: aarch64-apple, aarch64-gnu, i686-gnu-2, i686-gnu-nopt-2, i686-msvc-2, x86_64-apple-1, x86_64-gnu, x86_64-gnu-llvm-18-3, x86_64-gnu-llvm-19-3, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-2, x86_64-msvc-2
J3: aarch64-apple, aarch64-gnu, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-apple-1, x86_64-gnu, x86_64-gnu-llvm-18-1, x86_64-gnu-llvm-18-2, x86_64-gnu-llvm-19-1, x86_64-gnu-llvm-19-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1

Job duration changes

i686-gnu-2: 6541.8s -> 7285.8s (11.4%)
dist-apple-various: 6743.6s -> 7221.2s (7.1%)
dist-aarch64-msvc: 8523.0s -> 9050.5s (6.2%)
dist-x86_64-mingw: 7658.0s -> 8037.7s (5.0%)
i686-msvc-2: 7249.5s -> 7599.6s (4.8%)
x86_64-mingw-2: 7163.9s -> 7448.9s (4.0%)
aarch64-apple: 3876.0s -> 4026.2s (3.9%)
x86_64-gnu-llvm-18-1: 5509.5s -> 5678.9s (3.1%)
x86_64-msvc-1: 8483.4s -> 8736.9s (3.0%)
dist-i686-linux: 6046.3s -> 6203.5s (2.6%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-04-05T00:18:59Z

Finished benchmarking commit (17ffbc8): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -2.3%, secondary 3.5%)

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
Regressions ❌ (secondary)	3.5%	[2.2%, 4.7%]	2
Improvements ✅ (primary)	-2.3%	[-2.7%, -1.9%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.3%	[-2.7%, -1.9%]	2

Cycles

Results (secondary 3.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
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 779.508s -> 780.342s (0.11%)
Artifact size: 365.96 MiB -> 366.00 MiB (0.01%)

compiler-errors · 2025-04-05T16:32:50Z

@craterbot run mode=check-only start=master#5337252b9952fdd9482ed6a4add17254e5bd2c40 end=master#17ffbc81a30c094193836a5d7f90dff273b5df93

craterbot · 2025-04-05T16:33:03Z

👌 Experiment pr-138785 created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-04-05T16:33:16Z

🚧 Experiment pr-138785 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

rustbot assigned compiler-errors Mar 21, 2025