Move placeholder error handling to before region inference by amandasystems · Pull Request #142623 · rust-lang/rust

amandasystems · 2025-06-17T12:08:40Z

This PR supersedes #130227. It removes precise tracking of placeholders in borrowck, moving them to an earlier stage. This leads to better performance, since we track less information. It is also part of the preparations for Polonius by moving specialised logic out of region inference itself and into the region graph or other, common pre-processing steps. Originally, the intention was to do this in multiple steps, but since some of them incur performance penalties won back by later stages, doing them in one go makes the net results clearer.

Error reporting changes

Higher-kinded region errors (placeholder outliving a placeholder, etc) are only reported if there are no other borrowck errors.
Due to the less precise nature of the revised tracking, there can be multiple overlapping errors involving the same placeholders/existential regions. In that case, not all errors will be reported. Crucially, only one pair per constraint graph SCC will be reported. A common case is Trait<r1, r2>, which frequently gives two error messages, one for each region. Under this logic it only gives one.
In some cases, finding named regions to slot into diagnostics is now more difficult, since not all outlived named regions are traced. This degrades some diagnostics by replacing named regions with anonymous ones, or by picking differently named ones.
In precisely one test, tests/ui/impl-trait/nested-rpit-hrtb.rs, we now also add r: 'static, which causes an extra error. Being more conservative when rewriting the constraint graph and avoiding it when invalid outlives are already detected breaks existing diagnostics for many, many tests that rely on the extra r: 'static to find out why they error.

Due to reason 2., several UI tests change, a lot, since they rely on diagnostic duplication being turned off to find several duplicate errors.

Code structure changes

Placeholder errors are now detected earlier, if they happen.
SCC annotations are dynamically chosen to be less rich for region graphs with no placeholders, opting to skip a lot of the logic. This is crucial for performance; both runtime and memory use, since it allows simple function bodies to skip some allocations and extra computation during SCC construction.
SCC annotations are now written in an update-style as opposed to the previous, more functional style. It seems to be more performant and it's certainly more succinct.

r? lcnr

amandasystems · 2025-06-17T14:30:13Z

Wow, a merge conflict with upstream for a commit I JUST PUSHED, that's record time!

lcnr · 2025-07-03T13:51:10Z

@rustbot blocked

amandasystems · 2025-08-27T20:01:38Z

@rustbot ready

amandasystems · 2025-08-27T20:03:43Z

tests/ui/async-await/higher-ranked-auto-trait-13.assumptions.stderr

-   = note: `Getter<'1>` would have to be implemented for the type `GetterImpl<'0, ConstructableImpl<'_>>`, for any two lifetimes `'0` and `'1`...
+   = note: `Getter<'_>` would have to be implemented for the type `GetterImpl<'0, ConstructableImpl<'1>>`, for any two lifetimes `'0` and `'1`...


I hate this, but can't find a way around it. My gut feeling says it's another case of an order-dependent extraction procedure somewhere. For what it's worth, it wasn't in the earlier version of this code but it seems to have appeared after the recent changes to the base commit. Presumably the difference is from selecting smallest placeholder by rvid, etc. Help very much appreciated!

amandasystems · 2025-09-03T09:19:35Z

compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs

-            error_element
-        {
-            let adjusted_universe =
-                error_placeholder.universe.as_u32().checked_sub(base_universe.as_u32());


I think the "checked sub" part is what really broke me here. Why are universes being adjusted? What's a base universe? And why can the subtraction sometimes overflow?!?!

See also bound_region_errors, line 502

For the record, these are the only reads of base_universe on CanonicalTypeOpDeeplyNormalizeGoal, CanonicalTypeOpAscribeUserTypeGoal and InstantiateOpaqueType.

amandasystems · 2025-09-11T10:05:48Z

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)

 95 | |         constraints.clone(),
 96 | |         &universal_region_relations,
 97 | |         infcx,
 98 | |     );
    | |_____- argument #4 of type `&mut RegionErrors<'_>` is missing
    |
note: function defined here
   --> compiler/rustc_borrowck/src/handle_placeholders.rs:306:15
    |
