Remove placeholders completely by amandasystems · Pull Request #130227 · rust-lang/rust (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

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

Sep 11, 2024

rustbot added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Sep 11, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 27, 2024

…mpletely, r=

[WIP] Remove placeholders completely

This PR does shotgun surgery on borrowck to remove all special handling of placeholders, completely replacing them with a preprocessing step that rewrites placeholder leaks into constraints, removing constraint propagation of placeholders and all logic used to detect placeholder violations during error reporting. This finishes what rust-lang#123720 started.

The new method works like this:

1. during SCC construction, some information about SCC membership and reachability is retained
2. just after SCC construction, a constraint r - (from: to_invalid) - > 'static is added when r is the representative of an SCC and
   1. that SCC either has had its universe shrunk because it reaches a region with a smaller one (in which case to_invalid is the smallest-universed region it reaches), or if it reaches a region with a too large universe that isn't part of the SCC (in which case to_invalid is the region with a too large universe). In either case, from is also r.
3. some region reaches in r's SCC reaches another placeholder, reached, in which case the added constraint is r -> (reaches: reached) 'static. Through clever choice of defaults (chosing minimum elements), reached will be r if at all possible.

When tracing errors for diagnostics one of these special constraints along a path are treated much like a HTTP redirect: if we are explaining from: to and reach an edge with reaches: invalid we stop the search and start following reaches: invalid instead. When doing this the implicit edges x: 'static for every region x are ignored, since the search would otherwise be able to cheat by going through 'static and re-find the same edge again.

A bunch of optimisations are possible:

• Conservatively add constraints, e.g. one per SCC. May worsen error tracing!
• as a final pass, allow fusing the annotations for the SCC after adding the extra constraints to remove unnecessary information and save memory. This could be done cheaply since we already iterate over the entire SCC.
• currently, if constraints are added the entire set of SCCs are recomputed. This is of course rather wasteful, and we could maybe do better.

There are a bunch of rather nice bonuses:

• We now don't need to expose region indices in MirTypeckRegionConstraints to the entire crate. The only entry point is placeholder_region() so correctness of the indices is now guaranteed
• A lot of things that were previously iterations over lists is now a single lookup
• The constraint graph search functions are simple and at least one of them can now take a proper region as target rather than a predicate function. The only case that needs the predicate argument to find_constraint_path_to() is find_sub_region_live_at(), which may or may not be possible to work around.

r​? nikomatsakis

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 17, 2025

During work on rust-lang#130227, I discovered several situations not covered by any previously existing UI test. This commit introudces tests to cover that.

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 17, 2025

This rewires SCC annotations to have them be a separate, visitor-type data structure. It was broken out of rust-lang#130227, which needed them to be able to remove unused annotations after computation without recomputing the SCCs themselves.

As a drive-by it also removes some redundant code from the hot loop in SCC construction for a performance improvement.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Apr 17, 2025

…c-annotations, r=

Decouple SCC annotations from SCCs

This rewires SCC annotations to have them be a separate, visitor-type data structure. It was broken out of rust-lang#130227, which needed them to be able to remove unused annotations after computation without recomputing the SCCs themselves.

As a drive-by it also removes some redundant code from the hot loop in SCC construction for a performance improvement.

r? lcnr

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 17, 2025

During work on rust-lang#130227, I discovered several situations not covered by any previously existing UI test. This commit introudces tests to cover that.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Apr 17, 2025

…, r=lcnr

Add tests for two untested cases of placeholder relations

During work on rust-lang#130227, I discovered several situations not covered by any previously existing UI test. This commit introudces tests to cover that.

r? lcnr

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Apr 17, 2025

…, r=lcnr

Add tests for two untested cases of placeholder relations

During work on rust-lang#130227, I discovered several situations not covered by any previously existing UI test. This commit introudces tests to cover that.

r? lcnr

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 22, 2025

During work on rust-lang#130227, I discovered several situations not covered by any previously existing UI test. This commit introudces tests to cover that.

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 22, 2025

This rewires SCC annotations to have them be a separate, visitor-type data structure. It was broken out of rust-lang#130227, which needed them to be able to remove unused annotations after computation without recomputing the SCCs themselves.

As a drive-by it also removes some redundant code from the hot loop in SCC construction for a performance improvement.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Apr 22, 2025

…r=lcnr

Add tests for two untested cases of placeholder relations

During work on rust-lang#130227, I discovered several situations not covered by any previously existing UI test. This commit introudces tests to cover that.

