rollup merge of #22452: nikomatsakis/issue-22040-18956-Self

The big change here is that we update the object-safety rules to prohibit references to `Self` in the supertrait listing. See #22040 for the motivation. The other change is to handle the interaction of defaults that reference `Self` and object types (where `Self` is erased). We force users to give an explicit type in that scenario.

r? @aturon

Author: alexcrichton

src/librustc/middle/traits/object_safety.rs

@@ -20,7 +20,7 @@
 use super::supertraits;
 use super::elaborate_predicates;
 
-use middle::subst::{self, SelfSpace};
+use middle::subst::{self, SelfSpace, TypeSpace};
 use middle::traits;
 use middle::ty::{self, Ty};
 use std::rc::Rc;
@@ -31,6 +31,10 @@ pub enum ObjectSafetyViolation<'tcx> {
     /// Self : Sized declared on the trait
     SizedSelf,
 
+    /// Supertrait reference references `Self` an in illegal location
+    /// (e.g. `trait Foo : Bar<Self>`)
+    SupertraitSelf,
+
     /// Method has something illegal
     Method(Rc<ty::Method<'tcx>>, MethodViolationCode),
 }
@@ -110,6 +114,9 @@ fn object_safety_violations_for_trait<'tcx>(tcx: &ty::ctxt<'tcx>,
     if trait_has_sized_self(tcx, trait_def_id) {
         violations.push(ObjectSafetyViolation::SizedSelf);
     }
+    if supertraits_reference_self(tcx, trait_def_id) {
+        violations.push(ObjectSafetyViolation::SupertraitSelf);
+    }
 
     debug!("object_safety_violations_for_trait(trait_def_id={}) = {}",
            trait_def_id.repr(tcx),
@@ -118,6 +125,34 @@ fn object_safety_violations_for_trait<'tcx>(tcx: &ty::ctxt<'tcx>,
     violations
 }
 
+fn supertraits_reference_self<'tcx>(tcx: &ty::ctxt<'tcx>,
+                                    trait_def_id: ast::DefId)
+                                    -> bool
+{
+    let trait_def = ty::lookup_trait_def(tcx, trait_def_id);
+    let trait_ref = trait_def.trait_ref.clone();
+    let predicates = ty::predicates_for_trait_ref(tcx, &ty::Binder(trait_ref));
+    predicates
+        .into_iter()
+        .any(|predicate| {
+            match predicate {
+                ty::Predicate::Trait(ref data) => {
+                    // In the case of a trait predicate, we can skip the "self" type.
+                    data.0.trait_ref.substs.types.get_slice(TypeSpace)
+                                                 .iter()
+                                                 .cloned()
+                                                 .any(is_self)
+                }
+                ty::Predicate::Projection(..) |
+                ty::Predicate::TypeOutlives(..) |
+                ty::Predicate::RegionOutlives(..) |
+                ty::Predicate::Equate(..) => {
+                    false
+                }
+            }
+        })
+}
+
 fn trait_has_sized_self<'tcx>(tcx: &ty::ctxt<'tcx>,
                               trait_def_id: ast::DefId)
                               -> bool
@@ -138,11 +173,7 @@ fn trait_has_sized_self<'tcx>(tcx: &ty::ctxt<'tcx>,
         .any(|predicate| {
             match predicate {
                 ty::Predicate::Trait(ref trait_pred) if trait_pred.def_id() == sized_def_id => {
-                    let self_ty = trait_pred.0.self_ty();
-                    match self_ty.sty {
-                        ty::ty_param(ref data) => data.space == subst::SelfSpace,
-                        _ => false,
-                    }
+                    is_self(trait_pred.0.self_ty())
                 }
                 ty::Predicate::Projection(..) |
                 ty::Predicate::Trait(..) |
@@ -295,8 +326,17 @@ impl<'tcx> Repr<'tcx> for ObjectSafetyViolation<'tcx> {
         match *self {
             ObjectSafetyViolation::SizedSelf =>
                 format!("SizedSelf"),
+            ObjectSafetyViolation::SupertraitSelf =>
+                format!("SupertraitSelf"),
             ObjectSafetyViolation::Method(ref m, code) =>
                 format!("Method({},{:?})", m.repr(tcx), code),
         }
     }
 }
+
+fn is_self<'tcx>(ty: Ty<'tcx>) -> bool {
+    match ty.sty {
+        ty::ty_param(ref data) => data.space == subst::SelfSpace,
+        _ => false,
+    }
+}

