Skip to content

Commit

Permalink
constrained_type_params: make projections depend on their trait-ref
Browse files Browse the repository at this point in the history
As this is a soundness fix, it is a [breaking-change].

Fixes #29861.
  • Loading branch information
arielb1 committed Nov 19, 2015
1 parent 3c68f64 commit 3c0d55c
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 19 deletions.
10 changes: 5 additions & 5 deletions src/librustc_typeck/collect.rs
Expand Up @@ -2389,9 +2389,9 @@ fn enforce_impl_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
// reachable from there, to start (if this is an inherent impl,
// then just examine the self type).
let mut input_parameters: HashSet<_> =
ctp::parameters_for_type(impl_scheme.ty).into_iter().collect();
ctp::parameters_for_type(impl_scheme.ty, false).into_iter().collect();
if let Some(ref trait_ref) = impl_trait_ref {
input_parameters.extend(ctp::parameters_for_trait_ref(trait_ref));
input_parameters.extend(ctp::parameters_for_trait_ref(trait_ref, false));
}

ctp::setup_constraining_predicates(tcx,
Expand Down Expand Up @@ -2420,9 +2420,9 @@ fn enforce_impl_lifetimes_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
let impl_trait_ref = tcx.impl_trait_ref(impl_def_id);

let mut input_parameters: HashSet<_> =
ctp::parameters_for_type(impl_scheme.ty).into_iter().collect();
ctp::parameters_for_type(impl_scheme.ty, false).into_iter().collect();
if let Some(ref trait_ref) = impl_trait_ref {
input_parameters.extend(ctp::parameters_for_trait_ref(trait_ref));
input_parameters.extend(ctp::parameters_for_trait_ref(trait_ref, false));
}
ctp::identify_constrained_type_params(tcx,
&impl_predicates.predicates.as_slice(), impl_trait_ref, &mut input_parameters);
Expand All @@ -2434,7 +2434,7 @@ fn enforce_impl_lifetimes_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
ty::TypeTraitItem(ref assoc_ty) => assoc_ty.ty,
ty::ConstTraitItem(..) | ty::MethodTraitItem(..) => None
})
.flat_map(|ty| ctp::parameters_for_type(ty))
.flat_map(|ty| ctp::parameters_for_type(ty, true))
.filter_map(|p| match p {
ctp::Parameter::Type(_) => None,
ctp::Parameter::Region(r) => Some(r),
Expand Down
64 changes: 50 additions & 14 deletions src/librustc_typeck/constrained_type_params.rs
Expand Up @@ -19,15 +19,20 @@ pub enum Parameter {
Region(ty::EarlyBoundRegion),
}

/// Returns the list of parameters that are constrained by the type `ty`
/// - i.e. the value of each parameter in the list is uniquely determined
/// by `ty` (see RFC 447).
pub fn parameters_for_type<'tcx>(ty: Ty<'tcx>) -> Vec<Parameter> {
/// If `include_projections` is false, returns the list of parameters that are
/// constrained by the type `ty` - i.e. the value of each parameter in the list is
/// uniquely determined by `ty` (see RFC 447). If it is true, return the list
/// of parameters whose values are needed in order to constrain `ty` - these
/// differ, with the latter being a superset, in the presence of projections.
pub fn parameters_for_type<'tcx>(ty: Ty<'tcx>,
include_projections: bool) -> Vec<Parameter> {
let mut result = vec![];
ty.maybe_walk(|t| {
if let ty::TyProjection(..) = t.sty {
ty.maybe_walk(|t| match t.sty {
ty::TyProjection(..) if !include_projections => {

false // projections are not injective.
} else {
}
_ => {
result.append(&mut parameters_for_type_shallow(t));
// non-projection type constructors are injective.
true
Expand All @@ -36,13 +41,16 @@ pub fn parameters_for_type<'tcx>(ty: Ty<'tcx>) -> Vec<Parameter> {
result
}

pub fn parameters_for_trait_ref<'tcx>(trait_ref: &ty::TraitRef<'tcx>) -> Vec<Parameter> {
pub fn parameters_for_trait_ref<'tcx>(trait_ref: &ty::TraitRef<'tcx>,
include_projections: bool) -> Vec<Parameter> {
let mut region_parameters =
parameters_for_regions_in_substs(&trait_ref.substs);

let type_parameters =
trait_ref.substs.types.iter()
.flat_map(|ty| parameters_for_type(ty));
trait_ref.substs
.types
.iter()
.flat_map(|ty| parameters_for_type(ty, include_projections));

region_parameters.extend(type_parameters);

Expand All @@ -60,8 +68,14 @@ fn parameters_for_type_shallow<'tcx>(ty: Ty<'tcx>) -> Vec<Parameter> {
parameters_for_regions_in_substs(substs),
ty::TyTrait(ref data) =>
parameters_for_regions_in_substs(&data.principal.skip_binder().substs),
_ =>
vec![],
ty::TyProjection(ref pi) =>
parameters_for_regions_in_substs(&pi.trait_ref.substs),
ty::TyBool | ty::TyChar | ty::TyInt(..) | ty::TyUint(..) |
ty::TyFloat(..) | ty::TyBox(..) | ty::TyStr |
ty::TyArray(..) | ty::TySlice(..) | ty::TyBareFn(..) |
ty::TyTuple(..) | ty::TyRawPtr(..) |
ty::TyInfer(..) | ty::TyClosure(..) | ty::TyError =>
vec![]
}
}

Expand Down Expand Up @@ -113,6 +127,22 @@ pub fn identify_constrained_type_params<'tcx>(_tcx: &ty::ctxt<'tcx>,
/// pass, we want the projection to come first. In fact, as projections
/// can (acyclically) depend on one another - see RFC447 for details - we
/// need to topologically sort them.
///
/// We *do* have to be somewhat careful when projection targets contain
/// projections themselves, for example in
/// impl<S,U,V,W> Trait for U where
/// /* 0 */ S: Iterator<Item=U>,
/// /* - */ U: Iterator,
/// /* 1 */ <U as Iterator>::Item: ToOwned<Owned=(W,<V as Iterator>::Item)>
/// /* 2 */ W: Iterator<Item=V>
/// /* 3 */ V: Debug
/// we have to evaluate the projections in the order I wrote them:
/// `V: Debug` requires `V` to be evaluated. The only projection that
/// *determines* `V` is 2 (1 contains it, but *does not determine it*,
/// as it is only contained within a projection), but that requires `W`
/// which is determined by 1, which requires `U`, that is determined
/// by 0. I should probably pick a less tangled example, but I can't
/// think of any.
pub fn setup_constraining_predicates<'tcx>(_tcx: &ty::ctxt<'tcx>,
predicates: &mut [ty::Predicate<'tcx>],
impl_trait_ref: Option<ty::TraitRef<'tcx>>,
Expand Down Expand Up @@ -157,12 +187,18 @@ pub fn setup_constraining_predicates<'tcx>(_tcx: &ty::ctxt<'tcx>,
continue;
}

let inputs = parameters_for_trait_ref(&projection.projection_ty.trait_ref);
// A projection depends on its input types and determines its output
// type. For example, if we have
// `<<T as Bar>::Baz as Iterator>::Output = <U as Iterator>::Output`
// Then the projection only applies if `T` is known, but it still
// does not determine `U`.

let inputs = parameters_for_trait_ref(&projection.projection_ty.trait_ref, true);
let relies_only_on_inputs = inputs.iter().all(|p| input_parameters.contains(&p));
if !relies_only_on_inputs {
continue;
}
input_parameters.extend(parameters_for_type(projection.ty));
input_parameters.extend(parameters_for_type(projection.ty, false));
} else {
continue;
}
Expand Down
30 changes: 30 additions & 0 deletions src/test/compile-fail/issue-29861.rs
@@ -0,0 +1,30 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub trait MakeRef<'a> {
type Ref;
}
impl<'a, T: 'a> MakeRef<'a> for T {
type Ref = &'a T;
}

pub trait MakeRef2 {
type Ref2;
}
impl<'a, T: 'a> MakeRef2 for T {
//~^ ERROR the lifetime parameter `'a` is not constrained
type Ref2 = <T as MakeRef<'a>>::Ref;
}

fn foo() -> <String as MakeRef2>::Ref2 { &String::from("foo") }

fn main() {
println!("{}", foo());
}

0 comments on commit 3c0d55c

Please sign in to comment.