Skip to content

abby: always store nextgen region constraints in canonical form - #161306

Open
BoxyUwU wants to merge 8 commits into
rust-lang:mainfrom
BoxyUwU:abby_canonical_form_always
Open

abby: always store nextgen region constraints in canonical form#161306
BoxyUwU wants to merge 8 commits into
rust-lang:mainfrom
BoxyUwU:abby_canonical_form_always

Conversation

@BoxyUwU

@BoxyUwU BoxyUwU commented Aug 18, 2026

Copy link
Copy Markdown
Member

View all comments

title. introduce an And/Or/LeafConstraint/ types to reason about the structure of our region constraints. Never produce arbitrarily nested or/ands and always have constraints in an evaluated form.

I kinda mucked up this PR and accidentally did two things at the same time. Not only do we immediately put everything into canonical form, we also change what it means for a region constraint to be in canonical form. Whoops :>

Rough overview of what a RegionConstraint is:

  • RegionConstraint contains two things: an AND of LEAFs and an OR of AND of LEAFs. Another way of thinking about it would be to say its an AND consisting of arbitrarily many LEAFs and a single OR of AND of LEAFs
  • There are never any region constraints shared between all ANDs of the OR, instead they're moved into the top level AND
  • If the OR constraint is false then we wipe the top level AND as it doesn't matter what they are, the constraint is always going to be false
  • ORs never have two equivalent ANDs within them. Similarly, ANDs never have two equivalent LEAFs within them

this simplifies a lot of things conceptually as we now no longer need to worry about what state our region constraints are in. and our algorithms also don't need to handle arbitrary nesting of ors/ands :) and its a lot easier to read the debug logs 😅

I also wound up needing to do this while trying to compile std/core with -Zassumptions-on-binders as we would otherwise OOM from having both:

  • Lots of duplicate region constraints (e.g. And('a: 'b, 'a: 'b))
  • Lots of region constraints shared across all elements of an OR (e.g. Or(And('a: 'b, 'b: 'c), And('a: 'b, 'b: 'd)))

Some future work:

  • Remove RegionConstraint::splatted_and_constraints it's kind of weird to even need it and probably encourages bad-for-perf patterns
  • we Probably want some kind of fast path for pushing new leaf constraints to the region constraint storage. slash have a way to register a leaf constraint directly rather than having to make a RegionConstraint. Perf stuff :3

In theory this PR should mostly not have functional changes. In practice it might affect some things due to changing the exact repr of things affecting query responses. There's probably also some behaviour differences here due to us falling on our face more or less in WIP parts of abby now that we have different region constraints. I don't think any of this should be meaningful though. This PR is intended to not fundamentally change the abby algorithm :3

This PR should be reviewed commit-by-commit. There are a bunch of commits restructuring existing logic to assume their input is in canonical form as it will be by the end of the PR.

Then there's the core change in always canonical form which actually replaces RegionConstraint with all the new types and updates all the locations using them.

Finally there's propagate ambiguity not evaluate which deals with the leftover evaluate_solver_constraint which was mostly unnecessary now due to moving its main logic into construction of RegionConstraint and friends. I didn't want to make actual bug fixes in this PR so I just left some FIXMEs about some of the issues that propagate_ambiguity has instead of fixing them here.

Fixes rust-lang/project-assumptions-on-binders#14

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 18, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch 5 times, most recently from d860264 to c64bf00 Compare August 21, 2026 13:36
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch from c64bf00 to 57b2d5e Compare August 21, 2026 14:12
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch from 57b2d5e to c17800a Compare August 21, 2026 14:38
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch 2 times, most recently from aba4012 to 56320f9 Compare August 21, 2026 14:52
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch 2 times, most recently from 5bedfe3 to 65f3b73 Compare August 21, 2026 15:07
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch from a10cd16 to 930ca67 Compare August 21, 2026 16:15
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU

BoxyUwU commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

r? @lcnr @khyperia

@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
BoxyUwU marked this pull request as ready for review August 21, 2026 16:42
@rust-log-analyzer

This comment has been minimized.

Comment thread compiler/rustc_type_ir/src/region_constraint.rs Outdated
}

