auto merge of #17704 : nick29581/rust/object-safety, r=nikomatsakis

r? @nikomatsakis

Author: bors

src/librustc/diagnostics.rs

@@ -53,7 +53,6 @@ register_diagnostics!(
     E0035,
     E0036,
     E0038,
-    E0039,
     E0040,
     E0044,
     E0045,

src/librustc/middle/typeck/check/method.rs

@@ -619,14 +619,12 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
 
         let tcx = self.tcx();
 
-        // It is illegal to invoke a method on a trait instance that
-        // refers to the `Self` type. An error will be reported by
-        // `enforce_object_limitations()` if the method refers to the
-        // `Self` type anywhere other than the receiver. Here, we use
-        // a substitution that replaces `Self` with the object type
-        // itself. Hence, a `&self` method will wind up with an
-        // argument type like `&Trait`.
+        // It is illegal to invoke a method on a trait instance that refers to
+        // the `Self` type.  Here, we use a substitution that replaces `Self`
+        // with the object type itself. Hence, a `&self` method will wind up
+        // with an argument type like `&Trait`.
         let rcvr_substs = substs.with_self_ty(self_ty);
+
         let trait_ref = Rc::new(TraitRef {
             def_id: did,
             substs: rcvr_substs.clone()
@@ -1336,16 +1334,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
                self.ty_to_string(rcvr_ty),
                candidate.repr(self.tcx()));
 
-        let mut rcvr_substs = candidate.rcvr_substs.clone();
-
-        if !self.enforce_object_limitations(candidate) {
-            // Here we change `Self` from `Trait` to `err` in the case that
-            // this is an illegal object method. This is necessary to prevent
-            // the user from getting strange, derivative errors when the method
-            // takes an argument/return-type of type `Self` etc.
-            rcvr_substs.types.get_mut_slice(SelfSpace)[0] = ty::mk_err();
-        }
-
+        let rcvr_substs = candidate.rcvr_substs.clone();
         self.enforce_drop_trait_limitations(candidate);
 
         // Determine the values for the generic parameters of the method.
@@ -1554,71 +1543,6 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
         }
     }
 