r? lcnr

joshlf pushed a commit to joshlf/rust that referenced this pull request

Apr 22, 2025

During work on rust-lang#130227, I discovered several situations not covered by any previously existing UI test. This commit introudces tests to cover that.

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 24, 2025

This commit picks up a few odd ends discovered during the work on rust-lang#130227. It adds some documentation and renames a few methods with too generic names to describe what they actually do. It also adds some debug output that was helpful during bug hunting.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Apr 25, 2025

…leywiser

Some drive-by housecleaning in rustc_borrowck

This commit picks up a few odd ends discovered during the work on rust-lang#130227. It adds some documentation and renames a few methods with too generic names to describe what they actually do. It also adds some debug output that was helpful during bug hunting and generally cleans up a few things (for my values of "clean").

r? lcnr

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 25, 2025

This commit picks up a few odd ends discovered during the work on rust-lang#130227. It adds some documentation and renames a few methods with too generic names to describe what they actually do. It also adds some debug output that was helpful during bug hunting.

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Apr 25, 2025

Rollup merge of rust-lang#140257 - amandasystems:housecleaning, r=wesleywiser

Some drive-by housecleaning in rustc_borrowck

This commit picks up a few odd ends discovered during the work on rust-lang#130227. It adds some documentation and renames a few methods with too generic names to describe what they actually do. It also adds some debug output that was helpful during bug hunting and generally cleans up a few things (for my values of "clean").

r? lcnr

This commit moves all handling of placeholders, including those in type-tests, into a pre-processing step of the borrowcheck.

Along the way it extends the SCC metadata tracking a lot and uses the extra information to add p: 'static for any placeholder that reaches another placeholder. It also rewrites type-tests that have the same behaviour.

Error reporting is handled by introducing a new constraint kind that remembers which original outlives constraint caused an outlives-static. These work a lot like a HTTP redirect response during constraint search.

RegionInferenceContext is now a lot simpler and only tracks opaque regions without caring about their origin.

It also inlines the few measly bits of init_free_and_bound_regions() that still remain as relevant. This increases the constructor for RegionInferenceContexts somewhat, but I still think it's readable.

The documentation for init_free_and_bound_regions() was out of date, and the correct, up to date version is available in the various places where the logic was moved.

As a drive-by it also refactors the blame search somewhat, renames a few methods, and allows iterating over outgoing constraints without the implied edges from 'static.

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 28, 2025

This rewires SCC annotations to have them be a separate, visitor-type data structure. It was broken out of rust-lang#130227, which needed them to be able to remove unused annotations after computation without recomputing the SCCs themselves.

As a drive-by it also removes some redundant code from the hot loop in SCC construction for a performance improvement.

amandasystems added a commit to amandasystems/rust that referenced this pull request

Apr 29, 2025

This commit breaks out the logic of placheolder rewriting into its own preprocessing step. It's one of the more boring parts of rust-lang#130227.

The only functional change from this is that the preprocessing step (where extra r: 'static constraints are added) is performed upstream of Polonius legacy, finally affecting Polonius. That is mostly a by-product, though.

bors added a commit to rust-lang-ci/rust that referenced this pull request

May 1, 2025

…c-annotations, r=lcnr

Decouple SCC annotations from SCCs

This rewires SCC annotations to have them be a separate, visitor-type data structure. It was broken out of rust-lang#130227, which needed them to be able to remove unused annotations after computation without recomputing the SCCs themselves.

As a drive-by it also removes some redundant code from the hot loop in SCC construction for a performance improvement.

r? lcnr

bors added a commit to rust-lang-ci/rust that referenced this pull request

May 1, 2025

…c-annotations, r=lcnr

Decouple SCC annotations from SCCs

This rewires SCC annotations to have them be a separate, visitor-type data structure. It was broken out of rust-lang#130227, which needed them to be able to remove unused annotations after computation without recomputing the SCCs themselves.

As a drive-by it also removes some redundant code from the hot loop in SCC construction for a performance improvement.

r? lcnr

amandasystems added a commit to amandasystems/rust that referenced this pull request

May 2, 2025

This commit breaks out the logic of placheolder rewriting into its own preprocessing step. It's one of the more boring parts of rust-lang#130227.

The only functional change from this is that the preprocessing step (where extra r: 'static constraints are added) is performed upstream of Polonius legacy, finally affecting Polonius. That is mostly a by-product, though.