Move placeholder handling to a proper preprocessing step #140466
+421
−287
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
rustbot
added
S-waiting-on-review
lcnr
reviewed
Apr 30, 2025
|
r? lcnr |
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.
amandasystems
force-pushed
the
move-to-preprocessing-step
branch
from
8b2cab2 to
af75e13
Compare
amandasystems
changed the title
18 tasks
|
Ok; revised SCC annotations have landed and I think I addressed all of your concerns so far @lcnr? |
lcnr
reviewed
May 15, 2025
lcnr
reviewed
May 15, 2025
lcnr
reviewed
May 15, 2025
amandasystems
force-pushed
the
move-to-preprocessing-step
branch
from
af75e13 to
ef9cb23
Compare
|
@lcnr Fixed all the code review stuff now, I think! I amended it into the second commit. |
lcnr
reviewed
May 22, 2025
| max_placeholder_universe_reached: UniverseIndex, | ||
|
|
||
| /// The smallest universe index reachable form the nodes of this SCC. | ||
| min_reachable_universe: UniverseIndex, |
There was a problem hiding this comment.
Suggested change
| min_reachable_universe: UniverseIndex, | |
| max_nameable_universe: UniverseIndex, |
:3
| } | ||
|
|
||
| /// The smallest-indexed universe reachable from and/or in this SCC. | ||
| pub(crate) fn min_universe(self) -> UniverseIndex { |
There was a problem hiding this comment.
Suggested change
| pub(crate) fn min_universe(self) -> UniverseIndex { | |
| pub(crate) fn max_nameable_universe(self) -> UniverseIndex { |
| } | ||
|
|
||
| /// Determines if the region variable definitions contain | ||
| /// placeholers, and compute them for later use. |
There was a problem hiding this comment.
Suggested change
| /// placeholers, and compute them for later use. | |
| /// placeholders, and compute them for later use. |
| sup: annotation.representative.rvid(), | ||
| sub: fr_static, | ||
| category: ConstraintCategory::IllegalUniverse, | ||
| locations: Locations::All(rustc_span::DUMMY_SP), |
There was a problem hiding this comment.
could we track a span related to the placeholder in the scc annotation and use that here?
| pub(crate) definitions: Frozen<IndexVec<RegionVid, RegionDefinition<'tcx>>>, | ||
| pub(crate) scc_annotations: IndexVec<ConstraintSccIndex, RegionTracker>, | ||
| pub(crate) member_constraints: MemberConstraintSet<'tcx, RegionVid>, | ||
| pub(crate) outlives_constraints: OutlivesConstraintSet<'tcx>, |
There was a problem hiding this comment.
Suggested change
| pub(crate) outlives_constraints: OutlivesConstraintSet<'tcx>, | |
| pub(crate) outlives_constraints: Frozen<OutlivesConstraintSet<'tcx>>, |
| outlives_constraints.push(scc_representative_outlives_static); | ||
| } | ||
| } | ||
| outlives_static |
There was a problem hiding this comment.
feel like this can just return a bool? and we add a debug! stmtm when adding an outlives constraint here
| /// Since universes can also be involved in errors (if one placeholder | ||
| /// transitively outlives another), this function also flags those. | ||
| /// | ||
| /// Additionally, it similarly rewrites type-tests. |
There was a problem hiding this comment.
no :> generally, this comment isn't applicable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit breaks out the logic of placheolder rewriting into its own preprocessing step. It's one of the more boring
parts of #130227.
The only functional change from this is that the preprocessing step (where extra
r: 'staticconstraints are added) is performed upstream of Polonius legacy, finally affecting Polonius. That is mostly a by-product, though.This should be reviewable by anyone in the compiler team, so
r? rust-lang/compiler