306 | pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>(
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
310 |     errors_buffer: &mut RegionErrors<'tcx>,
    |     --------------------------------------
help: provide the argument
    |
 94 -     let lowered_constraints = compute_sccs_applying_placeholder_outlives_constraints(
 95 -         constraints.clone(),
 96 -         &universal_region_relations,
 97 -         infcx,
 98 -     );
 94 +     let lowered_constraints = compute_sccs_applying_placeholder_outlives_constraints(constraints.clone(), &universal_region_relations, infcx, /* &mut RegionErrors<'_> */);
    |

For more information about this error, try `rustc --explain E0061`.
[RUSTC-TIMING] rustc_borrowck test:false 4.478
error: could not compile `rustc_borrowck` (lib) due to 1 previous error

I don't understand where these errors come from. On my branch every single commit passes the ./x check command. What's emitting these? The error refers to a line that doesn't exist in a file that doesn't look like that during any commit in the PR.

bors · 2025-12-10T20:47:25Z

☔ The latest upstream changes (presumably #149535) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2026-01-12T15:03:38Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

- coroutine/auto-trait-regions.rs: also reformulate the test to make sure we don't miss errors -- just duplicate ones. - traits/trait-upcasting/higher-ranked-upcasting-ub.rs: remove duplicate ERROR mismatched types - ui/associated-types/associated-types-eq-hr.rs: remove 5 duplicate errors for "implementation not general enough" and mismatched types" - ui/borrowck/opaque-types-patterns-subtyping-ice-104779.rs: two redundant higher-ranked subtype errors

…er-ranked/higher-ranked-lifetime-equality.rs: remove literally idential error.

`bound_inv_a_b_vs_bound_inv_a` has one fewer identical error

… "not general enough"

…al "one type is more general"

…ong enough" This error is caused as a side-effect from added `r: 'static` constraints. It should not be necessary, but beig less aggressive with rewriting breaks many other diagnostics, and this is closer to the original logic.

lcnr · 2026-01-19T14:58:22Z

compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs

-            } else {
-                None
-            };
+        // FIXME: one day this should just be error_element, but see above about the adjusted universes. At that point, this method can probably be removed entirely.


Suggested change

// FIXME: one day this should just be error_element, but see above about the adjusted universes. At that point, this method can probably be removed entirely.

// FIXME: one day this should just be error_element,

// but see above about the adjusted universes. At that point,

// this method can probably be removed entirely.

lcnr · 2026-01-19T15:01:24Z

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

+            };
+            if bound_generic_params
+                .iter()
+                .rfind(|bgp| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)


Suggested change

.rfind(|bgp| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)

.rfind(|bgp| tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)

lcnr · 2026-01-19T15:02:06Z

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

+            };
+            if bound_generic_params
+                .iter()
+                .rfind(|bgp| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)


Suggested change

.rfind(|bgp| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)

.rfind(|bgp| tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)

lcnr · 2026-01-19T15:02:35Z

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

+        // FIXME(amandasystems) we can probably flatten this.
+        for pred in self
+            .infcx
+            .tcx


why use self.infcx.tcx instead of just tcx?

amandasystems · 2026-01-22T16:14:00Z

After discussing this with @lcnr we've decided to abandon this avenue. I'll submit a few PRs with cleanups and nice lessons from this work. It turns out that the main thrust of this -- removing placeholder tracking -- is incompatible with future trait solver work that would involve richer assumptions about outlives constraints.

Checking these would require knowing precisely which placeholder regions end up where and is thus incompatible with the spirit of this PR.

This change backports some changes from the now abandoned rust-lang#142623. Notably, it simplifies the `PlaceholderReachability` `enum` by replacing the case when no placeholders were reached with a standard `Option::None`. It also rewrites the API for `scc::Annotations` to be update-mut rather than a more Functional programming style. This snowed some slight performance impact in early tests of the PR and definitely makes the implementation simpler.

@lcnr

…te, r=lcnr Borrowck: Simplify SCC annotation computation, placeholder rewriting This change backports some changes from the now abandoned rust-lang#142623. Notably, it simplifies the `PlaceholderReachability` `enum` by replacing the case when no placeholders were reached with a standard `Option::None`. It also rewrites the API for `scc::Annotations` to be update-mut rather than a more Functional programming style. This showed some slight performance impact in early tests of the PR and definitely makes the implementation simpler. This probably wants a perf run just for good measure. r? @lcnr

@lcnr

Rollup merge of #151696 - amandasystems:scc-annotations-update, r=lcnr Borrowck: Simplify SCC annotation computation, placeholder rewriting This change backports some changes from the now abandoned #142623. Notably, it simplifies the `PlaceholderReachability` `enum` by replacing the case when no placeholders were reached with a standard `Option::None`. It also rewrites the API for `scc::Annotations` to be update-mut rather than a more Functional programming style. This showed some slight performance impact in early tests of the PR and definitely makes the implementation simpler. This probably wants a perf run just for good measure. r? @lcnr

@lcnr

Borrowck: Simplify SCC annotation computation, placeholder rewriting This change backports some changes from the now abandoned rust-lang/rust#142623. Notably, it simplifies the `PlaceholderReachability` `enum` by replacing the case when no placeholders were reached with a standard `Option::None`. It also rewrites the API for `scc::Annotations` to be update-mut rather than a more Functional programming style. This showed some slight performance impact in early tests of the PR and definitely makes the implementation simpler. This probably wants a perf run just for good measure. r? @lcnr

@lcnr

Borrowck: Simplify SCC annotation computation, placeholder rewriting This change backports some changes from the now abandoned rust-lang/rust#142623. Notably, it simplifies the `PlaceholderReachability` `enum` by replacing the case when no placeholders were reached with a standard `Option::None`. It also rewrites the API for `scc::Annotations` to be update-mut rather than a more Functional programming style. This showed some slight performance impact in early tests of the PR and definitely makes the implementation simpler. This probably wants a perf run just for good measure. r? @lcnr

