Skip to content

Commit

Permalink
Auto merge of #35143 - arielb1:rfc447-regions, r=eddyb
Browse files Browse the repository at this point in the history
typeck: use a TypeVisitor in ctp

Use a TypeVisitor in ctp instead of `ty::walk`

This fixes a few cases where a region could be projected out of a trait while not being constrained by the type parameters, violating rust-lang/rfcs#447 and breaking soundness. As such, this is a [breaking-change].

Fixes #35139

r? @eddyb
  • Loading branch information
bors authored Jul 31, 2016
2 parents 2b87f03 + 0a128f3 commit 7333c4a
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 71 deletions.
10 changes: 5 additions & 5 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2237,9 +2237,9 @@ fn enforce_impl_params_are_constrained<'a, 'tcx>(ccx: &CrateCtxt<'a, '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, false).into_iter().collect();
ctp::parameters_for(&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, false));
input_parameters.extend(ctp::parameters_for(trait_ref, false));
}

ctp::setup_constraining_predicates(impl_predicates.predicates.get_mut_slice(TypeSpace),
Expand Down Expand Up @@ -2267,9 +2267,9 @@ fn enforce_impl_lifetimes_are_constrained<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
let impl_trait_ref = ccx.tcx.impl_trait_ref(impl_def_id);

let mut input_parameters: HashSet<_> =
ctp::parameters_for_type(impl_scheme.ty, false).into_iter().collect();
ctp::parameters_for(&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, false));
input_parameters.extend(ctp::parameters_for(trait_ref, false));
}
ctp::identify_constrained_type_params(
&impl_predicates.predicates.as_slice(), impl_trait_ref, &mut input_parameters);
Expand All @@ -2280,7 +2280,7 @@ fn enforce_impl_lifetimes_are_constrained<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
ty::TypeTraitItem(ref assoc_ty) => assoc_ty.ty,
ty::ConstTraitItem(..) | ty::MethodTraitItem(..) => None
})
.flat_map(|ty| ctp::parameters_for_type(ty, true))
.flat_map(|ty| ctp::parameters_for(&ty, true))
.filter_map(|p| match p {
ctp::Parameter::Type(_) => None,
ctp::Parameter::Region(r) => Some(r),
Expand Down
108 changes: 42 additions & 66 deletions src/librustc_typeck/constrained_type_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc::ty::{self, subst, Ty};

use rustc::ty::{self, Ty};
use rustc::ty::fold::{TypeFoldable, TypeVisitor};
use std::collections::HashSet;

#[derive(Clone, PartialEq, Eq, Hash, Debug)]
Expand All @@ -19,77 +19,53 @@ pub enum 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
/// constrained by `t` - i.e. the value of each parameter in the list is
/// uniquely determined by `t` (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| match t.sty {
ty::TyProjection(..) if !include_projections => {
pub fn parameters_for<'tcx, T>(t: &T,
include_nonconstraining: bool)
-> Vec<Parameter>
where T: TypeFoldable<'tcx>
{

false // projections are not injective.
}
_ => {
result.append(&mut parameters_for_type_shallow(t));
// non-projection type constructors are injective.
true
}
});
result
let mut collector = ParameterCollector {
parameters: vec![],
include_nonconstraining: include_nonconstraining
};
t.visit_with(&mut collector);
collector.parameters
}

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, include_projections));

region_parameters.extend(type_parameters);

region_parameters
struct ParameterCollector {
parameters: Vec<Parameter>,
include_nonconstraining: bool
}

fn parameters_for_type_shallow<'tcx>(ty: Ty<'tcx>) -> Vec<Parameter> {
match ty.sty {
ty::TyParam(ref d) =>
vec![Parameter::Type(d.clone())],
ty::TyRef(region, _) =>
parameters_for_region(region).into_iter().collect(),
ty::TyStruct(_, substs) |
ty::TyEnum(_, substs) =>
parameters_for_regions_in_substs(substs),
ty::TyTrait(ref data) =>
parameters_for_regions_in_substs(&data.principal.skip_binder().substs),
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::TyFnDef(..) | ty::TyFnPtr(_) |
ty::TyTuple(..) | ty::TyRawPtr(..) |
ty::TyInfer(..) | ty::TyClosure(..) | ty::TyError =>
vec![]
}
}
impl<'tcx> TypeVisitor<'tcx> for ParameterCollector {
fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
match t.sty {
ty::TyProjection(..) if !self.include_nonconstraining => {
// projections are not injective
return false;
}
ty::TyParam(ref d) => {
self.parameters.push(Parameter::Type(d.clone()));
}
_ => {}
}

fn parameters_for_regions_in_substs(substs: &subst::Substs) -> Vec<Parameter> {
substs.regions
.iter()
.filter_map(|r| parameters_for_region(r))
.collect()
}
t.super_visit_with(self)
}

fn parameters_for_region(region: &ty::Region) -> Option<Parameter> {
match *region {
ty::ReEarlyBound(data) => Some(Parameter::Region(data)),
_ => None,
fn visit_region(&mut self, r: ty::Region) -> bool {
match r {
ty::ReEarlyBound(data) => {
self.parameters.push(Parameter::Region(data));
}
_ => {}
}
false
}
}

Expand Down Expand Up @@ -191,12 +167,12 @@ pub fn setup_constraining_predicates<'tcx>(predicates: &mut [ty::Predicate<'tcx>
// 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 inputs = parameters_for(&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, false));
input_parameters.extend(parameters_for(&projection.ty, false));
} else {
continue;
}
Expand Down
36 changes: 36 additions & 0 deletions src/test/compile-fail/issue-35139.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2016 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.

use std::fmt;

pub trait MethodType {
type GetProp: ?Sized;
}

pub struct MTFn;

impl<'a> MethodType for MTFn { //~ ERROR E0207
type GetProp = fmt::Debug + 'a;
}

fn bad(a: Box<<MTFn as MethodType>::GetProp>) -> Box<fmt::Debug+'static> {
a
}

fn dangling(a: &str) -> Box<fmt::Debug> {
bad(Box::new(a))
}

fn main() {
let mut s = "hello".to_string();
let x = dangling(&s);
s = String::new();
println!("{:?}", x);
}

0 comments on commit 7333c4a

Please sign in to comment.