From 4d17fbaf37a0641894317f016244943af66ce87b Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sat, 10 Jan 2015 23:51:27 +0530 Subject: [PATCH 1/7] Add ability to attach custom #[on_unimplemented] error messages for unimplemented traits (fixes #20783) --- src/libcore/iter.rs | 2 + src/librustc/lib.rs | 1 + src/librustc/lint/builtin.rs | 1 + src/librustc/middle/traits/error_reporting.rs | 76 ++++++++++++++++++- src/test/compile-fail/on-unimplemented.rs | 51 +++++++++++++ 5 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/on-unimplemented.rs diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index d4aa4c99a76bd..e22de10a974f0 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -101,6 +101,8 @@ pub trait Iterator { /// Conversion from an `Iterator` #[stable] +#[on_unimplemented="a collection of type `{Self}` cannot be \ + built from an iterator over elements of type `{A}`"] pub trait FromIterator { /// Build a container with elements from an external iterator. fn from_iter>(iterator: T) -> Self; diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index fb7c5296d020e..e720a5df59807 100644 --- a/src/librustc/lib.rs +++ b/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; diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 620b8f277dddc..300b9aaf10060 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -666,6 +666,7 @@ impl LintPass for UnusedAttributes { "must_use", "stable", "unstable", + "on_unimplemented", // FIXME: #19470 this shouldn't be needed forever "old_orphan_check", diff --git a/src/librustc/middle/traits/error_reporting.rs b/src/librustc/middle/traits/error_reporting.rs index 02c913a9e81ae..8903667505b32 100644 --- a/src/librustc/middle/traits/error_reporting.rs +++ b/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 middle::ty::{self, AsPredicate, ReferencesError, ToPolyTraitRef, TraitRef}; +use std::collections::HashMap; use syntax::codemap::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,69 @@ 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>) -> Option { + let def_id = trait_ref.def_id; + let mut report = None; + ty::each_attr(infcx.tcx, def_id, |item| { + if item.check_name("on_unimplemented") { + if let Some(ref istring) = item.value_str() { + let def = ty::lookup_trait_def(infcx.tcx, def_id); + 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::>(); + 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(item.meta().span, + format!("there is no type parameter \ + {} on trait {}", + s, def.trait_ref + .user_string(infcx.tcx)) + .as_slice()); + errored = true; + None + } + }, + _ => { + infcx.tcx.sess.span_err(item.meta().span, + "only named substitution \ + parameters are allowed"); + errored = true; + None + } + } + } + }).collect(); + // Report only if the format string checks out + if !errored { + report = Some(err); + } + } else { + infcx.tcx.sess.span_err(item.meta().span, + "this attribute must have a value, \ + eg `#[on_unimplemented = \"foo\"]`") + } + false + } else { + true + } + }); + report +} + pub fn report_selection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, obligation: &PredicateObligation<'tcx>, error: &SelectionError<'tcx>) @@ -88,12 +154,20 @@ pub fn report_selection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, infcx.resolve_type_vars_if_possible(trait_predicate); if !trait_predicate.references_error() { let trait_ref = trait_predicate.to_poly_trait_ref(); + // Check if it has a custom "#[on_unimplemented]" error message, + // report with that message if it does + let custom_note = report_on_unimplemented(infcx, &*trait_ref.0); infcx.tcx.sess.span_err( obligation.cause.span, format!( "the trait `{}` is not implemented for the type `{}`", trait_ref.user_string(infcx.tcx), trait_ref.self_ty().user_string(infcx.tcx)).as_slice()); + if let Some(s) = custom_note { + infcx.tcx.sess.span_note( + obligation.cause.span, + s.as_slice()); + } } } diff --git a/src/test/compile-fail/on-unimplemented.rs b/src/test/compile-fail/on-unimplemented.rs new file mode 100644 index 0000000000000..5a56e91cdda80 --- /dev/null +++ b/src/test/compile-fail/on-unimplemented.rs @@ -0,0 +1,51 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +// ignore-tidy-linelength + +#[on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] +trait Foo{} + +fn foobar>() -> T { + +} + +#[on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"] +trait MyFromIterator { + /// Build a container with elements from an external iterator. + fn my_from_iter>(iterator: T) -> Self; +} + +fn collect, B: MyFromIterator>(it: I) -> B { + MyFromIterator::my_from_iter(it) +} + +#[on_unimplemented] //~ ERROR this attribute must have a value +trait BadAnnotation1 {} + +#[on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] +//~^ ERROR there is no type parameter C on trait BadAnnotation2 +trait BadAnnotation2 {} + +fn trigger1(t: T) {} +fn trigger2>(t: T) {} + +pub fn main() { + let x = vec!(1u8, 2, 3, 4); + let y: Option> = collect(x.iter()); // this should give approximately the same error for x.iter().collect() + //~^ ERROR + //~^^ NOTE a collection of type `core::option::Option>` 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` + + // The following two have errors in their annotations, so the regular error should be thrown + trigger1(1u8); //~ ERROR the trait `BadAnnotation1` is not implemented for the type `u8` + trigger2::(1u8); //~ ERROR the trait `BadAnnotation2` is not implemented + +} From dc0de42035dc5c71e47ded2968fc6fd7c76641c6 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 11 Jan 2015 09:49:39 +0530 Subject: [PATCH 2/7] Add lint and test for malformed but unused #[on_unimplemented] attributes --- src/librustc/lint/builtin.rs | 63 +++++++++++++++++++ src/librustc/lint/context.rs | 1 + .../compile-fail/on-unimplemented-bad-anno.rs | 32 ++++++++++ 3 files changed, 96 insertions(+) create mode 100644 src/test/compile-fail/on-unimplemented-bad-anno.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 300b9aaf10060..29d37af30fc49 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -26,6 +26,9 @@ //! a `pub fn new()`. use self::MethodContext::*; + +use fmt_macros::{Parser, Piece, Position}; + use metadata::csearch; use middle::def::*; use middle::subst::Substs; @@ -1921,3 +1924,63 @@ impl LintPass for UnstableFeatures { } } } + +/// Checks usage of `#[on_unimplemented]` +#[derive(Copy)] +pub struct BadOnUnimplemented; + +declare_lint!(BAD_ON_UNIMPLEMENTED, Deny, + "Checks usage of `#[on_unimplemented]`"); + +impl LintPass for BadOnUnimplemented { + fn get_lints(&self) -> LintArray { + lint_array!(BAD_ON_UNIMPLEMENTED) + } + fn check_item(&mut self, ctx: &Context, item: &ast::Item) { + match item.node { + ast::ItemTrait(_, ref generics, _, _) => { + if let Some(ref attr) = item.attrs.iter().find(|&: a| { + a.check_name("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 => { + ctx.span_lint(BAD_ON_UNIMPLEMENTED, 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 => { + ctx.span_lint(BAD_ON_UNIMPLEMENTED, attr.span, + "only named substitution \ + parameters are allowed"); + } + } + } + } + } else { + ctx.span_lint(BAD_ON_UNIMPLEMENTED, attr.span, + "this attribute must have a value, \ + eg `#[on_unimplemented = \"foo\"]`") + } + } + }, + _ => () // Not a trait def, move along + } + } +} \ No newline at end of file diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 95e1e8d44bfc5..b50c505a3aced 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -211,6 +211,7 @@ impl LintStore { UnusedAllocation, MissingCopyImplementations, UnstableFeatures, + BadOnUnimplemented, ); add_builtin_with_new!(sess, diff --git a/src/test/compile-fail/on-unimplemented-bad-anno.rs b/src/test/compile-fail/on-unimplemented-bad-anno.rs new file mode 100644 index 0000000000000..7ca228a69b8b1 --- /dev/null +++ b/src/test/compile-fail/on-unimplemented-bad-anno.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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +// ignore-tidy-linelength + +#[allow(unused)] + +#[on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] +trait Foo{} + +#[on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"] +trait MyFromIterator { + /// Build a container with elements from an external iterator. + fn my_from_iter>(iterator: T) -> Self; +} + +#[on_unimplemented] //~ ERROR this attribute must have a value +trait BadAnnotation1 {} + +#[on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] +//~^ ERROR there is no type parameter C on trait BadAnnotation2 +trait BadAnnotation2 {} + + +pub fn main() { +} From e1832779483df51b59367250850213d8e7fe159a Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 11 Jan 2015 16:41:02 +0530 Subject: [PATCH 3/7] Make errors allow for cross-crate issues --- src/librustc/middle/traits/error_reporting.rs | 48 ++++++++++++------- src/test/compile-fail/on-unimplemented.rs | 4 +- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/librustc/middle/traits/error_reporting.rs b/src/librustc/middle/traits/error_reporting.rs index 8903667505b32..0b0079c062db3 100644 --- a/src/librustc/middle/traits/error_reporting.rs +++ b/src/librustc/middle/traits/error_reporting.rs @@ -22,7 +22,7 @@ use fmt_macros::{Parser, Piece, Position}; use middle::infer::InferCtxt; use middle::ty::{self, AsPredicate, ReferencesError, ToPolyTraitRef, TraitRef}; use std::collections::HashMap; -use syntax::codemap::Span; +use syntax::codemap::{DUMMY_SP, Span}; use syntax::attr::{AttributeMethods, AttrMetaMethods}; use util::ppaux::{Repr, UserString}; @@ -66,13 +66,20 @@ 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>) -> Option { + trait_ref: &TraitRef<'tcx>, + span: Span) -> Option { let def_id = trait_ref.def_id; let mut report = None; ty::each_attr(infcx.tcx, def_id, |item| { if item.check_name("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 def = ty::lookup_trait_def(infcx.tcx, def_id); let mut generic_map = def.generics.types.iter_enumerated() .map(|(param, i, gen)| { (gen.name.as_str().to_string(), @@ -91,20 +98,24 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, Some(val) => Some(val.as_slice()), None => { infcx.tcx.sess - .span_err(item.meta().span, - format!("there is no type parameter \ - {} on trait {}", - s, def.trait_ref - .user_string(infcx.tcx)) + .span_err(err_sp, + format!("the #[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(item.meta().span, - "only named substitution \ - parameters are allowed"); + infcx.tcx.sess + .span_err(err_sp, + format!("the #[on_unimplemented] attribute on \ + trait definition for {} must have named \ + format arguments, \ + eg `#[on_unimplemented = \"foo {{T}}\"]`", + trait_str).as_slice()); errored = true; None } @@ -116,9 +127,11 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, report = Some(err); } } else { - infcx.tcx.sess.span_err(item.meta().span, - "this attribute must have a value, \ - eg `#[on_unimplemented = \"foo\"]`") + infcx.tcx.sess.span_err(err_sp, + format!("the #[on_unimplemented] attribute on \ + trait definition for {} must have a value, \ + eg `#[on_unimplemented = \"foo\"]`", + trait_str).as_slice()); } false } else { @@ -154,15 +167,16 @@ pub fn report_selection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, infcx.resolve_type_vars_if_possible(trait_predicate); if !trait_predicate.references_error() { let trait_ref = trait_predicate.to_poly_trait_ref(); - // Check if it has a custom "#[on_unimplemented]" error message, - // report with that message if it does - let custom_note = report_on_unimplemented(infcx, &*trait_ref.0); infcx.tcx.sess.span_err( obligation.cause.span, format!( "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 "#[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, diff --git a/src/test/compile-fail/on-unimplemented.rs b/src/test/compile-fail/on-unimplemented.rs index 5a56e91cdda80..3f899288f50d2 100644 --- a/src/test/compile-fail/on-unimplemented.rs +++ b/src/test/compile-fail/on-unimplemented.rs @@ -26,11 +26,11 @@ fn collect, B: MyFromIterator>(it: I) -> B { MyFromIterator::my_from_iter(it) } -#[on_unimplemented] //~ ERROR this attribute must have a value +#[on_unimplemented] //~ ERROR the #[on_unimplemented] attribute on trait definition for BadAnnotation1 must have a value, eg `#[on_unimplemented = "foo"]` trait BadAnnotation1 {} #[on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] -//~^ ERROR there is no type parameter C on trait BadAnnotation2 +//~^ ERROR the #[on_unimplemented] attribute on trait definition for BadAnnotation2 refers to non-existent type parameter C trait BadAnnotation2 {} fn trigger1(t: T) {} From add20bbb6dae07fddc80bc46f53296fbf5679f27 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 11 Jan 2015 18:17:46 +0530 Subject: [PATCH 4/7] Move error to typeck::check --- src/librustc/lint/builtin.rs | 63 ----------------------- src/librustc/lint/context.rs | 1 - src/librustc_typeck/check/mod.rs | 50 +++++++++++++++++- src/librustc_typeck/lib.rs | 1 + src/test/compile-fail/on-unimplemented.rs | 15 ------ 5 files changed, 50 insertions(+), 80 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 29d37af30fc49..300b9aaf10060 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -26,9 +26,6 @@ //! a `pub fn new()`. use self::MethodContext::*; - -use fmt_macros::{Parser, Piece, Position}; - use metadata::csearch; use middle::def::*; use middle::subst::Substs; @@ -1924,63 +1921,3 @@ impl LintPass for UnstableFeatures { } } } - -/// Checks usage of `#[on_unimplemented]` -#[derive(Copy)] -pub struct BadOnUnimplemented; - -declare_lint!(BAD_ON_UNIMPLEMENTED, Deny, - "Checks usage of `#[on_unimplemented]`"); - -impl LintPass for BadOnUnimplemented { - fn get_lints(&self) -> LintArray { - lint_array!(BAD_ON_UNIMPLEMENTED) - } - fn check_item(&mut self, ctx: &Context, item: &ast::Item) { - match item.node { - ast::ItemTrait(_, ref generics, _, _) => { - if let Some(ref attr) = item.attrs.iter().find(|&: a| { - a.check_name("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 => { - ctx.span_lint(BAD_ON_UNIMPLEMENTED, 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 => { - ctx.span_lint(BAD_ON_UNIMPLEMENTED, attr.span, - "only named substitution \ - parameters are allowed"); - } - } - } - } - } else { - ctx.span_lint(BAD_ON_UNIMPLEMENTED, attr.span, - "this attribute must have a value, \ - eg `#[on_unimplemented = \"foo\"]`") - } - } - }, - _ => () // Not a trait def, move along - } - } -} \ No newline at end of file diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index b50c505a3aced..95e1e8d44bfc5 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -211,7 +211,6 @@ impl LintStore { UnusedAllocation, MissingCopyImplementations, UnstableFeatures, - BadOnUnimplemented, ); add_builtin_with_new!(sess, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 1d184131dede3..de6c884d6d71b 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/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; @@ -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}; @@ -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 { @@ -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("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 `#[on_unimplemented = \"foo\"]`") + } + } +} + /// Type checks a method body. /// /// # Parameters diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index f15d53290e761..68b152dee233b 100644 --- a/src/librustc_typeck/lib.rs +++ b/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; diff --git a/src/test/compile-fail/on-unimplemented.rs b/src/test/compile-fail/on-unimplemented.rs index 3f899288f50d2..20b4448ea7772 100644 --- a/src/test/compile-fail/on-unimplemented.rs +++ b/src/test/compile-fail/on-unimplemented.rs @@ -26,16 +26,6 @@ fn collect, B: MyFromIterator>(it: I) -> B { MyFromIterator::my_from_iter(it) } -#[on_unimplemented] //~ ERROR the #[on_unimplemented] attribute on trait definition for BadAnnotation1 must have a value, eg `#[on_unimplemented = "foo"]` -trait BadAnnotation1 {} - -#[on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] -//~^ ERROR the #[on_unimplemented] attribute on trait definition for BadAnnotation2 refers to non-existent type parameter C -trait BadAnnotation2 {} - -fn trigger1(t: T) {} -fn trigger2>(t: T) {} - pub fn main() { let x = vec!(1u8, 2, 3, 4); let y: Option> = collect(x.iter()); // this should give approximately the same error for x.iter().collect() @@ -43,9 +33,4 @@ pub fn main() { //~^^ NOTE a collection of type `core::option::Option>` 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` - - // The following two have errors in their annotations, so the regular error should be thrown - trigger1(1u8); //~ ERROR the trait `BadAnnotation1` is not implemented for the type `u8` - trigger2::(1u8); //~ ERROR the trait `BadAnnotation2` is not implemented - } From dd074ab4eedcb08ca8a1861fc1e62f125e23fa8b Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 11 Jan 2015 18:28:06 +0530 Subject: [PATCH 5/7] Rename #[on_unimplemented] -> #[rustc_on_unimplemented] --- src/libcore/iter.rs | 4 ++-- src/librustc/lint/builtin.rs | 2 +- src/librustc/middle/traits/error_reporting.rs | 17 ++++++++++------- src/librustc_typeck/check/mod.rs | 4 ++-- .../compile-fail/on-unimplemented-bad-anno.rs | 8 ++++---- src/test/compile-fail/on-unimplemented.rs | 4 ++-- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index e22de10a974f0..c88f3553e6b55 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -101,8 +101,8 @@ pub trait Iterator { /// Conversion from an `Iterator` #[stable] -#[on_unimplemented="a collection of type `{Self}` cannot be \ - built from an iterator over elements of type `{A}`"] +#[rustc_on_unimplemented="a collection of type `{Self}` cannot be \ + built from an iterator over elements of type `{A}`"] pub trait FromIterator { /// Build a container with elements from an external iterator. fn from_iter>(iterator: T) -> Self; diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 300b9aaf10060..8ed177c82a8f7 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -666,7 +666,7 @@ impl LintPass for UnusedAttributes { "must_use", "stable", "unstable", - "on_unimplemented", + "rustc_on_unimplemented", // FIXME: #19470 this shouldn't be needed forever "old_orphan_check", diff --git a/src/librustc/middle/traits/error_reporting.rs b/src/librustc/middle/traits/error_reporting.rs index 0b0079c062db3..da3b5f635e13b 100644 --- a/src/librustc/middle/traits/error_reporting.rs +++ b/src/librustc/middle/traits/error_reporting.rs @@ -71,7 +71,7 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, let def_id = trait_ref.def_id; let mut report = None; ty::each_attr(infcx.tcx, def_id, |item| { - if item.check_name("on_unimplemented") { + if item.check_name("rustc_on_unimplemented") { let err_sp = if item.meta().span == DUMMY_SP { span } else { @@ -99,7 +99,8 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, None => { infcx.tcx.sess .span_err(err_sp, - format!("the #[on_unimplemented] attribute on \ + format!("the #[rustc_on_unimplemented] \ + attribute on \ trait definition for {} refers to \ non-existent type parameter {}", trait_str, s) @@ -111,10 +112,12 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, _ => { infcx.tcx.sess .span_err(err_sp, - format!("the #[on_unimplemented] attribute on \ + format!("the #[rustc_on_unimplemented] \ + attribute on \ trait definition for {} must have named \ format arguments, \ - eg `#[on_unimplemented = \"foo {{T}}\"]`", + eg `#[rustc_on_unimplemented = \ + \"foo {{T}}\"]`", trait_str).as_slice()); errored = true; None @@ -128,9 +131,9 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, } } else { infcx.tcx.sess.span_err(err_sp, - format!("the #[on_unimplemented] attribute on \ + format!("the #[rustc_on_unimplemented] attribute on \ trait definition for {} must have a value, \ - eg `#[on_unimplemented = \"foo\"]`", + eg `#[rustc_on_unimplemented = \"foo\"]`", trait_str).as_slice()); } false @@ -173,7 +176,7 @@ 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 "#[on_unimplemented]" error message, + // 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); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index de6c884d6d71b..7513d50deeac0 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -783,7 +783,7 @@ 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("on_unimplemented") + a.check_name("rustc_on_unimplemented") }) { if let Some(ref istring) = attr.value_str() { let mut parser = Parser::new(istring.get()); @@ -819,7 +819,7 @@ fn check_trait_on_unimplemented<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, } else { ccx.tcx.sess.span_err(attr.span, "this attribute must have a value, \ - eg `#[on_unimplemented = \"foo\"]`") + eg `#[rustc_on_unimplemented = \"foo\"]`") } } } diff --git a/src/test/compile-fail/on-unimplemented-bad-anno.rs b/src/test/compile-fail/on-unimplemented-bad-anno.rs index 7ca228a69b8b1..ec825c1304331 100644 --- a/src/test/compile-fail/on-unimplemented-bad-anno.rs +++ b/src/test/compile-fail/on-unimplemented-bad-anno.rs @@ -11,19 +11,19 @@ #[allow(unused)] -#[on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] +#[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] trait Foo{} -#[on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"] +#[rustc_on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"] trait MyFromIterator { /// Build a container with elements from an external iterator. fn my_from_iter>(iterator: T) -> Self; } -#[on_unimplemented] //~ ERROR this attribute must have a value +#[rustc_on_unimplemented] //~ ERROR this attribute must have a value trait BadAnnotation1 {} -#[on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{C}>`"] +#[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 {} diff --git a/src/test/compile-fail/on-unimplemented.rs b/src/test/compile-fail/on-unimplemented.rs index 20b4448ea7772..7d579fb340a98 100644 --- a/src/test/compile-fail/on-unimplemented.rs +++ b/src/test/compile-fail/on-unimplemented.rs @@ -9,14 +9,14 @@ // except according to those terms. // ignore-tidy-linelength -#[on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] +#[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] trait Foo{} fn foobar>() -> T { } -#[on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"] +#[rustc_on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"] trait MyFromIterator { /// Build a container with elements from an external iterator. fn my_from_iter>(iterator: T) -> Self; From ad7e33efee023822bd63fa6f942c25582ef12a1e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 11 Jan 2015 20:53:24 +0530 Subject: [PATCH 6/7] Feature gate #[rustc_on_unimplemented] --- src/libcore/lib.rs | 1 + src/librustc/middle/traits/error_reporting.rs | 5 ++--- src/libsyntax/feature_gate.rs | 5 +++++ src/test/compile-fail/on-unimplemented-bad-anno.rs | 4 +++- src/test/compile-fail/on-unimplemented.rs | 2 ++ 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index 28f8cf588bea9..78e8a2a9e9164 100644 --- a/src/libcore/lib.rs +++ b/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] diff --git a/src/librustc/middle/traits/error_reporting.rs b/src/librustc/middle/traits/error_reporting.rs index da3b5f635e13b..6b4dd101286d6 100644 --- a/src/librustc/middle/traits/error_reporting.rs +++ b/src/librustc/middle/traits/error_reporting.rs @@ -181,9 +181,8 @@ pub fn report_selection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, 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()); + infcx.tcx.sess.span_note(obligation.cause.span, + s.as_slice()); } } } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 8175c0a9eecf4..8929bbe0232ed 100644 --- a/src/libsyntax/feature_gate.rs +++ b/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 { diff --git a/src/test/compile-fail/on-unimplemented-bad-anno.rs b/src/test/compile-fail/on-unimplemented-bad-anno.rs index ec825c1304331..3bd3f517dbccd 100644 --- a/src/test/compile-fail/on-unimplemented-bad-anno.rs +++ b/src/test/compile-fail/on-unimplemented-bad-anno.rs @@ -9,7 +9,9 @@ // except according to those terms. // ignore-tidy-linelength -#[allow(unused)] +#![feature(on_unimplemented)] + +#![allow(unused)] #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] trait Foo{} diff --git a/src/test/compile-fail/on-unimplemented.rs b/src/test/compile-fail/on-unimplemented.rs index 7d579fb340a98..7b406afcf1f58 100644 --- a/src/test/compile-fail/on-unimplemented.rs +++ b/src/test/compile-fail/on-unimplemented.rs @@ -9,6 +9,8 @@ // except according to those terms. // ignore-tidy-linelength +#![feature(on_unimplemented)] + #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}`"] trait Foo{} From 02d0a8bbcf0a64339e4279c4ddb4841189ba5069 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 12 Jan 2015 01:35:16 +0530 Subject: [PATCH 7/7] docs --- src/doc/reference.md | 7 +++++++ src/test/compile-fail/on-unimplemented-bad-anno.rs | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/doc/reference.md b/src/doc/reference.md index 2486466c8696d..62b922ea4beac 100644 --- a/src/doc/reference.md +++ b/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 diff --git a/src/test/compile-fail/on-unimplemented-bad-anno.rs b/src/test/compile-fail/on-unimplemented-bad-anno.rs index 3bd3f517dbccd..dda534cc489b5 100644 --- a/src/test/compile-fail/on-unimplemented-bad-anno.rs +++ b/src/test/compile-fail/on-unimplemented-bad-anno.rs @@ -29,6 +29,9 @@ trait BadAnnotation1 {} //~^ ERROR there is no type parameter C on trait BadAnnotation2 trait BadAnnotation2 {} +#[rustc_on_unimplemented = "Unimplemented trait error on `{Self}` with params `<{A},{B},{}>`"] +//~^ only named substitution parameters are allowed +trait BadAnnotation3 {} pub fn main() { }