Normalize type outlives obligations in NLL for new solver #120513

compiler-errors · 2024-01-30T21:56:08Z

Normalize the type outlives assumptions and obligations in MIR borrowck. This should fix any of the lazy-norm-related MIR borrowck problems.

Also some cleanups from last PR:

Normalize obligations in a loop in lexical region resolution
Use deeply_normalize_with_skipped_universes in lexical resolution since we may have, e.g. for<'a> Alias<'a>: 'b.

r? lcnr

compiler-errors · 2024-01-30T21:56:33Z

compiler/rustc_infer/src/infer/outlives/obligations.rs

-            );
-            let category = origin.to_constraint_category();
-            outlives.type_must_outlive(origin, sup_type, sub_region, category);
+        // Must loop since the process of normalizing may itself register region obligations.


I don't know whether this is purely theoretical or if it could be used for unsoundness, actually.

compiler-errors · 2024-01-30T21:59:13Z

@bors try @rust-timer queue

…r-nll, r=<try> Normalize type outlives obligations in NLL for new solver Normalize the type outlives assumptions and obligations in MIR borrowck. This should fix any of the lazy-norm-related MIR borrowck problems. Also some cleanups from last PR: 1. Normalize obligations in a loop in lexical region resolution 2. Use `deeply_normalize_with_skipped_universes` in lexical resolution since we may have, e.g. `for<'a> Alias<'a>: 'b`. r? lcnr

bors · 2024-01-30T22:00:22Z

⌛ Trying commit 870f77c with merge 78d5ab6...

bors · 2024-01-30T23:30:45Z

☀️ Try build successful - checks-actions
Build commit: 78d5ab6 (78d5ab60709b3df7978f028f2d525cea3df23c21)

rust-timer · 2024-01-31T00:44:28Z

Finished benchmarking commit (78d5ab6): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

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)	2.2%	[0.7%, 4.6%]	39
Regressions ❌ (secondary)	3.0%	[0.8%, 5.2%]	77
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.2%	[0.7%, 4.6%]	39

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: 660.046s -> 659.99s (-0.01%)
Artifact size: 308.08 MiB -> 308.11 MiB (0.01%)

lcnr · 2024-02-01T08:34:26Z

compiler/rustc_trait_selection/src/regions.rs

