E530 internal: Migrate more predicate things to next-solver by ShoyuVanilla · Pull Request #20717 · rust-lang/rust-analyzer · GitHub
[go: up one dir, main page]

Skip to content

Conversation

ShoyuVanilla
Copy link
Member
@ShoyuVanilla ShoyuVanilla commented Sep 21, 2025

Originally started this to migrate TraitEnvironment to next-solver but it needs few more touches 😅 (Will do it after this)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 21, 2025
// Skipping the inner binders is ok, as we don't handle quantified where
// clauses yet.
.into_value_and_skipped_binders();
stdx::always!(b.len(Interner) == 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

dumb question, but should we keep this assert around in some form?

Copy link
Member Author
@ShoyuVanilla ShoyuVanilla Sep 21, 2025

Choose a reason for hiding this comment

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

I think we wouldn't need it here. db.generic_predicates(..) returns chalk_ir's Binder and this assertion checks whether there exists any HRTB among such predicates, as Binders - both chalk's and rustc's - may contain such things.
But the migrated db.generic_predicates_ns(..) returns EarlyBinder, which doesn't contain any HRTBs because EarlyBinder primarily exists to remind that we have to instantiate it before using it.
EarlyBinders would be more appropriate in this context but unfortunately chalk_ir doesn't have EarlyBinder so I guess because of that we currently have such assertion here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I was a bit wrong. db.generic_predicates_ns(..) returns EarlyBinderGenericPredicates. But anyway, it's not a Binder so don't need to keep the assertion and anyway they are bound into EarlyBinder and then instantiated to be used both in our codebase and rustc.

Copy link
Contributor
@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

LGTM, just two nits.

FF8
@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue Sep 22, 2025
Merged via the queue into rust-lang:master with commit 7d9af9f Sep 22, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2025
@ShoyuVanilla ShoyuVanilla deleted the migrate-more branch September 22, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0