From 02e1d5ec0613c0c6e89e4cd55376b112c57a582c Mon Sep 17 00:00:00 2001
From: Niko Matsakis <niko@alum.mit.edu>
Date: Tue, 17 Feb 2015 10:56:26 -0500
Subject: [PATCH 1/2] When converting parameters for an object type, be careful
of defaults that reference `Self`. Fixes #18956.
---
src/librustc/util/ppaux.rs | 15 ++++++++++-
src/librustc_typeck/astconv.rs | 23 ++++++++++++----
...rameter-defaults-referencing-Self-ppaux.rs | 26 +++++++++++++++++++
...ype-parameter-defaults-referencing-Self.rs | 23 ++++++++++++++++
4 files changed, 81 insertions(+), 6 deletions(-)
create mode 100644 src/test/compile-fail/type-parameter-defaults-referencing-Self-ppaux.rs
create mode 100644 src/test/compile-fail/type-parameter-defaults-referencing-Self.rs
diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs
index 0363978bada..cb928ce64bc 100644
--- a/src/librustc/util/ppaux.rs
+++ b/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()
diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs
index 0183b3474a5..19cf3bb93ea 100644
--- a/src/librustc_typeck/astconv.rs
+++ b/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");
}
}
diff --git a/src/test/compile-fail/type-parameter-defaults-referencing-Self-ppaux.rs b/src/test/compile-fail/type-parameter-defaults-referencing-Self-ppaux.rs
new file mode 100644
index 00000000000..8ff514e04e3
--- /dev/null
+++ b/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>`
+}
diff --git a/src/test/compile-fail/type-parameter-defaults-referencing-Self.rs b/src/test/compile-fail/type-parameter-defaults-referencing-Self.rs
new file mode 100644
index 00000000000..9982d485024
--- /dev/null
+++ b/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() { }
From ff388c12770dd8aa33ed48595f780d2aa49cd5d2 Mon Sep 17 00:00:00 2001
From: Niko Matsakis <niko@alum.mit.edu>
Date: Tue, 17 Feb 2015 10:57:15 -0500
Subject: [PATCH 2/2] Traits that reference `Self` in the supertrait list are
not object-safe. Fixes #22040.
---
src/librustc/middle/traits/object_safety.rs | 52 ++++++++++++++++---
src/librustc/middle/ty.rs | 36 ++++++++++++-
src/librustc_typeck/check/vtable.rs | 7 +++
.../compile-fail/object-safety-issue-22040.rs | 50 ++++++++++++++++++
.../object-safety-supertrait-mentions-Self.rs | 32 ++++++++++++
5 files changed, 170 insertions(+), 7 deletions(-)
create mode 100644 src/test/compile-fail/object-safety-issue-22040.rs
create mode 100644 src/test/compile-fail/object-safety-supertrait-mentions-Self.rs
diff --git a/src/librustc/middle/traits/object_safety.rs b/src/librustc/middle/traits/object_safety.rs
index b2701ae875c..d8d75f9084b 100644
--- a/src/librustc/middle/traits/object_safety.rs
+++ b/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,
+ }
+}
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
index 107715a8261..02eeed1cfda 100644
--- a/src/librustc/middle/ty.rs
+++ b/src/librustc/middle/ty.rs
@@ -78,7 +78,7 @@ use std::marker;
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;
@@ -2034,6 +2034,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(),
diff --git a/src/librustc_typeck/check/vtable.rs b/src/librustc_typeck/check/vtable.rs
index 2e7eff68bd5..3666b69d1c6 100644
--- a/src/librustc_typeck/check/vtable.rs
+++ b/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,
diff --git a/src/test/compile-fail/object-safety-issue-22040.rs b/src/test/compile-fail/object-safety-issue-22040.rs
new file mode 100644
index 00000000000..edf32131b68
--- /dev/null
+++ b/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);
+}
diff --git a/src/test/compile-fail/object-safety-supertrait-mentions-Self.rs b/src/test/compile-fail/object-safety-supertrait-mentions-Self.rs
new file mode 100644
index 00000000000..d3f9dc73020
--- /dev/null
+++ 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() {
+}