-    fn enforce_object_limitations(&self, candidate: &Candidate) -> bool {
-        /*!
-         * There are some limitations to calling functions through an
-         * object, because (a) the self type is not known
-         * (that's the whole point of a trait instance, after all, to
-         * obscure the self type) and (b) the call must go through a
-         * vtable and hence cannot be monomorphized.
-         */
-
-        match candidate.origin {
-            MethodStatic(..) |
-            MethodTypeParam(..) |
-            MethodStaticUnboxedClosure(..) => {
-                return true; // not a call to a trait instance
-            }
-            MethodTraitObject(..) => {}
-        }
-
-        match candidate.method_ty.explicit_self {
-            ty::StaticExplicitSelfCategory => { // reason (a) above
-                self.tcx().sess.span_err(
-                    self.span,
-                    "cannot call a method without a receiver \
-                     through an object");
-                return false;
-            }
-
-            ty::ByValueExplicitSelfCategory |
-            ty::ByReferenceExplicitSelfCategory(..) |
-            ty::ByBoxExplicitSelfCategory => {}
-        }
-
-        // reason (a) above
-        let check_for_self_ty = |ty| -> bool {
-            if ty::type_has_self(ty) {
-                span_err!(self.tcx().sess, self.span, E0038,
-                    "cannot call a method whose type contains a \
-                     self-type through an object");
-                false
-            } else {
-                true
-            }
-        };
-        let ref sig = candidate.method_ty.fty.sig;
-        for &input_ty in sig.inputs[1..].iter() {
-            if !check_for_self_ty(input_ty) {
-                return false;
-            }
-        }
-        if let ty::FnConverging(result_type) = sig.output {
-            if !check_for_self_ty(result_type) {
-                return false;
-            }
-        }
-
-        if candidate.method_ty.generics.has_type_params(subst::FnSpace) {
-            // reason (b) above
-            span_err!(self.tcx().sess, self.span, E0039,
-                "cannot call a generic method through an object");
-            return false;
-        }
-
-        true
-    }
-
     fn enforce_drop_trait_limitations(&self, candidate: &Candidate) {
         // No code can call the finalize method explicitly.
         let bad = match candidate.origin {

src/librustc/middle/typeck/check/mod.rs

@@ -1687,6 +1687,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 self.register_unsize_obligations(span, &**u)
             }
             ty::UnsizeVtable(ref ty_trait, self_ty) => {
+                vtable::check_object_safety(self.tcx(), ty_trait, span);
                 // If the type is `Foo+'a`, ensures that the type
                 // being cast to `Foo+'a` implements `Foo`:
                 vtable::register_object_cast_obligations(self,

src/librustc/middle/typeck/check/vtable.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use middle::subst::{SelfSpace};
+use middle::subst::{SelfSpace, FnSpace};
 use middle::traits;
 use middle::traits::{SelectionError, OutputTypeParameterMismatch, Overflow, Unimplemented};
 use middle::traits::{Obligation, obligation_for_builtin_bound};
@@ -21,8 +21,7 @@ use middle::typeck::infer;
 use std::rc::Rc;
 use syntax::ast;
 use syntax::codemap::Span;
-use util::ppaux::UserString;
-use util::ppaux::Repr;
+use util::ppaux::{UserString, Repr, ty_to_string};
 
 pub fn check_object_cast(fcx: &FnCtxt,
                          cast_expr: &ast::Expr,
@@ -46,6 +45,7 @@ pub fn check_object_cast(fcx: &FnCtxt,
 
             // Ensure that if ~T is cast to ~Trait, then T : Trait
             push_cast_obligation(fcx, cast_expr, object_trait, referent_ty);
+            check_object_safety(fcx.tcx(), object_trait, source_expr.span);
         }
 
         (&ty::ty_rptr(referent_region, ty::mt { ty: referent_ty,
@@ -68,6 +68,8 @@ pub fn check_object_cast(fcx: &FnCtxt,
                                infer::RelateObjectBound(source_expr.span),
                                target_region,
                                referent_region);
+
+                check_object_safety(fcx.tcx(), object_trait, source_expr.span);
             }
         }
 
@@ -128,6 +130,103 @@ pub fn check_object_cast(fcx: &FnCtxt,
     }
 }
 
+// Check that a trait is 'object-safe'. This should be checked whenever a trait object
+// is created (by casting or coercion, etc.). A trait is object-safe if all its
+// methods are object-safe. A trait method is object-safe if it does not take
+// self by value, has no type parameters and does not use the `Self` type, except
+// in self position.
+pub fn check_object_safety(tcx: &ty::ctxt, object_trait: &ty::TyTrait, span: Span) {
+    // Skip the fn_once lang item trait since only the compiler should call
+    // `call_once` which is the method which takes self by value. What could go
+    // wrong?
+    match tcx.lang_items.fn_once_trait() {
+        Some(def_id) if def_id == object_trait.def_id => return,
+        _ => {}
+    }
+
+    let trait_items = ty::trait_items(tcx, object_trait.def_id);
+
+    let mut errors = Vec::new();
+    for item in trait_items.iter() {
+        match *item {
+            ty::MethodTraitItem(ref m) => {
+                errors.push(check_object_safety_of_method(tcx, &**m))
+            }
+            ty::TypeTraitItem(_) => {}
+        }
+    }
+
+    let mut errors = errors.iter().flat_map(|x| x.iter()).peekable();
+    if errors.peek().is_some() {
+        let trait_name = ty::item_path_str(tcx, object_trait.def_id);
+        span_err!(tcx.sess, span, E0038,
+            "cannot convert to a trait object because trait `{}` is not object-safe",
+            trait_name);
+
+        for msg in errors {
+            tcx.sess.note(msg.as_slice());
+        }
+    }
+
+    // Returns a vec of error messages. If hte vec is empty - no errors!
+    fn check_object_safety_of_method(tcx: &ty::ctxt, method: &ty::Method) -> Vec<String> {
+        /*!
+         * There are some limitations to calling functions through an
+         * object, because (a) the self type is not known
+         * (that's the whole point of a trait instance, after all, to
+         * obscure the self type) and (b) the call must go through a
+         * vtable and hence cannot be monomorphized.
+         */
+        let mut msgs = Vec::new();
+
+        let method_name = method.name.repr(tcx);
+
+        match method.explicit_self {
+            ty::ByValueExplicitSelfCategory => { // reason (a) above
+                msgs.push(format!("cannot call a method (`{}`) with a by-value \
+                                   receiver through a trait object", method_name))
+            }
+
+            ty::StaticExplicitSelfCategory |
+            ty::ByReferenceExplicitSelfCategory(..) |
+            ty::ByBoxExplicitSelfCategory => {}
+        }
+
+        // reason (a) above
+        let check_for_self_ty = |ty| {
+            if ty::type_has_self(ty) {
+                Some(format!(
+                    "cannot call a method (`{}`) whose type contains \
+                     a self-type (`{}`) through a trait object",
+                    method_name, ty_to_string(tcx, ty)))
+            } else {
+                None
+            }
+        };
+        let ref sig = method.fty.sig;
+        for &input_ty in sig.inputs[1..].iter() {
+            match check_for_self_ty(input_ty) {
+                Some(msg) => msgs.push(msg),
+                _ => {}
+            }
+        }
+        if let ty::FnConverging(result_type) = sig.output {
+            match check_for_self_ty(result_type) {
+                Some(msg) => msgs.push(msg),
+                _ => {}
+            }
+        }
+
+        if method.generics.has_type_params(FnSpace) {
+            // reason (b) above
+            msgs.push(format!("cannot call a generic method (`{}`) through a trait object",
+                              method_name));
+        }
+
+        msgs
+    }
+}
+
 pub fn register_object_cast_obligations(fcx: &FnCtxt,
                                         span: Span,
                                         object_trait: &ty::TyTrait,

src/libstd/io/extensions.rs

@@ -172,7 +172,7 @@ pub fn u64_from_be_bytes(data: &[u8], start: uint, size: uint) -> u64 {
 mod test {
     use prelude::*;
     use io;
-    use io::{MemReader, MemWriter};
+    use io::{MemReader, MemWriter, BytesReader};
 
     struct InitialZeroByteReader {
         count: int,

src/libstd/io/mod.rs

@@ -712,17 +712,6 @@ pub trait Reader {
         })
     }
 
-    /// Create an iterator that reads a single byte on
-    /// each iteration, until EOF.
-    ///
-    /// # Error
-    ///
-    /// Any error other than `EndOfFile` that is produced by the underlying Reader
-    /// is returned by the iterator and should be handled by the caller.
-    fn bytes<'r>(&'r mut self) -> extensions::Bytes<'r, Self> {
-        extensions::Bytes::new(self)
-    }
-
     // Byte conversion helpers
 
     /// Reads `n` little-endian unsigned integer bytes.
@@ -932,16 +921,41 @@ pub trait Reader {
     fn read_i8(&mut self) -> IoResult<i8> {
         self.read_byte().map(|i| i as i8)
     }
+}
 
+/// A reader which can be converted to a RefReader.
+pub trait AsRefReader {
     /// Creates a wrapper around a mutable reference to the reader.
     ///
     /// This is useful to allow applying adaptors while still
     /// retaining ownership of the original value.
-    fn by_ref<'a>(&'a mut self) -> RefReader<'a, Self> {
+    fn by_ref<'a>(&'a mut self) -> RefReader<'a, Self>;
+}
+
+impl<T: Reader> AsRefReader for T {
+    fn by_ref<'a>(&'a mut self) -> RefReader<'a, T> {
         RefReader { inner: self }
     }
 }
 
