From d8406a3fb27305b69d7dec377686414977c05555 Mon Sep 17 00:00:00 2001 From: ggomez Date: Thu, 21 Jan 2016 10:57:21 +0100 Subject: [PATCH 1/2] Extend rustc_on_unimplemented flag: if a message is available at an impl, this message will be displayed instead --- src/libcore/lib.rs | 1 + src/librustc/middle/infer/mod.rs | 2 +- src/librustc/middle/infer/type_variable.rs | 2 +- src/librustc/middle/subst.rs | 1 - src/librustc/middle/traits/error_reporting.rs | 124 ++++++++++++++---- src/librustc/middle/traits/project.rs | 8 +- .../compile-fail/check_on_unimplemented.rs | 34 +++++ .../check_on_unimplemented_on_slice.rs | 20 +++ 8 files changed, 163 insertions(+), 29 deletions(-) create mode 100644 src/test/compile-fail/check_on_unimplemented.rs create mode 100644 src/test/compile-fail/check_on_unimplemented_on_slice.rs diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index f76b8655ad1ed..6d2922b28048f 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -43,6 +43,7 @@ // Since libcore defines many fundamental lang items, all tests live in a // separate crate, libcoretest, to avoid bizarre issues. +#![cfg_attr(stage0, allow(unused_attributes))] #![crate_name = "core"] #![stable(feature = "core", since = "1.6.0")] #![crate_type = "rlib"] diff --git a/src/librustc/middle/infer/mod.rs b/src/librustc/middle/infer/mod.rs index 1e3546269dbc2..1699065aea743 100644 --- a/src/librustc/middle/infer/mod.rs +++ b/src/librustc/middle/infer/mod.rs @@ -104,7 +104,7 @@ pub struct InferCtxt<'a, 'tcx: 'a> { /// A map returned by `skolemize_late_bound_regions()` indicating the skolemized /// region that each late-bound region was replaced with. -pub type SkolemizationMap = FnvHashMap; +pub type SkolemizationMap = FnvHashMap; /// Why did we require that the two types be related? /// diff --git a/src/librustc/middle/infer/type_variable.rs b/src/librustc/middle/infer/type_variable.rs index e4af098c2a42d..ec3fe97b85db3 100644 --- a/src/librustc/middle/infer/type_variable.rs +++ b/src/librustc/middle/infer/type_variable.rs @@ -127,7 +127,7 @@ impl<'tcx> TypeVariableTable<'tcx> { let (relations, default) = match old_value { Bounded { relations, default } => (relations, default), Known(_) => panic!("Asked to instantiate variable that is \ - already instantiated") + already instantiated") }; for &(dir, vid) in &relations { diff --git a/src/librustc/middle/subst.rs b/src/librustc/middle/subst.rs index 02dfeb80b928f..dd0e22acc2e0b 100644 --- a/src/librustc/middle/subst.rs +++ b/src/librustc/middle/subst.rs @@ -175,7 +175,6 @@ impl<'tcx> Substs<'tcx> { } impl<'tcx> Encodable for Substs<'tcx> { - fn encode(&self, s: &mut S) -> Result<(), S::Error> { cstore::tls::with_encoding_context(s, |ecx, rbml_w| { ecx.encode_substs(rbml_w, self); diff --git a/src/librustc/middle/traits/error_reporting.rs b/src/librustc/middle/traits/error_reporting.rs index dd051471a4d1b..c0c99abe06a42 100644 --- a/src/librustc/middle/traits/error_reporting.rs +++ b/src/librustc/middle/traits/error_reporting.rs @@ -25,13 +25,15 @@ use super::{ use fmt_macros::{Parser, Piece, Position}; use middle::def_id::DefId; -use middle::infer::InferCtxt; +use middle::infer::{self, InferCtxt, TypeOrigin}; use middle::ty::{self, ToPredicate, ToPolyTraitRef, TraitRef, Ty, TyCtxt, TypeFoldable}; use middle::ty::fast_reject; +use middle::subst::{self, Subst}; use util::nodemap::{FnvHashMap, FnvHashSet}; use std::cmp; use std::fmt; +use syntax::ast; use syntax::attr::{AttributeMethods, AttrMetaMethods}; use syntax::codemap::Span; use syntax::errors::DiagnosticBuilder; @@ -105,15 +107,93 @@ pub fn report_projection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, } } +fn impl_self_ty<'a, 'tcx>(fcx: &InferCtxt<'a, 'tcx>, + did: DefId, + obligation: PredicateObligation<'tcx>) + -> subst::Substs<'tcx> { + let tcx = fcx.tcx; + + let ity = tcx.lookup_item_type(did); + let (tps, rps, _) = + (ity.generics.types.get_slice(subst::TypeSpace), + ity.generics.regions.get_slice(subst::TypeSpace), + ity.ty); + + let rps = fcx.region_vars_for_defs(obligation.cause.span, rps); + let mut substs = subst::Substs::new( + subst::VecPerParamSpace::empty(), + subst::VecPerParamSpace::new(rps, Vec::new(), Vec::new())); + fcx.type_vars_for_defs(obligation.cause.span, subst::ParamSpace::TypeSpace, &mut substs, tps); + substs +} + +fn get_current_failing_impl<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, + trait_ref: &TraitRef<'tcx>, + obligation: &PredicateObligation<'tcx>) + -> Option { + let simp = fast_reject::simplify_type(infcx.tcx, + trait_ref.self_ty(), + true); + let trait_def = infcx.tcx.lookup_trait_def(trait_ref.def_id); + + match simp { + Some(_) => { + let mut ret = None; + trait_def.for_each_impl(infcx.tcx, |def_id| { + let imp = infcx.tcx.impl_trait_ref(def_id).unwrap(); + let imp = imp.subst(infcx.tcx, &impl_self_ty(infcx, def_id, obligation.clone())); + if ret.is_none() { + for error in infcx.reported_trait_errors.borrow().iter() { + if let ty::Predicate::Trait(ref t) = error.predicate { + if infer::mk_eqty(infcx, true, TypeOrigin::Misc(obligation.cause.span), + t.skip_binder().trait_ref.self_ty(), + imp.self_ty()).is_ok() { + ret = Some(def_id); + break; + } + } + } + } + }); + ret + }, + None => None, + } +} + +fn find_attr<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, + def_id: DefId, + attr_name: &str) + -> Option { + for item in infcx.tcx.get_attrs(def_id).iter() { + if item.check_name(attr_name) { + return Some(item.clone()); + } + } + None +} + fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, trait_ref: &TraitRef<'tcx>, - span: Span) -> Option { - let def_id = trait_ref.def_id; + obligation: &PredicateObligation<'tcx>) + -> Option { + let def_id = match get_current_failing_impl(infcx, trait_ref, obligation) { + Some(def_id) => { + if let Some(_) = find_attr(infcx, def_id, "rustc_on_unimplemented") { + def_id + } else { + trait_ref.def_id + } + }, + None => trait_ref.def_id, + }; + let span = obligation.cause.span; let mut report = None; + for item in infcx.tcx.get_attrs(def_id).iter() { if item.check_name("rustc_on_unimplemented") { let err_sp = item.meta().span.substitute_dummy(span); - let def = infcx.tcx.lookup_trait_def(def_id); + let def = infcx.tcx.lookup_trait_def(trait_ref.def_id); let trait_str = def.trait_ref.to_string(); if let Some(ref istring) = item.value_str() { let mut generic_map = def.generics.types.iter_enumerated() @@ -134,24 +214,24 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, Some(val) => Some(val), None => { span_err!(infcx.tcx.sess, err_sp, E0272, - "the #[rustc_on_unimplemented] \ - attribute on \ - trait definition for {} refers to \ - non-existent type parameter {}", - trait_str, s); + "the #[rustc_on_unimplemented] \ + attribute on \ + trait definition for {} refers to \ + non-existent type parameter {}", + trait_str, s); errored = true; None } }, _ => { - span_err!(infcx.tcx.sess, err_sp, E0273, - "the #[rustc_on_unimplemented] \ - attribute on \ - trait definition for {} must have named \ - format arguments, \ - eg `#[rustc_on_unimplemented = \ - \"foo {{T}}\"]`", - trait_str); + span_err!(infcx.tcx.sess, err_sp, E0273, + "the #[rustc_on_unimplemented] \ + attribute on \ + trait definition for {} must have named \ + format arguments, \ + eg `#[rustc_on_unimplemented = \ + \"foo {{T}}\"]`", + trait_str); errored = true; None } @@ -164,10 +244,10 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, } } else { span_err!(infcx.tcx.sess, err_sp, E0274, - "the #[rustc_on_unimplemented] attribute on \ - trait definition for {} must have a value, \ - eg `#[rustc_on_unimplemented = \"foo\"]`", - trait_str); + "the #[rustc_on_unimplemented] attribute on \ + trait definition for {} must have a value, \ + eg `#[rustc_on_unimplemented = \"foo\"]`", + trait_str); } break; } @@ -368,7 +448,7 @@ pub fn report_selection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, // 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); + obligation); if let Some(s) = custom_note { err.fileline_note(obligation.cause.span, &s); } else { diff --git a/src/librustc/middle/traits/project.rs b/src/librustc/middle/traits/project.rs index e36307feddbf7..0e692d32527d6 100644 --- a/src/librustc/middle/traits/project.rs +++ b/src/librustc/middle/traits/project.rs @@ -198,10 +198,10 @@ fn consider_unification_despite_ambiguity<'cx,'tcx>(selcx: &mut SelectionContext /// them with a fully resolved type where possible. The return value /// combines the normalized result and any additional obligations that /// were incurred as result. -pub fn normalize<'a,'b,'tcx,T>(selcx: &'a mut SelectionContext<'b,'tcx>, - cause: ObligationCause<'tcx>, - value: &T) - -> Normalized<'tcx, T> +pub fn normalize<'a, 'b, 'tcx, T>(selcx: &'a mut SelectionContext<'b,'tcx>, + cause: ObligationCause<'tcx>, + value: &T) + -> Normalized<'tcx, T> where T : TypeFoldable<'tcx> { normalize_with_depth(selcx, cause, 0, value) diff --git a/src/test/compile-fail/check_on_unimplemented.rs b/src/test/compile-fail/check_on_unimplemented.rs new file mode 100644 index 0000000000000..042fdb070f42f --- /dev/null +++ b/src/test/compile-fail/check_on_unimplemented.rs @@ -0,0 +1,34 @@ +// Copyright 2016 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. + +// Test if the on_unimplemented message override works + +#![feature(on_unimplemented)] +#![feature(rustc_attrs)] + +#[rustc_on_unimplemented = "invalid"] +trait Index { + type Output: ?Sized; + fn index(&self, index: Idx) -> &Self::Output; +} + +#[rustc_on_unimplemented = "a usize is required to index into a slice"] +impl Index for [i32] { + type Output = i32; + fn index(&self, index: usize) -> &i32 { + &self[index] + } +} + +#[rustc_error] +fn main() { + Index::::index(&[1, 2, 3] as &[i32], 2u32); //~ ERROR E0277 + //~| NOTE a usize is required +} diff --git a/src/test/compile-fail/check_on_unimplemented_on_slice.rs b/src/test/compile-fail/check_on_unimplemented_on_slice.rs new file mode 100644 index 0000000000000..d594b1cea8bce --- /dev/null +++ b/src/test/compile-fail/check_on_unimplemented_on_slice.rs @@ -0,0 +1,20 @@ +// Copyright 2016 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. + +// Test new Index error message for slices + +#![feature(rustc_attrs)] + +#[rustc_error] +fn main() { + let x = &[1, 2, 3] as &[i32]; + x[1i32]; //~ ERROR E0277 + //~| NOTE a usize is required +} From de2fb72d1f21a56238f50a206cd2bf4ad9459f7d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 23 Jan 2016 06:44:38 +0100 Subject: [PATCH 2/2] Improve error message for Index trait on slices --- src/libcore/slice.rs | 4 ++ src/librustc/middle/traits/error_reporting.rs | 64 ++++++++++++++----- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index 8acd0c8f2cf06..d99f6a8347da7 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -503,6 +503,8 @@ impl SliceExt for [T] { } #[stable(feature = "rust1", since = "1.0.0")] +#[allow(unused_attributes)] +#[rustc_on_unimplemented = "a usize is required to index into a slice"] impl ops::Index for [T] { type Output = T; @@ -513,6 +515,8 @@ impl ops::Index for [T] { } #[stable(feature = "rust1", since = "1.0.0")] +#[allow(unused_attributes)] +#[rustc_on_unimplemented = "a usize is required to index into a slice"] impl ops::IndexMut for [T] { #[inline] fn index_mut(&mut self, index: usize) -> &mut T { diff --git a/src/librustc/middle/traits/error_reporting.rs b/src/librustc/middle/traits/error_reporting.rs index c0c99abe06a42..8e011f3f599e2 100644 --- a/src/librustc/middle/traits/error_reporting.rs +++ b/src/librustc/middle/traits/error_reporting.rs @@ -107,7 +107,7 @@ pub fn report_projection_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, } } -fn impl_self_ty<'a, 'tcx>(fcx: &InferCtxt<'a, 'tcx>, +fn impl_substs<'a, 'tcx>(fcx: &InferCtxt<'a, 'tcx>, did: DefId, obligation: PredicateObligation<'tcx>) -> subst::Substs<'tcx> { @@ -127,10 +127,35 @@ fn impl_self_ty<'a, 'tcx>(fcx: &InferCtxt<'a, 'tcx>, substs } +fn check_type_parameters<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, + trait_substs: &subst::Substs<'tcx>, + impl_substs: &subst::Substs<'tcx>, + obligation: &PredicateObligation<'tcx>) -> bool { + let trait_types = trait_substs.types.as_slice(); + let impl_types = impl_substs.types.as_slice(); + + let mut failed = 0; + for index_to_ignore in 0..trait_types.len() { + for (index, (trait_type, impl_type)) in trait_types.iter() + .zip(impl_types.iter()) + .enumerate() { + if index_to_ignore != index && + infer::mk_eqty(infcx, true, + TypeOrigin::Misc(obligation.cause.span), + trait_type, + impl_type).is_err() { + failed += 1; + break; + } + } + } + failed == trait_types.len() - 1 +} + fn get_current_failing_impl<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, trait_ref: &TraitRef<'tcx>, obligation: &PredicateObligation<'tcx>) - -> Option { + -> Option<(DefId, subst::Substs<'tcx>)> { let simp = fast_reject::simplify_type(infcx.tcx, trait_ref.self_ty(), true); @@ -138,24 +163,29 @@ fn get_current_failing_impl<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, match simp { Some(_) => { - let mut ret = None; + let mut matching_impls = Vec::new(); trait_def.for_each_impl(infcx.tcx, |def_id| { let imp = infcx.tcx.impl_trait_ref(def_id).unwrap(); - let imp = imp.subst(infcx.tcx, &impl_self_ty(infcx, def_id, obligation.clone())); - if ret.is_none() { - for error in infcx.reported_trait_errors.borrow().iter() { - if let ty::Predicate::Trait(ref t) = error.predicate { - if infer::mk_eqty(infcx, true, TypeOrigin::Misc(obligation.cause.span), - t.skip_binder().trait_ref.self_ty(), - imp.self_ty()).is_ok() { - ret = Some(def_id); - break; - } - } + let substs = impl_substs(infcx, def_id, obligation.clone()); + let imp = imp.subst(infcx.tcx, &substs); + + if infer::mk_eqty(infcx, true, + TypeOrigin::Misc(obligation.cause.span), + trait_ref.self_ty(), + imp.self_ty()).is_ok() { + if check_type_parameters(infcx, &trait_ref.substs, &imp.substs, obligation) { + matching_impls.push((def_id, imp.substs.clone())); } } }); - ret + if matching_impls.len() == 0 { + None + } else if matching_impls.len() == 1 { + Some(matching_impls[0].clone()) + } else { + // we need to determine which type is the good one! + Some(matching_impls[0].clone()) + } }, None => None, } @@ -178,14 +208,14 @@ fn report_on_unimplemented<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, obligation: &PredicateObligation<'tcx>) -> Option { let def_id = match get_current_failing_impl(infcx, trait_ref, obligation) { - Some(def_id) => { + Some((def_id, _)) => { if let Some(_) = find_attr(infcx, def_id, "rustc_on_unimplemented") { def_id } else { trait_ref.def_id } }, - None => trait_ref.def_id, + None => trait_ref.def_id, }; let span = obligation.cause.span; let mut report = None;