Skip to content

Commit

Permalink
Rollup merge of rust-lang#72788 - matthewjasper:projection-bound-vali…
Browse files Browse the repository at this point in the history
…dation, r=nikomatsakis

Projection bound validation

During selection we use bounds declared on associated types (e.g. `type X: Copy`) to satisfy trait/projection bounds. This would be fine so long as those bounds are checked on any impls/trait objects. For simple cases they are because the bound `Self::X: Copy` gets normalized when we check the impl.

However, for default values with specialization and higher-ranked bounds from GATs or otherwise, we can't normalize when checking the impl, and so we use the bound from the trait to prove that the bound applies to the impl, which is clearly unsound.

This PR makes 2 fixes for this:

1. Requiring that the bounds on the trait apply to a projection type with the corresponding substs, so a bound `for<'a> <Self as X<'a>>::U: Copy` on the trait cannot be used to prove `<T as X<'_>>::U: Copy`.
2. Actually checking that the bounds that we still allow apply to generic/default associated types.

Opening for a crater run.

Closes rust-lang#68641
Closes rust-lang#68642
Closes rust-lang#68643
Closes rust-lang#68644
Closes rust-lang#68645
Closes rust-lang#68656

r? @ghost
  • Loading branch information
Manishearth committed Jun 16, 2020
2 parents dd83a9b + e785472 commit 0daff91
Show file tree
Hide file tree
Showing 86 changed files with 1,554 additions and 489 deletions.
21 changes: 8 additions & 13 deletions src/librustc_infer/infer/outlives/verify.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use crate::infer::outlives::env::RegionBoundPairs;
use crate::infer::{GenericKind, VerifyBound};
use crate::traits;
use rustc_data_structures::captures::Captures;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, InternalSubsts, Subst};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst};
use rustc_middle::ty::{self, Ty, TyCtxt};

/// The `TypeOutlives` struct has the job of "lowering" a `T: 'a`
Expand Down Expand Up @@ -311,18 +310,14 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
fn region_bounds_declared_on_associated_item(
&self,
assoc_item_def_id: DefId,
) -> impl Iterator<Item = ty::Region<'tcx>> + 'cx + Captures<'tcx> {
) -> impl Iterator<Item = ty::Region<'tcx>> {
let tcx = self.tcx;
let assoc_item = tcx.associated_item(assoc_item_def_id);
let trait_def_id = assoc_item.container.assert_trait();
let trait_predicates = tcx.predicates_of(trait_def_id).predicates.iter().map(|(p, _)| *p);
let identity_substs = InternalSubsts::identity_for_item(tcx, assoc_item_def_id);
let identity_proj = tcx.mk_projection(assoc_item_def_id, identity_substs);
self.collect_outlives_from_predicate_list(
move |ty| ty == identity_proj,
traits::elaborate_predicates(tcx, trait_predicates).map(|o| o.predicate),
)
.map(|b| b.1)
let predicates = tcx.projection_predicates(assoc_item_def_id);
predicates
.into_iter()
.filter_map(|p| p.to_opt_type_outlives())
.filter_map(|p| p.no_bound_vars())
.map(|b| b.1)
}

/// Searches through a predicate list for a predicate `T: 'a`.
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_infer/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ pub use self::project::{
Normalized, NormalizedTy, ProjectionCache, ProjectionCacheEntry, ProjectionCacheKey,
ProjectionCacheStorage, Reveal,
};
crate use self::util::elaborate_predicates;

pub use rustc_middle::traits::*;

/// An `Obligation` represents some trait reference (e.g., `int: Eq`) for
Expand Down
17 changes: 17 additions & 0 deletions src/librustc_middle/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,23 @@ rustc_queries! {
cache_on_disk_if { key.is_local() }
}

/// Returns the list of predicates that can be used for
/// `SelectionCandidate::ProjectionCandidate` and
/// `ProjectionTyCandidate::TraitDef`.
/// Specifically this is the bounds (equivalent to) those
/// written on the trait's type definition, or those
/// after the `impl` keyword
///
/// type X: Bound + 'lt
/// ^^^^^^^^^^^
/// impl Debug + Display
/// ^^^^^^^^^^^^^^^
///
/// `key` is the `DefId` of the associated type or opaque type.
query projection_predicates(key: DefId) -> &'tcx ty::List<ty::Predicate<'tcx>> {
desc { |tcx| "finding projection predicates for `{}`", tcx.def_path_str(key) }
}