+/// A reader which can be converted to bytes.
+pub trait BytesReader {
+    /// Create an iterator that reads a single byte on
+    /// each iteration, until EOF.
+    ///
+    /// # Error
+    ///
+    /// Any error other than `EndOfFile` that is produced by the underlying Reader
+    /// is returned by the iterator and should be handled by the caller.
+    fn bytes<'r>(&'r mut self) -> extensions::Bytes<'r, Self>;
+}
+
+impl<T: Reader> BytesReader for T {
+    fn bytes<'r>(&'r mut self) -> extensions::Bytes<'r, T> {
+        extensions::Bytes::new(self)
+    }
+}
+
 impl<'a> Reader for Box<Reader+'a> {
     fn read(&mut self, buf: &mut [u8]) -> IoResult<uint> {
         let reader: &mut Reader = &mut **self;
@@ -986,6 +1000,7 @@ unsafe fn slice_vec_capacity<'a, T>(v: &'a mut Vec<T>, start: uint, end: uint) -
 /// # fn process_input<R: Reader>(r: R) {}
 /// # fn foo() {
 /// use std::io;
+/// use std::io::AsRefReader;
 /// use std::io::util::LimitReader;
 ///
 /// let mut stream = io::stdin();
@@ -1268,13 +1283,20 @@ pub trait Writer {
     fn write_i8(&mut self, n: i8) -> IoResult<()> {
         self.write([n as u8])
     }
+}
 
+/// A writer which can be converted to a RefWriter.
+pub trait AsRefWriter {
     /// Creates a wrapper around a mutable reference to the writer.
     ///
     /// This is useful to allow applying wrappers while still
     /// retaining ownership of the original value.
     #[inline]
-    fn by_ref<'a>(&'a mut self) -> RefWriter<'a, Self> {
+    fn by_ref<'a>(&'a mut self) -> RefWriter<'a, Self>;
+}
+
+impl<T: Writer> AsRefWriter for T {
+    fn by_ref<'a>(&'a mut self) -> RefWriter<'a, T> {
         RefWriter { inner: self }
     }
 }
@@ -1309,7 +1331,7 @@ impl<'a> Writer for &'a mut Writer+'a {
 /// # fn process_input<R: Reader>(r: R) {}
 /// # fn foo () {
 /// use std::io::util::TeeReader;
-/// use std::io::{stdin, MemWriter};
+/// use std::io::{stdin, MemWriter, AsRefWriter};
 ///
 /// let mut output = MemWriter::new();
 ///

src/libstd/io/util.rs

@@ -265,7 +265,7 @@ impl<T: Iterator<u8>> Reader for IterReader<T> {
 
 #[cfg(test)]
 mod test {
-    use io::{MemReader, MemWriter, BufReader};
+    use io::{MemReader, MemWriter, BufReader, AsRefReader};
     use io;
     use boxed::Box;
     use super::*;