new_constraints
RegionConstraint::new_from_or(

@lcnr lcnr Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also horrible for perf xd, pls FIXME

View changes since the review

@lcnr lcnr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch 6 times, most recently from 989777a to 4eebcea Compare September 2, 2026 12:26
@BoxyUwU
BoxyUwU force-pushed the abby_canonical_form_always branch from 4eebcea to b22bffa Compare September 2, 2026 12:38
@rustbot

rustbot commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

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.

@BoxyUwU

BoxyUwU commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

@bors r+ rollup=never

@rust-bors

rust-bors Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📌 Commit b22bffa has been approved by BoxyUwU

It is now in the queue for this repository.

@rust-bors rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 2, 2026
@BoxyUwU

BoxyUwU commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

@bors r=lcnr,khyperia

@rust-bors rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2026
@rust-bors

rust-bors Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📌 Commit b22bffa has been approved by lcnr,khyperia

It is now in the queue for this repository.

@rust-bors

rust-bors Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⌛ Testing commit b22bffa with merge d8df826...

Workflow: https://github.com/rust-lang/rust/actions/runs/33744111368

rust-bors Bot pushed a commit that referenced this pull request Sep 3, 2026
…yperia

abby: always store nextgen region constraints in canonical form



title. introduce an `And`/`Or`/`LeafConstraint`/ types to reason about the structure of our region constraints. Never produce arbitrarily nested or/ands and always have constraints in an evaluated form.

I kinda mucked up this PR and accidentally did two things at the same time. Not only do we immediately put everything into canonical form, we also change what it means for a region constraint to be in canonical form. Whoops :>

Rough overview of what a `RegionConstraint` is:
- `RegionConstraint` contains two things: an `AND of LEAFs` and an `OR of AND of LEAFs`. Another way of thinking about it would be to say its an `AND` consisting of arbitrarily many LEAFs and a single `OR of AND of LEAFs`
- There are never any region constraints shared between all ANDs of the OR, instead they're moved into the top level AND
- If the OR constraint is `false` then we wipe the top level `AND` as it doesn't matter what they are, the constraint is always going to be false
- ORs never have two equivalent ANDs within them. Similarly, ANDs never have two equivalent LEAFs within them

this simplifies a lot of things conceptually as we now no longer need to worry about what state our region constraints are in. and our algorithms also don't need to handle arbitrary nesting of ors/ands :) and its a lot easier to read the debug logs 😅 

I also wound up needing to do this while trying to compile `std`/`core` with `-Zassumptions-on-binders` as we would otherwise OOM from having both:
- Lots of duplicate region constraints (e.g. `And('a: 'b, 'a: 'b)`)
- Lots of region constraints shared across all elements of an `OR` (e.g. `Or(And('a: 'b, 'b: 'c), And('a: 'b, 'b: 'd))`)

Some future work:
- Remove `RegionConstraint::splatted_and_constraints` it's kind of weird to even need it and probably encourages bad-for-perf patterns
- we Probably want some kind of fast path for pushing new leaf constraints to the region constraint storage. slash have a way to register a leaf constraint directly rather than having to make a `RegionConstraint`. Perf stuff :3

In theory this PR should mostly not have functional changes. In practice it might affect some things due to changing the exact repr of things affecting query responses. There's probably also some behaviour differences here due to us falling on our face more or less in WIP parts of abby now that we have different region constraints. I don't think any of this should be meaningful though. This PR is intended to not fundamentally change the abby algorithm :3

This PR should be reviewed commit-by-commit. There are a bunch of commits restructuring existing logic to assume their input is in canonical form as it will be by the end of the PR.

Then there's the core change in `always canonical form` which actually replaces `RegionConstraint` with all the new types and updates all the locations using them.

Finally there's `propagate ambiguity not evaluate` which deals with the leftover `evaluate_solver_constraint` which was mostly unnecessary now due to moving its main logic into construction of `RegionConstraint` and friends. I didn't want to make actual bug fixes in this PR so I just left some FIXMEs about some of the issues that `propagate_ambiguity` has instead of fixing them here.

Fixes rust-lang/project-assumptions-on-binders#14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Work Item]: Only have canonical form region constraints

5 participants