src/librustc/middle/ty.rs

@@ -76,7 +76,7 @@ use std::hash::{Hash, Writer, SipHasher, Hasher};
 use std::mem;
 use std::ops;
 use std::rc::Rc;
-use std::vec::CowVec;
+use std::vec::{CowVec, IntoIter};
 use collections::enum_set::{EnumSet, CLike};
 use std::collections::{HashMap, HashSet};
 use syntax::abi;
@@ -2004,6 +2004,40 @@ impl<'tcx> AsPredicate<'tcx> for PolyProjectionPredicate<'tcx> {
 }
 
 impl<'tcx> Predicate<'tcx> {
+    /// Iterates over the types in this predicate. Note that in all
+    /// cases this is skipping over a binder, so late-bound regions
+    /// with depth 0 are bound by the predicate.
+    pub fn walk_tys(&self) -> IntoIter<Ty<'tcx>> {
+        let vec: Vec<_> = match *self {
+            ty::Predicate::Trait(ref data) => {
+                data.0.trait_ref.substs.types.as_slice().to_vec()
+            }
+            ty::Predicate::Equate(ty::Binder(ref data)) => {
+                vec![data.0, data.1]
+            }
+            ty::Predicate::TypeOutlives(ty::Binder(ref data)) => {
+                vec![data.0]
+            }
+            ty::Predicate::RegionOutlives(..) => {
+                vec![]
+            }
+            ty::Predicate::Projection(ref data) => {
+                let trait_inputs = data.0.projection_ty.trait_ref.substs.types.as_slice();
+                trait_inputs.iter()
+                            .cloned()
+                            .chain(Some(data.0.ty).into_iter())
+                            .collect()
+            }
+        };
+
+        // The only reason to collect into a vector here is that I was
+        // too lazy to make the full (somewhat complicated) iterator
+        // type that would be needed here. But I wanted this fn to
+        // return an iterator conceptually, rather than a `Vec`, so as
+        // to be closer to `Ty::walk`.
+        vec.into_iter()
+    }
+
     pub fn has_escaping_regions(&self) -> bool {
         match *self {
             Predicate::Trait(ref trait_ref) => trait_ref.has_escaping_regions(),

src/librustc/util/ppaux.rs

@@ -508,13 +508,26 @@ pub fn parameterized<'tcx,GG>(cx: &ctxt<'tcx>,
     // avoid those ICEs.
     let generics = get_generics();
 
+    let has_self = substs.self_ty().is_some();
     let tps = substs.types.get_slice(subst::TypeSpace);
     let ty_params = generics.types.get_slice(subst::TypeSpace);
     let has_defaults = ty_params.last().map_or(false, |def| def.default.is_some());
     let num_defaults = if has_defaults {
         ty_params.iter().zip(tps.iter()).rev().take_while(|&(def, &actual)| {
             match def.default {
-                Some(default) => default.subst(cx, substs) == actual,
+                Some(default) => {
+                    if !has_self && ty::type_has_self(default) {
+                        // In an object type, there is no `Self`, and
+                        // thus if the default value references Self,
+                        // the user will be required to give an
+                        // explicit value. We can't even do the
+                        // substitution below to check without causing
+                        // an ICE. (#18956).
+                        false
+                    } else {
+                        default.subst(cx, substs) == actual
+                    }
+                }
                 None => false
             }
         }).count()

src/librustc_typeck/astconv.rs

@@ -404,17 +404,30 @@ fn create_substs_for_ast_path<'tcx>(
 
     let actual_supplied_ty_param_count = substs.types.len(TypeSpace);
     for param in &ty_param_defs[actual_supplied_ty_param_count..] {
-        match param.default {
-            Some(default) => {
+        if let Some(default) = param.default {
+            // If we are converting an object type, then the
+            // `Self` parameter is unknown. However, some of the
+            // other type parameters may reference `Self` in their
+            // defaults. This will lead to an ICE if we are not
+            // careful!
+            if self_ty.is_none() && ty::type_has_self(default) {
+                tcx.sess.span_err(
+                    span,
+                    &format!("the type parameter `{}` must be explicitly specified \
+                              in an object type because its default value `{}` references \
+                              the type `Self`",
+                             param.name.user_string(tcx),
+                             default.user_string(tcx)));
+                substs.types.push(TypeSpace, tcx.types.err);
+            } else {
                 // This is a default type parameter.
                 let default = default.subst_spanned(tcx,
                                                     &substs,
                                                     Some(span));
                 substs.types.push(TypeSpace, default);
             }
-            None => {
-                tcx.sess.span_bug(span, "extra parameter without default");
-            }
+        } else {
+            tcx.sess.span_bug(span, "extra parameter without default");
         }
     }
 

src/librustc_typeck/check/vtable.rs

@@ -126,6 +126,13 @@ pub fn check_object_safety<'tcx>(tcx: &ty::ctxt<'tcx>,
                     "the trait cannot require that `Self : Sized`");
             }
 
+            ObjectSafetyViolation::SupertraitSelf => {
+                tcx.sess.span_note(
+                    span,
+                    "the trait cannot use `Self` as a type parameter \
+                     in the supertrait listing");
+            }
+
             ObjectSafetyViolation::Method(method, MethodViolationCode::ByValueSelf) => {
                 tcx.sess.span_note(
                     span,

src/test/compile-fail/object-safety-issue-22040.rs

@@ -0,0 +1,50 @@
+// 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.
+
+// Regression test for #22040.
+
+use std::fmt::Debug;
+
+trait Expr: Debug + PartialEq {
+    fn print_element_count(&self);
+}
+
+//#[derive(PartialEq)]
+#[derive(Debug)]
+struct SExpr<'x> {
+    elements: Vec<Box<Expr+ 'x>>,
+}
+
+impl<'x> PartialEq for SExpr<'x> {
+    fn eq(&self, other:&SExpr<'x>) -> bool {
+        println!("L1: {} L2: {}", self.elements.len(), other.elements.len());
+        let result = self.elements.len() == other.elements.len();
+
+        println!("Got compare {}", result);
+        return result;
+    }
+}
+
+impl <'x> SExpr<'x> {
+    fn new() -> SExpr<'x> { return SExpr{elements: Vec::new(),}; }
+}
+
+impl <'x> Expr for SExpr<'x> {
+    fn print_element_count(&self) {
+        println!("element count: {}", self.elements.len());
+    }
+}
+
+fn main() {
+    let a: Box<Expr> = Box::new(SExpr::new()); //~ ERROR trait `Expr` is not object-safe
+    let b: Box<Expr> = Box::new(SExpr::new()); //~ ERROR trait `Expr` is not object-safe
+
+    assert_eq!(a , b);
+}

src/test/compile-fail/object-safety-supertrait-mentions-Self.rs

@@ -0,0 +1,32 @@
+// Copyright 2014 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.
+
+// Check that we correctly prevent users from making trait objects
+// form traits that make use of `Self` in an argument or return position.
+
+trait Bar<T> {
+    fn bar(&self, x: &T);
+}
+
+trait Baz : Bar<Self> {
+}
+
+fn make_bar<T:Bar<u32>>(t: &T) -> &Bar<u32> {
+    t
+}
+
+fn make_baz<T:Baz>(t: &T) -> &Baz {
+    t
+        //~^ ERROR `Baz` is not object-safe
+        //~| NOTE the trait cannot use `Self` as a type parameter in the supertrait listing
+}
+
+fn main() {
+}

src/test/compile-fail/type-parameter-defaults-referencing-Self-ppaux.rs

@@ -0,0 +1,26 @@
+// 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.
+
+// Test a default that references `Self` which is then used in an
+// object type. Issue #18956. In this case, the value is supplied by
+// the user, but pretty-printing the type during the error message
+// caused an ICE.
+
+trait MyAdd<Rhs=Self> { fn add(&self, other: &Rhs) -> Self; }
+
+impl MyAdd for i32 {
+    fn add(&self, other: &i32) -> i32 { *self + *other }
+}
+
+fn main() {
+    let x = 5;
+    let y = x as MyAdd<i32>;
+    //~^ ERROR as `MyAdd<i32>`
+}

src/test/compile-fail/type-parameter-defaults-referencing-Self.rs

@@ -0,0 +1,23 @@
+// 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.
+
+// Test a default that references `Self` which is then used in an object type.
+// Issue #18956.
+
+#![feature(default_type_params)]
+
+trait Foo<T=Self> {
+    fn method(&self);
+}
+
+fn foo(x: &Foo) { }
+//~^ ERROR the type parameter `T` must be explicitly specified
+
+fn main() { }