-
Notifications
You must be signed in to change notification settings - Fork 1.8k
internal: Migrate more predicate things to next-solver #20717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
// 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 Binder
s - 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.
EarlyBinder
s 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.
There was a problem hiding this comment.
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 EarlyBinder
GenericPredicates
. 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.
There was a problem hiding this 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.
b6bbbdf
to
196bd25
Compare
196bd25
to
0a5457a
Compare
Originally started this to migrate
TraitEnvironment
to next-solver but it needs few more touches 😅 (Will do it after this)