-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Migrate more things in the new solver to specific DefId
s
#146111
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,7 +123,7 @@ where | |
fn consider_impl_candidate( | ||
ecx: &mut EvalCtxt<'_, D>, | ||
goal: Goal<I, Self>, | ||
impl_def_id: I::DefId, | ||
impl_def_id: I::ImplId, | ||
) -> Result<Candidate<I>, NoSolution> { | ||
let cx = ecx.cx(); | ||
|
||
|
@@ -152,20 +152,20 @@ where | |
} | ||
|
||
ecx.probe_trait_candidate(CandidateSource::Impl(impl_def_id)).enter(|ecx| { | ||
let impl_args = ecx.fresh_args_for_item(impl_def_id); | ||
let impl_args = ecx.fresh_args_for_item(impl_def_id.into()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. accept There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a specific reason to want that? This will make the method generic, meaning monomorphization costs, and the |
||
ecx.record_impl_args(impl_args); | ||
let impl_trait_ref = impl_trait_ref.instantiate(cx, impl_args); | ||
|
||
ecx.eq(goal.param_env, goal.predicate.trait_ref, impl_trait_ref)?; | ||
let where_clause_bounds = cx | ||
.predicates_of(impl_def_id) | ||
.predicates_of(impl_def_id.into()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
.iter_instantiated(cx, impl_args) | ||
.map(|pred| goal.with(cx, pred)); | ||
ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds); | ||
|
||
// For this impl to be `const`, we need to check its `[const]` bounds too. | ||
let const_conditions = cx | ||
.const_conditions(impl_def_id) | ||
.const_conditions(impl_def_id.into()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and same |
||
.iter_instantiated(cx, impl_args) | ||
.map(|bound_trait_ref| { | ||
goal.with( | ||
|
@@ -240,7 +240,7 @@ where | |
ty::TraitRef::new(cx, cx.require_trait_lang_item(SolverTraitLangItem::Sized), [output]) | ||
}); | ||
let requirements = cx | ||
.const_conditions(def_id) | ||
.const_conditions(def_id.into()) | ||
.iter_instantiated(cx, args) | ||
.map(|trait_ref| { | ||
( | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
This is the only place in the PR I think there is a (slight) regression in code quality. The reason is that now the types of the variables are different. It's possible to avoid if we unify
ClosureId
,CoroutineId
andCoroutineClosureId
. This will be slightly worse for r-a and slightly better here. Given that the code duplicated is very small, and for r-a the effects of unifying them will be bigger, I decided to duplicate the code here, but I agree to reverse my decision if the reviewer disagrees.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.
Could you do
I don't remember if this got implemented - being able to write guards for each pattern. (Even if it is, maybe you need a turbofish here, idk).
You could also remove the duplication by moving this into a closure and then calling
check(did.into())
for each armThere 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.
Doesn't seem to work: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=33963a324a2ffe51340b0ca533c710ef.
I can extract to a closure if wanted, I just didn't because the duplicated code is really small.
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.
Yeah, so
guard_patterns
seems implemented? But doesn't supportif let
. Not sure if that's planned or not - sure seems possibly useful.In this case, duplicating is probably fine, but again would defer to lcnr.