@@ -24,12 +24,13 @@ impl<'tcx> InferCtxtRegionExt<'tcx> for InferCtxt<'tcx> {
            let ty = self.resolve_vars_if_possible(ty);

            if self.next_trait_solver() {
-                crate::solve::deeply_normalize(
+                crate::solve::deeply_normalize_with_skipped_universes(


probably nicer to change process_registered_region_obligations to first rebind and then normalize instead of accepting skipped universes here?

This is somewhat annoying, since then the deeply_normalize predicate needs to take a PolyTypeOutlivesPredicate here. When processing region obligations, these type outlives obligations are not bound, so we'd need to bind them up, and then skip their binder again. It just feels redundant :/

I'd rather do something like pass the outermost exclusive binder as part of normalize so we ICE if we find bound vars further than ty::INNERMOST, and only when normalizing the param-envs.

When processing region obligations, these type outlives obligations are not bound, so we'd need to bind them up, and then skip their binder again. It just feels redundant :/

I would prefer doing so and using ty::Binder::dummy(sup_type) when normalizing region obligations 🤔 we can talk about this in sync, but I strongly dislike having escaping bound vars anywhere. And believe that having to think about escaping bound vars adds unnecessary mental complexity. While this is mostly vibe based, I do feel fairly strongly about this.

compiler/rustc_borrowck/src/type_check/constraint_conversion.rs

lcnr

can you add a FIXME(-Znext-solver) to both loops that its likely they'll have to handle infinite recursion somehow? can imagine this to cause hangs

compiler/rustc_borrowck/src/type_check/constraint_conversion.rs

lcnr · 2024-02-02T09:11:21Z

compiler/rustc_borrowck/src/type_check/free_region_relations.rs

+
+            // In the new solver, normalize the type-outlives obligation assumptions.
+            if self.infcx.next_trait_solver() {
+                match deeply_normalize_with_skipped_universes(


this can definitely use deeply_normalize by moving the rebind eagerly :<

lcnr · 2024-02-02T09:18:29Z

compiler/rustc_trait_selection/src/regions.rs

@@ -24,12 +24,13 @@ impl<'tcx> InferCtxtRegionExt<'tcx> for InferCtxt<'tcx> {
            let ty = self.resolve_vars_if_possible(ty);

            if self.next_trait_solver() {
-                crate::solve::deeply_normalize(
+                crate::solve::deeply_normalize_with_skipped_universes(


When processing region obligations, these type outlives obligations are not bound, so we'd need to bind them up, and then skip their binder again. It just feels redundant :/

I would prefer doing so and using ty::Binder::dummy(sup_type) when normalizing region obligations 🤔 we can talk about this in sync, but I strongly dislike having escaping bound vars anywhere. And believe that having to think about escaping bound vars adds unnecessary mental complexity. While this is mostly vibe based, I do feel fairly strongly about this.

…utlives

lcnr · 2024-02-02T20:41:43Z

@bors r+ rollup

bors · 2024-02-02T20:41:46Z

📌 Commit e951bcf has been approved by lcnr

It is now in the queue for this repository.

bors · 2024-02-02T20:41:47Z

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

lcnr · 2024-02-02T20:41:56Z

@bors rollup=never

compiler-errors · 2024-02-05T16:24:33Z

@bors rollup=maybe

…for-nll, r=lcnr Normalize type outlives obligations in NLL for new solver Normalize the type outlives assumptions and obligations in MIR borrowck. This should fix any of the lazy-norm-related MIR borrowck problems. Also some cleanups from last PR: 1. Normalize obligations in a loop in lexical region resolution 2. Use `deeply_normalize_with_skipped_universes` in lexical resolution since we may have, e.g. `for<'a> Alias<'a>: 'b`. r? lcnr

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#119614 (unstably allow constants to refer to statics and read from immutable statics) - rust-lang#119939 (Improve 'generic param from outer item' error for `Self` and inside `static`/`const` items) - rust-lang#120206 (hir: Make sure all `HirId`s have corresponding HIR `Node`s) - rust-lang#120331 (pattern_analysis: use a plain `Vec` in `DeconstructedPat`) - rust-lang#120396 (Account for unbounded type param receiver in suggestions) - rust-lang#120423 (update indirect structural match lints to match RFC and to show up for dependencies) - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg) - rust-lang#120502 (Remove `ffi_returns_twice` feature) - rust-lang#120507 (Account for non-overlapping unmet trait bounds in suggestion) - rust-lang#120513 (Normalize type outlives obligations in NLL for new solver) r? `@ghost` `@rustbot` modify labels: rollup

…for-nll, r=lcnr Normalize type outlives obligations in NLL for new solver Normalize the type outlives assumptions and obligations in MIR borrowck. This should fix any of the lazy-norm-related MIR borrowck problems. Also some cleanups from last PR: 1. Normalize obligations in a loop in lexical region resolution 2. Use `deeply_normalize_with_skipped_universes` in lexical resolution since we may have, e.g. `for<'a> Alias<'a>: 'b`. r? lcnr

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#119939 (Improve 'generic param from outer item' error for `Self` and inside `static`/`const` items) - rust-lang#120206 (hir: Make sure all `HirId`s have corresponding HIR `Node`s) - rust-lang#120331 (pattern_analysis: use a plain `Vec` in `DeconstructedPat`) - rust-lang#120396 (Account for unbounded type param receiver in suggestions) - rust-lang#120423 (update indirect structural match lints to match RFC and to show up for dependencies) - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg) - rust-lang#120502 (Remove `ffi_returns_twice` feature) - rust-lang#120507 (Account for non-overlapping unmet trait bounds in suggestion) - rust-lang#120513 (Normalize type outlives obligations in NLL for new solver) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#119939 (Improve 'generic param from outer item' error for `Self` and inside `static`/`const` items) - rust-lang#120331 (pattern_analysis: use a plain `Vec` in `DeconstructedPat`) - rust-lang#120396 (Account for unbounded type param receiver in suggestions) - rust-lang#120423 (update indirect structural match lints to match RFC and to show up for dependencies) - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg) - rust-lang#120502 (Remove `ffi_returns_twice` feature) - rust-lang#120507 (Account for non-overlapping unmet trait bounds in suggestion) - rust-lang#120513 (Normalize type outlives obligations in NLL for new solver) - rust-lang#120707 (Don't expect early-bound region to be local when reporting errors in RPITIT well-formedness) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#120513 - compiler-errors:normalize-regions-for-nll, r=lcnr Normalize type outlives obligations in NLL for new solver Normalize the type outlives assumptions and obligations in MIR borrowck. This should fix any of the lazy-norm-related MIR borrowck problems. Also some cleanups from last PR: 1. Normalize obligations in a loop in lexical region resolution 2. Use `deeply_normalize_with_skipped_universes` in lexical resolution since we may have, e.g. `for<'a> Alias<'a>: 'b`. r? lcnr

rustbot assigned lcnr Jan 30, 2024

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. labels Jan 30, 2024

compiler-errors commented Jan 30, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 30, 2024

compiler-errors mentioned this pull request Jan 30, 2024

-Znext-solver with TinyVec and Rand #119262

Closed

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 31, 2024

lcnr reviewed Feb 1, 2024

View reviewed changes

compiler/rustc_borrowck/src/type_check/constraint_conversion.rs Outdated Show resolved Hide resolved

lcnr reviewed Feb 1, 2024

View reviewed changes

rust-cloud-vms bot force-pushed the normalize-regions-for-nll branch from 870f77c to 2c3e238 Compare February 1, 2024 18:06

lcnr reviewed Feb 2, 2024

View reviewed changes

compiler-errors added 5 commits February 2, 2024 18:30

Use deeply_normalize_with_skipped_universes in when processing type o…

0e16885

…utlives

Do process_registered_region_obligations in a loop

7576b77

Normalize type outlives obligations in NLL

7d1fda7

Don't hang when there's an infinite loop of outlives obligations

a371059

Normalize the whole PolyTypeOutlivesPredicate, more simplifications

e951bcf

rust-cloud-vms bot force-pushed the normalize-regions-for-nll branch from 2c3e238 to e951bcf Compare February 2, 2024 18:31

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 2, 2024

matthiaskrgr mentioned this pull request Feb 6, 2024

Rollup of 10 pull requests #120709

Closed

matthiaskrgr mentioned this pull request Feb 6, 2024

Rollup of 9 pull requests #120714

Closed

matthiaskrgr mentioned this pull request Feb 6, 2024

Rollup of 9 pull requests #120722

Merged

bors merged commit 012ce8a into rust-lang:master Feb 7, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Feb 7, 2024

lcnr mentioned this pull request Feb 9, 2024

we eagerly normalize Type-outlives bounds rust-lang/trait-system-refactor-initiative#63

Open

compiler-errors mentioned this pull request Apr 23, 2024

Outlives obligations aren't normalized before processing rust-lang/trait-system-refactor-initiative#61

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize type outlives obligations in NLL for new solver #120513

Normalize type outlives obligations in NLL for new solver #120513

compiler-errors commented Jan 30, 2024

compiler-errors Jan 30, 2024

compiler-errors commented Jan 30, 2024

This comment has been minimized.

bors commented Jan 30, 2024

bors commented Jan 30, 2024

This comment has been minimized.

rust-timer commented Jan 31, 2024

lcnr Feb 1, 2024

compiler-errors Feb 1, 2024

lcnr Feb 2, 2024

lcnr left a comment

lcnr Feb 2, 2024

lcnr Feb 2, 2024

lcnr commented Feb 2, 2024

bors commented Feb 2, 2024

bors commented Feb 2, 2024

lcnr commented Feb 2, 2024

compiler-errors commented Feb 5, 2024

Normalize type outlives obligations in NLL for new solver #120513

Normalize type outlives obligations in NLL for new solver #120513

Conversation

compiler-errors commented Jan 30, 2024

Choose a reason for hiding this comment

compiler-errors commented Jan 30, 2024

This comment has been minimized.

bors commented Jan 30, 2024

bors commented Jan 30, 2024

This comment has been minimized.

rust-timer commented Jan 31, 2024

Overall result: no relevant changes - no action needed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr commented Feb 2, 2024

bors commented Feb 2, 2024

bors commented Feb 2, 2024

lcnr commented Feb 2, 2024

compiler-errors commented Feb 5, 2024