auto merge of #20889 : Manishearth/rust/trait-error, r=nikomatsakis

fixes #20783

r? @nikomatsakis

Author: bors

src/doc/reference.md

@@ -2117,6 +2117,13 @@ macro scope.
   destructors from being run twice. Destructors might be run multiple times on
   the same object with this attribute.
 - `doc` - Doc comments such as `/// foo` are equivalent to `#[doc = "foo"]`.
+- `rustc_on_unimplemented` - Write a custom note to be shown along with the error
+   when the trait is found to be unimplemented on a type.
+   You may use format arguments like `{T}`, `{A}` to correspond to the
+   types at the point of use corresponding to the type parameters of the
+   trait of the same name. `{Self}` will be replaced with the type that is supposed
+   to implement the trait but doesn't. To use this, the `on_unimplemented` feature gate
+   must be enabled.
 
 ### Conditional compilation
 

src/libcore/iter.rs

@@ -101,6 +101,8 @@ pub trait Iterator {
 
 /// Conversion from an `Iterator`
 #[stable]
+#[rustc_on_unimplemented="a collection of type `{Self}` cannot be \
+                          built from an iterator over elements of type `{A}`"]
 pub trait FromIterator<A> {
     /// Build a container with elements from an external iterator.
     fn from_iter<T: Iterator<Item=A>>(iterator: T) -> Self;

src/libcore/lib.rs

@@ -63,6 +63,7 @@
 #![feature(simd, unsafe_destructor, slicing_syntax)]
 #![feature(unboxed_closures)]
 #![allow(unknown_features)] #![feature(int_uint)]
+#![feature(on_unimplemented)]
 #![deny(missing_docs)]
 
 #[macro_use]

src/librustc/lib.rs

@@ -32,6 +32,7 @@
 
 extern crate arena;
 extern crate flate;
+extern crate fmt_macros;
 extern crate getopts;
 extern crate graphviz;
 extern crate libc;

src/librustc/lint/builtin.rs

@@ -666,6 +666,7 @@ impl LintPass for UnusedAttributes {
             "must_use",
             "stable",
             "unstable",
+            "rustc_on_unimplemented",
 
             // FIXME: #19470 this shouldn't be needed forever
             "old_orphan_check",

src/librustc/middle/traits/error_reporting.rs

@@ -18,9 +18,12 @@ use super::{
     SelectionError,
 };
 
+use fmt_macros::{Parser, Piece, Position};
 use middle::infer::InferCtxt;
-use middle::ty::{self, AsPredicate, ReferencesError, ToPolyTraitRef};
-use syntax::codemap::Span;
+use middle::ty::{self, AsPredicate, ReferencesError, ToPolyTraitRef, TraitRef};
+use std::collections::HashMap;
+use syntax::codemap::{DUMMY_SP, Span};
+use syntax::attr::{AttributeMethods, AttrMetaMethods};
 use util::ppaux::{Repr, UserString};
 
 pub fn report_fulfillment_errors<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
@@ -62,6 +65,85 @@ pub fn report_projection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
     }
 }
 
+fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
+                                     trait_ref: &TraitRef<'tcx>,
+                                     span: Span) -> Option<String> {
+    let def_id = trait_ref.def_id;
+    let mut report = None;
+    ty::each_attr(infcx.tcx, def_id, |item| {
+        if item.check_name("rustc_on_unimplemented") {
+            let err_sp = if item.meta().span == DUMMY_SP {
+                span
+            } else {
+                item.meta().span
+            };
+            let def = ty::lookup_trait_def(infcx.tcx, def_id);
+            let trait_str = def.trait_ref.user_string(infcx.tcx);
+            if let Some(ref istring) = item.value_str() {
+                let mut generic_map = def.generics.types.iter_enumerated()
+                                         .map(|(param, i, gen)| {
+                                               (gen.name.as_str().to_string(),
+                                                trait_ref.substs.types.get(param, i)
+                                                         .user_string(infcx.tcx))
+                                              }).collect::<HashMap<String, String>>();
+                generic_map.insert("Self".to_string(),
+                                   trait_ref.self_ty().user_string(infcx.tcx));
+                let parser = Parser::new(istring.get());
+                let mut errored = false;
+                let err: String = parser.filter_map(|p| {
+                    match p {
+                        Piece::String(s) => Some(s),
+                        Piece::NextArgument(a) => match a.position {
+                            Position::ArgumentNamed(s) => match generic_map.get(s) {
+                                Some(val) => Some(val.as_slice()),
+                                None => {
+                                    infcx.tcx.sess
+                                         .span_err(err_sp,
+                                                   format!("the #[rustc_on_unimplemented] \
+                                                            attribute on \
+                                                            trait definition for {} refers to \
+                                                            non-existent type parameter {}",
+                                                           trait_str, s)
+                                                   .as_slice());
+                                    errored = true;
+                                    None
+                                }
+                            },
+                            _ => {
+                                infcx.tcx.sess
+                                     .span_err(err_sp,
+                                               format!("the #[rustc_on_unimplemented] \
+                                                        attribute on \
+                                                        trait definition for {} must have named \
+                                                        format arguments, \
+                                                        eg `#[rustc_on_unimplemented = \
+                                                        \"foo {{T}}\"]`",
+                                                       trait_str).as_slice());
+                                errored = true;
+                                None
+                            }
+                        }
+                    }
+                }).collect();
+                // Report only if the format string checks out
+                if !errored {
+                    report = Some(err);
+                }
+            } else {
+                infcx.tcx.sess.span_err(err_sp,
+                                        format!("the #[rustc_on_unimplemented] attribute on \
+                                                 trait definition for {} must have a value, \
+                                                 eg `#[rustc_on_unimplemented = \"foo\"]`",
+                                                 trait_str).as_slice());
+            }
+            false
+        } else {
+            true
+        }
+    });
+    report
+}
+
 pub fn report_selection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
                                         obligation: &PredicateObligation<'tcx>,
                                         error: &SelectionError<'tcx>)