query native_libraries(_: CrateNum) -> Lrc<Vec<NativeLib>> {
desc { "looking up the native libraries of a linked crate" }
}
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_middle/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ impl FlagComputation {

&ty::Bound(debruijn, _) => {
self.add_binder(debruijn);
self.add_flags(TypeFlags::STILL_FURTHER_SPECIALIZABLE);
}

&ty::Placeholder(..) => {
Expand Down Expand Up @@ -216,7 +215,6 @@ impl FlagComputation {
}
ty::ConstKind::Bound(debruijn, _) => {
self.add_binder(debruijn);
self.add_flags(TypeFlags::STILL_FURTHER_SPECIALIZABLE);
}
ty::ConstKind::Param(_) => {
self.add_flags(TypeFlags::HAS_CT_PARAM);
Expand Down
3 changes: 0 additions & 3 deletions src/librustc_middle/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1583,19 +1583,16 @@ impl RegionKind {
flags = flags | TypeFlags::HAS_FREE_REGIONS;
flags = flags | TypeFlags::HAS_FREE_LOCAL_REGIONS;
flags = flags | TypeFlags::HAS_RE_INFER;
flags = flags | TypeFlags::STILL_FURTHER_SPECIALIZABLE;
}
ty::RePlaceholder(..) => {
flags = flags | TypeFlags::HAS_FREE_REGIONS;
flags = flags | TypeFlags::HAS_FREE_LOCAL_REGIONS;
flags = flags | TypeFlags::HAS_RE_PLACEHOLDER;
flags = flags | TypeFlags::STILL_FURTHER_SPECIALIZABLE;
}
ty::ReEarlyBound(..) => {
flags = flags | TypeFlags::HAS_FREE_REGIONS;
flags = flags | TypeFlags::HAS_FREE_LOCAL_REGIONS;
flags = flags | TypeFlags::HAS_RE_PARAM;
flags = flags | TypeFlags::STILL_FURTHER_SPECIALIZABLE;
}
ty::ReFree { .. } => {
flags = flags | TypeFlags::HAS_FREE_REGIONS;
Expand Down
11 changes: 11 additions & 0 deletions src/librustc_middle/ty/subst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,17 @@ impl<'a, 'tcx> InternalSubsts<'tcx> {
/// in a different item, with `target_substs` as the base for
/// the target impl/trait, with the source child-specific
/// parameters (e.g., method parameters) on top of that base.
///
/// For example given:
///
/// trait X<S> { fn f<T>(); }
/// impl<U> X<U> for U { fn f<V>() {} }
///
/// * If `self` is `[Self, S, T]`: the identity substs of `f` in the trait.
/// * If `source_ancestor` is the def_id of the trait.
/// * If `target_substs` is `[U]`, the substs for the impl.
/// * Then we will return `[U, T]`, the subst for `f` in the impl that
/// are needed for it to match the trait.
pub fn rebase_onto(
&self,
tcx: TyCtxt<'tcx>,
Expand Down
13 changes: 8 additions & 5 deletions src/librustc_resolve/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
})
.collect();
if !lifetimes.is_empty() {
self.trait_ref_hack = true;
let next_early_index = self.next_early_index();
let scope = Scope::Binder {
lifetimes,
Expand All @@ -895,9 +894,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
let result = self.with(scope, |old_scope, this| {
this.check_lifetime_params(old_scope, &bound_generic_params);
this.visit_ty(&bounded_ty);
this.trait_ref_hack = true;
walk_list!(this, visit_param_bound, bounds);
this.trait_ref_hack = false;
});
self.trait_ref_hack = false;
result
} else {
self.visit_ty(&bounded_ty);
Expand Down Expand Up @@ -932,13 +932,15 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
debug!("visit_poly_trait_ref(trait_ref={:?})", trait_ref);

let should_pop_missing_lt = self.is_trait_ref_fn_scope(trait_ref);
if !self.trait_ref_hack

let trait_ref_hack = take(&mut self.trait_ref_hack);
if !trait_ref_hack
|| trait_ref.bound_generic_params.iter().any(|param| match param.kind {
GenericParamKind::Lifetime { .. } => true,
_ => false,
})
{
if self.trait_ref_hack {
if trait_ref_hack {
struct_span_err!(
self.tcx.sess,
trait_ref.span,
Expand Down Expand Up @@ -968,10 +970,11 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
this.check_lifetime_params(old_scope, &trait_ref.bound_generic_params);
walk_list!(this, visit_generic_param, trait_ref.bound_generic_params);
this.visit_trait_ref(&trait_ref.trait_ref);
})
});
} else {
self.visit_trait_ref(&trait_ref.trait_ref);
}
self.trait_ref_hack = trait_ref_hack;
if should_pop_missing_lt {
self.missing_named_lifetime_spots.pop();
}
Expand Down
21 changes: 13 additions & 8 deletions src/librustc_trait_selection/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,9 +895,12 @@ fn assemble_candidates_from_trait_def<'cx, 'tcx>(

let tcx = selcx.tcx();
// Check whether the self-type is itself a projection.
let (def_id, substs) = match obligation_trait_ref.self_ty().kind {
ty::Projection(ref data) => (data.trait_ref(tcx).def_id, data.substs),
ty::Opaque(def_id, substs) => (def_id, substs),
// If so, extract what we know from the trait and try to come up with a good answer.
let bounds = match obligation_trait_ref.self_ty().kind {
ty::Projection(ref data) => {
tcx.projection_predicates(data.item_def_id).subst(tcx, data.substs)
}
ty::Opaque(def_id, substs) => tcx.projection_predicates(def_id).subst(tcx, substs),
ty::Infer(ty::TyVar(_)) => {
// If the self-type is an inference variable, then it MAY wind up
// being a projected type, so induce an ambiguity.
Expand All @@ -907,17 +910,13 @@ fn assemble_candidates_from_trait_def<'cx, 'tcx>(
_ => return,
};

// If so, extract what we know from the trait and try to come up with a good answer.
let trait_predicates = tcx.predicates_of(def_id);
let bounds = trait_predicates.instantiate(tcx, substs);
let bounds = elaborate_predicates(tcx, bounds.predicates.into_iter()).map(|o| o.predicate);
assemble_candidates_from_predicates(
selcx,
obligation,
obligation_trait_ref,
candidate_set,
ProjectionTyCandidate::TraitDef,
bounds,
bounds.iter(),
)
}

Expand Down Expand Up @@ -1474,6 +1473,12 @@ fn confirm_impl_candidate<'cx, 'tcx>(
);
return Progress { ty: tcx.types.err, obligations: nested };
}
// If we're trying to normalize `<Vec<u32> as X>::A<S>` using
//`impl<T> X for Vec<T> { type A<Y> = Box<Y>; }`, then:
//
// * `obligation.predicate.substs` is `[Vec<u32>, S]`
// * `substs` is `[u32]`
// * `substs` ends up as `[u32, S]`
let substs = obligation.predicate.substs.rebase_onto(tcx, trait_def_id, substs);
let substs =
translate_substs(selcx.infcx(), param_env, impl_def_id, substs, assoc_ty.defining_node);
Expand Down
50 changes: 22 additions & 28 deletions src/librustc_trait_selection/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,9 +1273,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
placeholder_trait_predicate,
);

let (def_id, substs) = match placeholder_trait_predicate.trait_ref.self_ty().kind {
ty::Projection(ref data) => (data.trait_ref(self.tcx()).def_id, data.substs),
ty::Opaque(def_id, substs) => (def_id, substs),
let tcx = self.infcx.tcx;
let predicates = match placeholder_trait_predicate.trait_ref.self_ty().kind {
ty::Projection(ref data) => {
tcx.projection_predicates(data.item_def_id).subst(tcx, data.substs)
}
ty::Opaque(def_id, substs) => tcx.projection_predicates(def_id).subst(tcx, substs),
_ => {
span_bug!(
obligation.cause.span,
Expand All @@ -1285,32 +1288,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
);
}
};
debug!(
"match_projection_obligation_against_definition_bounds: \
def_id={:?}, substs={:?}",
def_id, substs
);

let predicates_of = self.tcx().predicates_of(def_id);
let bounds = predicates_of.instantiate(self.tcx(), substs);
debug!(
"match_projection_obligation_against_definition_bounds: \
bounds={:?}",
bounds
);

let elaborated_predicates =
util::elaborate_predicates(self.tcx(), bounds.predicates.into_iter());
let matching_bound = elaborated_predicates.filter_to_traits().find(|bound| {
self.infcx.probe(|_| {
self.match_projection(
obligation,
*bound,
placeholder_trait_predicate.trait_ref,
&placeholder_map,
snapshot,
)
})
let matching_bound = predicates.iter().find_map(|bound| {
if let ty::PredicateKind::Trait(bound, _) = bound.kind() {
let bound = bound.to_poly_trait_ref();
if self.infcx.probe(|_| {
self.match_projection(
obligation,
bound,
placeholder_trait_predicate.trait_ref,
&placeholder_map,
snapshot,
)
}) {
return Some(bound);
}
}
None
});

debug!(
Expand Down
Loading

0 comments on commit 0daff91

Please sign in to comment.