Auto merge of rust-lang#35143 - arielb1:rfc447-regions, r=eddyb

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 rust-lang#35139

r? @eddyb

src/librustc_typeck/collect.rs

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

src/librustc_typeck/constrained_type_params.rs

@@ -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)]
@@ -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
     }
 }
 
@@ -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;
             }

src/test/compile-fail/issue-35139.rs

@@ -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);
+}