remove more trivial regions in evaluate_added_goals_and_make_canonical_response - #162032
Open
sjwang05 wants to merge 1 commit into
Open
remove more trivial regions in evaluate_added_goals_and_make_canonical_response#162032sjwang05 wants to merge 1 commit into
sjwang05 wants to merge 1 commit into
Conversation
rustbot
added
S-waiting-on-review
Contributor
Author
|
Fun fact: the number of outlives constraints grows exactly as fib(2 * depth) :D |
This comment has been minimized.
This comment has been minimized.
…ke_canonical_response
sjwang05
force-pushed
the
more-region-dedup
branch
from
August 31, 2026 00:16
0125479 to
bc71d10
Compare
Member
|
@bors try @rust-timer queue |
Collaborator
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
rust-bors Bot
pushed a commit
that referenced
this pull request
Aug 31, 2026
remove more trivial regions in evaluate_added_goals_and_make_canonical_response
|
|
||
| // If we have a constraint like `'re: '?1`, and `?1` appears nowhere else in the response | ||
| // besides the constraint itself, then this kind of constraint is also trivial, since | ||
| // one can always pick `'?1 := 'empty`, and `'re: 'empty` is always true for any 're. |
Member
There was a problem hiding this comment.
Shouldn't we consider the universes of region vars?
Contributor
Collaborator
|
Queued 0be4976 with parent 5321a4f, future comparison URL. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
cc #161575. I don't think it fixes the issue per se, since the most principled fix would be to just deny unconstrained lifetime args like we do for types and consts already. At the very least, though, it Makes Things Go Faster.
In the example from #161575 (comment), each
'unconstrainedappears only once in the entire response: in the rhs of the'a: 'unconstrained#Nbound. Since they are mentioned nowhere else and are created only when proving our own nested goals, these outlives constraints are all satisfiable by setting'unconstrained := 'empty, which tells us nothing about'a. Therefore, (I think) that makes it safe to treat all of these constraints as trivial and drop them entirely, drop all of these requirements entirely, similar to what we already do with reflexive or duplicate region constraints. In other words, if aReVarappears only once in the entire response, and that place is the rhs of an outlives constraint, then it is safe to drop that constraint.I'm a little worried about the perf impact of the visitor on "normal" code, but fwiw even a 100-deep nested version of the reproducer compiles in about 0.05s on my machine.
r? lcnr