Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to attach custom #[on_unimplemented] error messages for unimplemented traits #20889

Merged
merged 7 commits into from
Jan 12, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/doc/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions src/libcore/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

extern crate arena;
extern crate flate;
extern crate fmt_macros;
extern crate getopts;
extern crate graphviz;
extern crate libc;
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
94 changes: 92 additions & 2 deletions src/librustc/middle/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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>)
Expand Down Expand Up @@ -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());
}
}
}

Expand Down
50 changes: 49 additions & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -113,6 +114,7 @@ use std::mem::replace;
use std::rc::Rc;
use std::iter::repeat;
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};
Expand Down Expand Up @@ -726,7 +728,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 {
Expand Down Expand Up @@ -776,6 +779,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
Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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 {
Expand Down
37 changes: 37 additions & 0 deletions src/test/compile-fail/on-unimplemented-bad-anno.rs
Original file line number Diff line number Diff line change
@@ -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> {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a test with an illegal thing, like {}?


#[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() {
}
38 changes: 38 additions & 0 deletions src/test/compile-fail/on-unimplemented.rs
Original file line number Diff line number Diff line change
@@ -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`
}