-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
clean up unneeded ToPredicate
impls
#115566
clean up unneeded ToPredicate
impls
#115566
Conversation
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
let pred: ty::Predicate<'_> = ty::Binder::bind_with_vars( | ||
ty::ProjectionPredicate { | ||
projection_ty: tcx.mk_alias_ty(trait_ty.def_id, rebased_args), | ||
term: normalize_impl_ty.into(), | ||
}, | ||
bound_vars, | ||
) | ||
.to_predicate(tcx), | ||
), | ||
.to_predicate(tcx); | ||
|
||
predicates.push(pred.expect_clause()) |
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 a bit roundabout. We should probably add a way to create clauses directly
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.
yea, considering all the uses work like that, adding a new
method to Clause
seems best.
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 suppose it's something like this?
impl Clause {
fn new(tcx: TyCtxt<'tcx>, pred: impl ToPredicate<'tcx>) -> Clause<'tcx> {
// ...
}
}
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.
just a ClauseKind
as the argument. Even if internally you go through ToPredicate
, there's nothing that can fail if the argument is ClauseKind
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.
Ohhh. nice.
d263dc3
to
b62141e
Compare
Just realized I forgot to push the commit. Can't use |
This comment has been minimized.
This comment has been minimized.
b62141e
to
3c69a10
Compare
@bors r+ rollup |
…llaumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#115566 (clean up unneeded `ToPredicate` impls) - rust-lang#115962 (coverage: Remove debug code from the instrumentor) - rust-lang#115988 (rustdoc: add test cases, and fix, search tabs layout jank) - rust-lang#115991 (Ensure `build/tmp` exists in `rustdoc_themes::get_themes`) - rust-lang#115997 (RELEASES.md: Add missing patch releases) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#115566 - zirconium-n:issue-107250-clean-up-unused-to-predicate, r=oli-obk clean up unneeded `ToPredicate` impls Part of rust-lang#107250. Removed all totally unused impls. And inlined two impls not need to satisify trait bound. r? `@oli-obk`
Part of #107250.
Removed all totally unused impls. And inlined two impls not need to satisify trait bound.
r? @oli-obk