@@ -94,6 +176,14 @@ pub fn report_selection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
                                 "the trait `{}` is not implemented for the type `{}`",
                                 trait_ref.user_string(infcx.tcx),
                                 trait_ref.self_ty().user_string(infcx.tcx)).as_slice());
+                        // Check if it has a custom "#[rustc_on_unimplemented]" error message,
+                        // report with that message if it does
+                        let custom_note = report_on_unimplemented(infcx, &*trait_ref.0,
+                                                                  obligation.cause.span);
+                        if let Some(s) = custom_note {
+                           infcx.tcx.sess.span_note(obligation.cause.span,
+                                                    s.as_slice());
+                        }
                     }
                 }
 

src/librustc_typeck/check/mod.rs

@@ -83,6 +83,7 @@ use self::TupleArgumentsFlag::*;
 
 use astconv::{self, ast_region_to_region, ast_ty_to_ty, AstConv};
 use check::_match::pat_ctxt;
+use fmt_macros::{Parser, Piece, Position};
 use middle::{const_eval, def};
 use middle::infer;
 use middle::lang_items::IteratorItem;
@@ -114,6 +115,7 @@ use std::rc::Rc;
 use std::iter::repeat;
 use std::slice;
 use syntax::{self, abi, attr};
+use syntax::attr::AttrMetaMethods;
 use syntax::ast::{self, ProvidedMethod, RequiredMethod, TypeTraitItem, DefId};
 use syntax::ast_util::{self, local_def, PostExpansionMethod};
 use syntax::codemap::{self, Span};
@@ -727,7 +729,8 @@ pub fn check_item(ccx: &CrateCtxt, it: &ast::Item) {
         }
 
       }