rustbot assigned lcnr Jun 17, 2025

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 Jun 17, 2025

This comment has been minimized.

Sign in to view

amandasystems force-pushed the early-placeholder-errors branch 2 times, most recently from fd18095 to 199f961 Compare June 17, 2025 15:11

amandasystems changed the title ~~[WIP] Move placeholder error handling to before region inference~~ Move placeholder error handling to before region inference Jun 25, 2025

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 3, 2025

amandasystems mentioned this pull request Jul 7, 2025

Remove precise placeholder tracking from region inference #130227

Closed

amandasystems force-pushed the early-placeholder-errors branch from 199f961 to 3a46042 Compare August 27, 2025 11:03

This comment has been minimized.

Sign in to view

amandasystems force-pushed the early-placeholder-errors branch from 3a46042 to 9b555d3 Compare August 27, 2025 19:44

This comment has been minimized.

Sign in to view

amandasystems force-pushed the early-placeholder-errors branch from 9b555d3 to f31393f Compare August 27, 2025 20:01

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 27, 2025

amandasystems commented Aug 27, 2025

View reviewed changes

amandasystems force-pushed the early-placeholder-errors branch from f31393f to f542ec5 Compare September 3, 2025 09:17

amandasystems commented Sep 3, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

amandasystems force-pushed the early-placeholder-errors branch from f542ec5 to 8269a42 Compare September 11, 2025 09:41

This comment has been minimized.

Sign in to view

amandasystems force-pushed the early-placeholder-errors branch from 8269a42 to e0b30fa Compare September 11, 2025 10:15

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 25, 2025

amandasystems force-pushed the early-placeholder-errors branch from fbe2418 to a9e1a79 Compare January 12, 2026 15:03

This comment has been minimized.

Sign in to view

amandasystems added 2 commits January 15, 2026 15:26

Move handling of placeholder errors to before region inference

31b7743

amandasystems force-pushed the early-placeholder-errors branch from a9e1a79 to 21ddce3 Compare January 15, 2026 14:58

amandasystems added 5 commits January 15, 2026 16:03

tests/ui/higher-ranked/higher-ranked-lifetime-equality.rsests/ui/high…

998cc4a

…er-ranked/higher-ranked-lifetime-equality.rs: remove literally idential error.

tests/ui/higher-ranked/subtype/hr-subtype.rs: remove redundant error

edbbc88

`bound_inv_a_b_vs_bound_inv_a` has one fewer identical error

higher-ranked/trait-bounds/hrtb-conflate-regions.rs: remove redundant…

339b5a0

… "not general enough"

tests/ui/lub-glb/old-lub-glb-object.rs: remove literally identical error

f935d57

tests/ui/nll/relate_tys/hr-fn-aau-eq-abu.rs: remove literally identic…

09133f6

…al "one type is more general"

This comment has been minimized.

Sign in to view

amandasystems added 4 commits January 15, 2026 16:55

These explicitly marked as such duplicate errors are gone too

9bacce1

Removed redundant error, placeholders move around a bit

f631208

Missing note about limitations in the borrow checker

52f3cac

lcnr reviewed Jan 19, 2026

View reviewed changes

lcnr reviewed Jan 22, 2026

View reviewed changes

amandasystems closed this Jan 22, 2026

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2026

amandasystems mentioned this pull request Jan 26, 2026

Borrowck: Simplify SCC annotation computation, placeholder rewriting #151696

Merged

amandasystems mentioned this pull request Jan 30, 2026

Borrowck: simplify diagnostics for placeholders #151863

Open

		= note: `Getter<'1>` would have to be implemented for the type `GetterImpl<'0, ConstructableImpl<'_>>`, for any two lifetimes `'0` and `'1`...
		= note: `Getter<'_>` would have to be implemented for the type `GetterImpl<'0, ConstructableImpl<'1>>`, for any two lifetimes `'0` and `'1`...

	.rfind(\|bgp\| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)
	.rfind(\|bgp\| tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id)

Uh oh!

Conversation

amandasystems commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Error reporting changes

Code structure changes

Uh oh!

This comment has been minimized.

amandasystems commented Jun 17, 2025

Uh oh!

lcnr commented Jul 3, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

amandasystems commented Aug 27, 2025

Uh oh!

amandasystems Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

amandasystems Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amandasystems Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

amandasystems Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

amandasystems commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Dec 10, 2025

Uh oh!

rustbot commented Jan 12, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

lcnr Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

lcnr Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

lcnr Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

lcnr Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

amandasystems commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

amandasystems commented Jun 17, 2025 •

edited

Loading

amandasystems Sep 3, 2025 •

edited

Loading

amandasystems commented Sep 11, 2025 •

edited

Loading