Stop calling impl_polarity when impl_trait_ref was also called

This commit is contained in:
Oli Scherer 2024-02-10 22:27:03 +00:00
parent ab0e8b3145
commit b43fbe63e7
12 changed files with 96 additions and 77 deletions

View File

@ -1671,9 +1671,9 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
.is_accessible_from(self.item_def_id(), tcx)
&& tcx.all_impls(*trait_def_id)
.any(|impl_def_id| {
let trait_ref = tcx.impl_trait_ref(impl_def_id);
trait_ref.is_some_and(|trait_ref| {
let impl_ = trait_ref.instantiate(
let impl_header = tcx.impl_trait_header(impl_def_id);
impl_header.is_some_and(|header| {
let header = header.instantiate(
tcx,
infcx.fresh_args_for_item(DUMMY_SP, impl_def_id),
);
@ -1685,11 +1685,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
infcx
.can_eq(
ty::ParamEnv::empty(),
impl_.self_ty(),
header.trait_ref.self_ty(),
value,
)
) && header.polarity != ty::ImplPolarity::Negative
})
&& tcx.impl_polarity(impl_def_id) != ty::ImplPolarity::Negative
})
})
.map(|trait_def_id| tcx.def_path_str(trait_def_id))
@ -1735,13 +1734,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
} else {
// Find all the types that have an `impl` for the trait.
tcx.all_impls(trait_def_id)
.filter(|impl_def_id| {
.filter_map(|impl_def_id| tcx.impl_trait_header(impl_def_id))
.filter(|header| {
// Consider only accessible traits
tcx.visibility(trait_def_id).is_accessible_from(self.item_def_id(), tcx)
&& tcx.impl_polarity(impl_def_id) != ty::ImplPolarity::Negative
&& header.skip_binder().polarity != ty::ImplPolarity::Negative
})
.filter_map(|impl_def_id| tcx.impl_trait_ref(impl_def_id))
.map(|impl_| impl_.instantiate_identity().self_ty())
.map(|header| header.instantiate_identity().trait_ref.self_ty())
// We don't care about blanket impls.
.filter(|self_ty| !self_ty.has_non_region_param())
.map(|self_ty| tcx.erase_regions(self_ty).to_string())

View File

@ -474,8 +474,12 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
}
DefKind::Fn => {} // entirely within check_item_body
DefKind::Impl { of_trait } => {
if of_trait && let Some(impl_trait_ref) = tcx.impl_trait_ref(def_id) {
check_impl_items_against_trait(tcx, def_id, impl_trait_ref.instantiate_identity());
if of_trait && let Some(impl_trait_header) = tcx.impl_trait_header(def_id) {
check_impl_items_against_trait(
tcx,
def_id,
impl_trait_header.instantiate_identity(),
);
check_on_unimplemented(tcx, def_id);
}
}
@ -666,19 +670,19 @@ pub(super) fn check_specialization_validity<'tcx>(
fn check_impl_items_against_trait<'tcx>(
tcx: TyCtxt<'tcx>,
impl_id: LocalDefId,
impl_trait_ref: ty::TraitRef<'tcx>,
impl_trait_header: ty::ImplTraitHeader<'tcx>,
) {
// If the trait reference itself is erroneous (so the compilation is going
// to fail), skip checking the items here -- the `impl_item` table in `tcx`
// isn't populated for such impls.
if impl_trait_ref.references_error() {
if impl_trait_header.references_error() {
return;
}
let impl_item_refs = tcx.associated_item_def_ids(impl_id);
// Negative impls are not expected to have any items
match tcx.impl_polarity(impl_id) {
match impl_trait_header.polarity {
ty::ImplPolarity::Reservation | ty::ImplPolarity::Positive => {}
ty::ImplPolarity::Negative => {
if let [first_item_ref, ..] = impl_item_refs {
@ -695,7 +699,7 @@ fn check_impl_items_against_trait<'tcx>(
}
}
let trait_def = tcx.trait_def(impl_trait_ref.def_id);
let trait_def = tcx.trait_def(impl_trait_header.trait_ref.def_id);
for &impl_item in impl_item_refs {
let ty_impl_item = tcx.associated_item(impl_item);
@ -714,10 +718,10 @@ fn check_impl_items_against_trait<'tcx>(
));
}
ty::AssocKind::Fn => {
compare_impl_method(tcx, ty_impl_item, ty_trait_item, impl_trait_ref);
compare_impl_method(tcx, ty_impl_item, ty_trait_item, impl_trait_header.trait_ref);
}
ty::AssocKind::Type => {
compare_impl_ty(tcx, ty_impl_item, ty_trait_item, impl_trait_ref);
compare_impl_ty(tcx, ty_impl_item, ty_trait_item, impl_trait_header.trait_ref);
}
}
@ -737,7 +741,7 @@ fn check_impl_items_against_trait<'tcx>(
let mut must_implement_one_of: Option<&[Ident]> =
trait_def.must_implement_one_of.as_deref();
for &trait_item_id in tcx.associated_item_def_ids(impl_trait_ref.def_id) {
for &trait_item_id in tcx.associated_item_def_ids(impl_trait_header.trait_ref.def_id) {
let leaf_def = ancestors.leaf_def(tcx, trait_item_id);
let is_implemented = leaf_def
@ -815,7 +819,7 @@ fn check_impl_items_against_trait<'tcx>(
if let Some(missing_items) = must_implement_one_of {
let attr_span = tcx
.get_attr(impl_trait_ref.def_id, sym::rustc_must_implement_one_of)
.get_attr(impl_trait_header.trait_ref.def_id, sym::rustc_must_implement_one_of)
.map(|attr| attr.span);
missing_items_must_implement_one_of_err(

View File

@ -245,9 +245,9 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) -> Result<()
// won't be allowed unless there's an *explicit* implementation of `Send`
// for `T`
hir::ItemKind::Impl(impl_) => {
let is_auto = tcx
.impl_trait_ref(def_id)
.is_some_and(|trait_ref| tcx.trait_is_auto(trait_ref.skip_binder().def_id));
let header = tcx.impl_trait_header(def_id);
let is_auto = header
.is_some_and(|header| tcx.trait_is_auto(header.skip_binder().trait_ref.def_id));
let mut res = Ok(());
if let (hir::Defaultness::Default { .. }, true) = (impl_.defaultness, is_auto) {
let sp = impl_.of_trait.as_ref().map_or(item.span, |t| t.path.span);
@ -259,11 +259,12 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) -> Result<()
.emit());
}
// We match on both `ty::ImplPolarity` and `ast::ImplPolarity` just to get the `!` span.
match tcx.impl_polarity(def_id) {
ty::ImplPolarity::Positive => {
match header.map(|h| h.skip_binder().polarity) {
// `None` means this is an inherent impl
Some(ty::ImplPolarity::Positive) | None => {
res = res.and(check_impl(tcx, item, impl_.self_ty, &impl_.of_trait));
}
ty::ImplPolarity::Negative => {
Some(ty::ImplPolarity::Negative) => {
let ast::ImplPolarity::Negative(span) = impl_.polarity else {
bug!("impl_polarity query disagrees with impl's polarity in AST");
};
@ -280,7 +281,7 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) -> Result<()
.emit());
}
}
ty::ImplPolarity::Reservation => {
Some(ty::ImplPolarity::Reservation) => {
// FIXME: what amount of WF checking do we need for reservation impls?
}
}

View File

@ -3134,12 +3134,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if self
.tcx
.all_impls(candidate.def_id)
.filter(|imp_did| {
self.tcx.impl_polarity(*imp_did) == ty::ImplPolarity::Negative
.map(|imp_did| {
self.tcx.impl_trait_header(imp_did).expect(
"inherent impls can't be candidates, only trait impls can be",
)
})
.any(|imp_did| {
let imp =
self.tcx.impl_trait_ref(imp_did).unwrap().instantiate_identity();
.filter(|header| {
header.skip_binder().polarity == ty::ImplPolarity::Negative
})
.any(|header| {
let imp = header.instantiate_identity().trait_ref;
let imp_simp =
simplify_type(self.tcx, imp.self_ty(), TreatParams::ForLookup);
imp_simp.is_some_and(|s| s == simp_rcvr_ty)

View File

@ -1590,15 +1590,16 @@ impl<'tcx> TyCtxt<'tcx> {
def_id1: DefId,
def_id2: DefId,
) -> Option<ImplOverlapKind> {
let impl_trait_ref1 = self.impl_trait_ref(def_id1).unwrap().instantiate_identity();
let impl_trait_ref2 = self.impl_trait_ref(def_id2).unwrap().instantiate_identity();
let impl1 = self.impl_trait_header(def_id1).unwrap().instantiate_identity();
let impl2 = self.impl_trait_header(def_id2).unwrap().instantiate_identity();
// If either trait impl references an error, they're allowed to overlap,
// as one of them essentially doesn't exist.
if impl_trait_ref1.references_error() || impl_trait_ref2.references_error() {
if impl1.references_error() || impl2.references_error() {
return Some(ImplOverlapKind::Permitted { marker: false });
}
match (self.impl_polarity(def_id1), self.impl_polarity(def_id2)) {
match (impl1.polarity, impl2.polarity) {
(ImplPolarity::Reservation, _) | (_, ImplPolarity::Reservation) => {
// `#[rustc_reservation_impl]` impls don't overlap with anything
return Some(ImplOverlapKind::Permitted { marker: false });
@ -1615,7 +1616,7 @@ impl<'tcx> TyCtxt<'tcx> {
let is_marker_overlap = {
let is_marker_impl =
|trait_ref: TraitRef<'_>| -> bool { self.trait_def(trait_ref.def_id).is_marker };
is_marker_impl(impl_trait_ref1) && is_marker_impl(impl_trait_ref2)
is_marker_impl(impl1.trait_ref) && is_marker_impl(impl2.trait_ref)
};
if is_marker_overlap {

View File

@ -1347,8 +1347,11 @@ fn create_mono_items_for_default_impls<'tcx>(
item: hir::ItemId,
output: &mut MonoItems<'tcx>,
) {
let polarity = tcx.impl_polarity(item.owner_id);
if matches!(polarity, ty::ImplPolarity::Negative) {
let Some(impl_) = tcx.impl_trait_header(item.owner_id) else {
return;
};
if matches!(impl_.skip_binder().polarity, ty::ImplPolarity::Negative) {
return;
}
@ -1356,10 +1359,6 @@ fn create_mono_items_for_default_impls<'tcx>(
return;
}
let Some(trait_ref) = tcx.impl_trait_ref(item.owner_id) else {
return;
};
// Lifetimes never affect trait selection, so we are allowed to eagerly
// instantiate an instance of an impl method if the impl (and method,
// which we check below) is only parameterized over lifetime. In that case,
@ -1376,7 +1375,7 @@ fn create_mono_items_for_default_impls<'tcx>(
}
};
let impl_args = GenericArgs::for_item(tcx, item.owner_id.to_def_id(), only_region_params);
let trait_ref = trait_ref.instantiate(tcx, impl_args);
let trait_ref = impl_.instantiate(tcx, impl_args).trait_ref;
// Unlike 'lazy' monomorphization that begins by collecting items transitively
// called by `main` or other global items, when eagerly monomorphizing impl

View File

@ -39,15 +39,18 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
) -> Result<Candidate<'tcx>, NoSolution> {
let tcx = ecx.tcx();
let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
let impl_trait_header = tcx.impl_trait_header(impl_def_id).unwrap();
let drcx = DeepRejectCtxt { treat_obligation_params: TreatParams::ForLookup };
if !drcx.args_may_unify(goal.predicate.trait_ref.args, impl_trait_ref.skip_binder().args) {
if !drcx.args_may_unify(
goal.predicate.trait_ref.args,
impl_trait_header.skip_binder().trait_ref.args,
) {
return Err(NoSolution);
}
let impl_polarity = tcx.impl_polarity(impl_def_id);
// An upper bound of the certainty of this goal, used to lower the certainty
// of reservation impl to ambiguous during coherence.
let impl_polarity = impl_trait_header.skip_binder().polarity;
let maximal_certainty = match impl_polarity {
ty::ImplPolarity::Positive | ty::ImplPolarity::Negative => {
match impl_polarity == goal.predicate.polarity {
@ -63,7 +66,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
ecx.probe_trait_candidate(CandidateSource::Impl(impl_def_id)).enter(|ecx| {
let impl_args = ecx.fresh_args_for_item(impl_def_id);
let impl_trait_ref = impl_trait_ref.instantiate(tcx, impl_args);
let impl_trait_ref = impl_trait_header.instantiate(tcx, impl_args).trait_ref;
ecx.eq(goal.param_env, goal.predicate.trait_ref, impl_trait_ref)?;
let where_clause_bounds = tcx

View File

@ -1980,13 +1980,13 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
.tcx
.all_impls(trait_pred.def_id())
.filter_map(|def_id| {
if self.tcx.impl_polarity(def_id) == ty::ImplPolarity::Negative
let imp = self.tcx.impl_trait_header(def_id).unwrap().skip_binder();
if imp.polarity == ty::ImplPolarity::Negative
|| !self.tcx.is_user_visible_dep(def_id.krate)
{
return None;
}
let imp = self.tcx.impl_trait_ref(def_id).unwrap().skip_binder();
let imp = imp.trait_ref;
self.fuzzy_match_tys(trait_pred.skip_binder().self_ty(), imp.self_ty(), false).map(
|similarity| ImplCandidate { trait_ref: imp, similarity, impl_def_id: def_id },
@ -2165,12 +2165,13 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
.tcx
.all_impls(def_id)
// Ignore automatically derived impls and `!Trait` impls.
.filter(|&def_id| {
self.tcx.impl_polarity(def_id) != ty::ImplPolarity::Negative
.filter_map(|def_id| self.tcx.impl_trait_header(def_id))
.map(ty::EarlyBinder::instantiate_identity)
.filter(|header| {
header.polarity != ty::ImplPolarity::Negative
|| self.tcx.is_automatically_derived(def_id)
})
.filter_map(|def_id| self.tcx.impl_trait_ref(def_id))
.map(ty::EarlyBinder::instantiate_identity)
.map(|header| header.trait_ref)
.filter(|trait_ref| {
let self_ty = trait_ref.self_ty();
// Avoid mentioning type parameters.

View File

@ -560,20 +560,22 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// Before we create the substitutions and everything, first
// consider a "quick reject". This avoids creating more types
// and so forth that we need to.
let impl_trait_ref = self.tcx().impl_trait_ref(impl_def_id).unwrap();
if !drcx.args_may_unify(obligation_args, impl_trait_ref.skip_binder().args) {
let impl_trait_header = self.tcx().impl_trait_header(impl_def_id).unwrap();
if !drcx
.args_may_unify(obligation_args, impl_trait_header.skip_binder().trait_ref.args)
{
return;
}
if self.reject_fn_ptr_impls(
impl_def_id,
obligation,
impl_trait_ref.skip_binder().self_ty(),
impl_trait_header.skip_binder().trait_ref.self_ty(),
) {
return;
}
self.infcx.probe(|_| {
if let Ok(_args) = self.match_impl(impl_def_id, impl_trait_ref, obligation) {
if let Ok(_args) = self.match_impl(impl_def_id, impl_trait_header, obligation) {
candidates.vec.push(ImplCandidate(impl_def_id));
}
});

View File

@ -2404,8 +2404,8 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
impl_def_id: DefId,
obligation: &PolyTraitObligation<'tcx>,
) -> Normalized<'tcx, GenericArgsRef<'tcx>> {
let impl_trait_ref = self.tcx().impl_trait_ref(impl_def_id).unwrap();
match self.match_impl(impl_def_id, impl_trait_ref, obligation) {
let impl_trait_header = self.tcx().impl_trait_header(impl_def_id).unwrap();
match self.match_impl(impl_def_id, impl_trait_header, obligation) {
Ok(args) => args,
Err(()) => {
// FIXME: A rematch may fail when a candidate cache hit occurs
@ -2438,7 +2438,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
fn match_impl(
&mut self,
impl_def_id: DefId,
impl_trait_ref: EarlyBinder<ty::TraitRef<'tcx>>,
impl_trait_header: EarlyBinder<ty::ImplTraitHeader<'tcx>>,
obligation: &PolyTraitObligation<'tcx>,
) -> Result<Normalized<'tcx, GenericArgsRef<'tcx>>, ()> {
let placeholder_obligation =
@ -2447,12 +2447,12 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
let impl_args = self.infcx.fresh_args_for_item(obligation.cause.span, impl_def_id);
let impl_trait_ref = impl_trait_ref.instantiate(self.tcx(), impl_args);
if impl_trait_ref.references_error() {
let impl_trait_header = impl_trait_header.instantiate(self.tcx(), impl_args);
if impl_trait_header.references_error() {
return Err(());
}
debug!(?impl_trait_ref);
debug!(?impl_trait_header);
let Normalized { value: impl_trait_ref, obligations: mut nested_obligations } =
ensure_sufficient_stack(|| {
@ -2461,7 +2461,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth + 1,
impl_trait_ref,
impl_trait_header.trait_ref,
)
});
@ -2482,9 +2482,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
})?;
nested_obligations.extend(obligations);
if !self.is_intercrate()
&& self.tcx().impl_polarity(impl_def_id) == ty::ImplPolarity::Reservation
{
if !self.is_intercrate() && impl_trait_header.polarity == ty::ImplPolarity::Reservation {
debug!("reservation impls only apply in intercrate mode");
return Err(());
}

View File

@ -168,6 +168,8 @@ pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId,
}
}
let impl1_trait_header = tcx.impl_trait_header(impl1_def_id).unwrap().instantiate_identity();
// We determine whether there's a subset relationship by:
//
// - replacing bound vars with placeholders in impl1,
@ -181,21 +183,25 @@ pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId,
// See RFC 1210 for more details and justification.
// Currently we do not allow e.g., a negative impl to specialize a positive one
if tcx.impl_polarity(impl1_def_id) != tcx.impl_polarity(impl2_def_id) {
if impl1_trait_header.polarity != tcx.impl_polarity(impl2_def_id) {
return false;
}
// create a parameter environment corresponding to a (placeholder) instantiation of impl1
let penv = tcx.param_env(impl1_def_id);
let impl1_trait_ref = tcx.impl_trait_ref(impl1_def_id).unwrap().instantiate_identity();
// Create an infcx, taking the predicates of impl1 as assumptions:
let infcx = tcx.infer_ctxt().build();
// Attempt to prove that impl2 applies, given all of the above.
fulfill_implication(&infcx, penv, impl1_trait_ref, impl1_def_id, impl2_def_id, |_, _| {
ObligationCause::dummy()
})
fulfill_implication(
&infcx,
penv,
impl1_trait_header.trait_ref,
impl1_def_id,
impl2_def_id,
|_, _| ObligationCause::dummy(),
)
.is_ok()
}

View File

@ -257,14 +257,15 @@ fn param_env_reveal_all_normalized(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamE
fn issue33140_self_ty(tcx: TyCtxt<'_>, def_id: DefId) -> Option<EarlyBinder<Ty<'_>>> {
debug!("issue33140_self_ty({:?})", def_id);
let trait_ref = tcx
.impl_trait_ref(def_id)
let impl_ = tcx
.impl_trait_header(def_id)
.unwrap_or_else(|| bug!("issue33140_self_ty called on inherent impl {:?}", def_id))
.skip_binder();
let trait_ref = impl_.trait_ref;
debug!("issue33140_self_ty({:?}), trait-ref={:?}", def_id, trait_ref);
let is_marker_like = tcx.impl_polarity(def_id) == ty::ImplPolarity::Positive
let is_marker_like = impl_.polarity == ty::ImplPolarity::Positive
&& tcx.associated_item_def_ids(trait_ref.def_id).is_empty();
// Check whether these impls would be ok for a marker trait.