-      ast::ItemTrait(_, _, _, ref trait_methods) => {
+      ast::ItemTrait(_, ref generics, _, ref trait_methods) => {
+        check_trait_on_unimplemented(ccx, generics, it);
         let trait_def = ty::lookup_trait_def(ccx.tcx, local_def(it.id));
         for trait_method in trait_methods.iter() {
             match *trait_method {
@@ -777,6 +780,51 @@ pub fn check_item(ccx: &CrateCtxt, it: &ast::Item) {
     }
 }
 
+fn check_trait_on_unimplemented<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
+                               generics: &ast::Generics,
+                               item: &ast::Item) {
+    if let Some(ref attr) = item.attrs.iter().find(|&: a| {
+        a.check_name("rustc_on_unimplemented")
+    }) {
+        if let Some(ref istring) = attr.value_str() {
+            let mut parser = Parser::new(istring.get());
+            let types = generics.ty_params.as_slice();
+            for token in parser {
+                match token {
+                    Piece::String(_) => (), // Normal string, no need to check it
+                    Piece::NextArgument(a) => match a.position {
+                        // `{Self}` is allowed
+                        Position::ArgumentNamed(s) if s == "Self" => (),
+                        // So is `{A}` if A is a type parameter
+                        Position::ArgumentNamed(s) => match types.iter().find(|t| {
+                            t.ident.as_str() == s
+                        }) {
+                            Some(_) => (),
+                            None => {
+                                ccx.tcx.sess.span_err(attr.span,
+                                                 format!("there is no type parameter \
+                                                          {} on trait {}",
+                                                           s, item.ident.as_str())
+                                            .as_slice());
+                            }
+                        },
+                        // `{:1}` and `{}` are not to be used
+                        Position::ArgumentIs(_) | Position::ArgumentNext => {
+                            ccx.tcx.sess.span_err(attr.span,
+                                                  "only named substitution \
+                                                   parameters are allowed");
+                        }
+                    }
+                }
+            }
+        } else {
+            ccx.tcx.sess.span_err(attr.span,
+                                  "this attribute must have a value, \
+                                   eg `#[rustc_on_unimplemented = \"foo\"]`")
+        }
+    }
+}
+
 /// Type checks a method body.
 ///
 /// # Parameters

src/librustc_typeck/lib.rs

@@ -84,6 +84,7 @@ This API is completely unstable and subject to change.
 #[macro_use] extern crate syntax;
 
 extern crate arena;
+extern crate fmt_macros;
 extern crate rustc;
 
 pub use rustc::lint;

src/libsyntax/feature_gate.rs

@@ -71,6 +71,7 @@ static KNOWN_FEATURES: &'static [(&'static str, Status)] = &[
     ("visible_private_types", Active),
     ("slicing_syntax", Active),
     ("box_syntax", Active),
+    ("on_unimplemented", Active),
 
     ("if_let", Accepted),
     ("while_let", Accepted),
@@ -249,6 +250,10 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
                 self.gate_feature("linkage", i.span,
                                   "the `linkage` attribute is experimental \
                                    and not portable across platforms")
+            } else if attr.name() == "rustc_on_unimplemented" {
+                self.gate_feature("on_unimplemented", i.span,
+                                  "the `#[rustc_on_unimplemented]` attribute \
+                                  is an experimental feature")
             }
         }
         match i.node {

src/test/compile-fail/on-unimplemented-bad-anno.rs

@@ -0,0 +1,37 @@
+// 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.
+// ignore-tidy-linelength
+
+#![feature(on_unimplemented)]
+
+#![allow(unused)]
+
+#[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"]
+trait Foo<Bar, Baz, Quux>{}
+
+#[rustc_on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"]
+trait MyFromIterator<A> {
+    /// Build a container with elements from an external iterator.
+    fn my_from_iter<T: Iterator<Item=A>>(iterator: T) -> Self;
+}
+
+#[rustc_on_unimplemented] //~ ERROR this attribute must have a value
+trait BadAnnotation1 {}
+
+#[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"]
+//~^ ERROR there is no type parameter C on trait BadAnnotation2
+trait BadAnnotation2<A,B> {}
+
+#[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{}>`"]
+//~^ only named substitution parameters are allowed
+trait BadAnnotation3<A,B> {}
+
+pub fn main() {
+}

src/test/compile-fail/on-unimplemented.rs

@@ -0,0 +1,38 @@
+// 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.
+// ignore-tidy-linelength
+
+#![feature(on_unimplemented)]
+
+#[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"]
+trait Foo<Bar, Baz, Quux>{}
+
+fn foobar<U: Clone, T: Foo<u8, U, u32>>() -> T {
+
+}
+
+#[rustc_on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"]
+trait MyFromIterator<A> {
+    /// Build a container with elements from an external iterator.
+    fn my_from_iter<T: Iterator<Item=A>>(iterator: T) -> Self;
+}
+
+fn collect<A, I: Iterator<Item=A>, B: MyFromIterator<A>>(it: I) -> B {
+    MyFromIterator::my_from_iter(it)
+}
+
+pub fn main() {
+    let x = vec!(1u8, 2, 3, 4);
+    let y: Option<Vec<u8>> = collect(x.iter()); // this should give approximately the same error for x.iter().collect()
+    //~^ ERROR
+    //~^^ NOTE a collection of type `core::option::Option<collections::vec::Vec<u8>>` cannot be built from an iterator over elements of type `&u8`
+    let x: String = foobar(); //~ ERROR
+    //~^ NOTE test error `collections::string::String` with `u8` `_` `u32`
+}