From cea88ebb39402ceee9ec5f7cd61c877ae4cd16dc Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 16 Jul 2016 23:18:20 +0300 Subject: [PATCH] refactor type error reporting --- src/librustc/infer/error_reporting.rs | 233 +++++++------------- src/librustc/infer/mod.rs | 48 ++-- src/librustc/macros.rs | 12 + src/librustc_typeck/check/callee.rs | 2 +- src/librustc_typeck/check/cast.rs | 15 +- src/librustc_typeck/check/method/suggest.rs | 3 +- src/librustc_typeck/check/mod.rs | 27 +-- src/librustc_typeck/check/op.rs | 2 +- 8 files changed, 140 insertions(+), 202 deletions(-) diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs index a0fa188c4f8..be73818c8a4 100644 --- a/src/librustc/infer/error_reporting.rs +++ b/src/librustc/infer/error_reporting.rs @@ -83,7 +83,7 @@ use hir::def_id::DefId; use infer::{self, TypeOrigin}; use middle::region; use ty::subst; -use ty::{self, Ty, TyCtxt, TypeFoldable}; +use ty::{self, TyCtxt, TypeFoldable}; use ty::{Region, ReFree}; use ty::error::TypeError; @@ -462,52 +462,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } - fn report_type_error(&self, - trace: TypeTrace<'tcx>, - terr: &TypeError<'tcx>) - -> DiagnosticBuilder<'tcx> { - let (expected, found) = match self.values_str(&trace.values) { - Some(v) => v, - None => { - return self.tcx.sess.diagnostic().struct_dummy(); /* derived error */ - } - }; - - let is_simple_error = if let &TypeError::Sorts(ref values) = terr { - values.expected.is_primitive() && values.found.is_primitive() - } else { - false - }; - - let mut err = struct_span_err!(self.tcx.sess, - trace.origin.span(), - E0308, - "{}", - trace.origin); - - if !is_simple_error || check_old_school() { - err.note_expected_found(&"type", &expected, &found); - } - - err.span_label(trace.origin.span(), &terr); - - self.check_and_note_conflicting_crates(&mut err, terr, trace.origin.span()); - - match trace.origin { - TypeOrigin::MatchExpressionArm(_, arm_span, source) => match source { - hir::MatchSource::IfLetDesugar{..} => { - err.span_note(arm_span, "`if let` arm with an incompatible type"); - } - _ => { - err.span_note(arm_span, "match arm with an incompatible type"); - } - }, - _ => () - } - - err - } - /// Adds a note if the types come from similarly named crates fn check_and_note_conflicting_crates(&self, err: &mut DiagnosticBuilder, @@ -550,43 +504,91 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } - pub fn report_and_explain_type_error(&self, - trace: TypeTrace<'tcx>, - terr: &TypeError<'tcx>) - -> DiagnosticBuilder<'tcx> { - let trace = self.resolve_type_vars_if_possible(&trace); + fn note_error_origin(&self, + err: &mut DiagnosticBuilder<'tcx>, + origin: &TypeOrigin) + { + match origin { + &TypeOrigin::MatchExpressionArm(_, arm_span, source) => match source { + hir::MatchSource::IfLetDesugar {..} => { + err.span_note(arm_span, "`if let` arm with an incompatible type"); + } + _ => { + err.span_note(arm_span, "match arm with an incompatible type"); + } + }, + _ => () + } + } + + pub fn report_and_explain_type_error_with_code(&self, + trace: TypeTrace<'tcx>, + terr: &TypeError<'tcx>, + message: &str, + code: &str) + -> DiagnosticBuilder<'tcx> + { + let (expected, found) = match self.values_str(&trace.values) { + Some((expected, found)) => (expected, found), + None => return self.tcx.sess.diagnostic().struct_dummy() /* derived error */ + }; + let span = trace.origin.span(); - let mut err = self.report_type_error(trace, terr); + + let is_simple_error = if let &TypeError::Sorts(ref values) = terr { + values.expected.is_primitive() && values.found.is_primitive() + } else { + false + }; + + let mut err = self.tcx.sess.struct_span_err_with_code( + trace.origin.span(), + message, + code); + + if !is_simple_error || check_old_school() { + err.note_expected_found(&"type", &expected, &found); + } + + err.span_label(span, &terr); + + self.note_error_origin(&mut err, &trace.origin); + self.check_and_note_conflicting_crates(&mut err, terr, span); self.tcx.note_and_explain_type_err(&mut err, terr, span); + err } - /// Returns a string of the form "expected `{}`, found `{}`", or None if this is a derived - /// error. + pub fn report_and_explain_type_error(&self, + trace: TypeTrace<'tcx>, + terr: &TypeError<'tcx>) + -> DiagnosticBuilder<'tcx> + { + // FIXME: do we want to use a different error code for each origin? + let failure_str = trace.origin.as_failure_str(); + type_err!(self, trace, terr, E0308, "{}", failure_str) + } + + /// Returns a string of the form "expected `{}`, found `{}`". fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<(String, String)> { match *values { infer::Types(ref exp_found) => self.expected_found_str(exp_found), infer::TraitRefs(ref exp_found) => self.expected_found_str(exp_found), - infer::PolyTraitRefs(ref exp_found) => self.expected_found_str(exp_found) + infer::PolyTraitRefs(ref exp_found) => self.expected_found_str(exp_found), } } - fn expected_found_str + TypeFoldable<'tcx>>( + fn expected_found_str>( &self, exp_found: &ty::error::ExpectedFound) -> Option<(String, String)> { - let expected = exp_found.expected.resolve(self); - if expected.references_error() { + let exp_found = self.resolve_type_vars_if_possible(exp_found); + if exp_found.references_error() { return None; } - let found = exp_found.found.resolve(self); - if found.references_error() { - return None; - } - - Some((format!("{}", expected), format!("{}", found))) + Some((format!("{}", exp_found.expected), format!("{}", exp_found.found))) } fn report_generic_bound_failure(&self, @@ -1609,68 +1611,21 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { fn note_region_origin(&self, err: &mut DiagnosticBuilder, origin: &SubregionOrigin<'tcx>) { match *origin { infer::Subtype(ref trace) => { - let desc = match trace.origin { - TypeOrigin::Misc(_) => { - "types are compatible" - } - TypeOrigin::MethodCompatCheck(_) => { - "method type is compatible with trait" - } - TypeOrigin::ExprAssignable(_) => { - "expression is assignable" - } - TypeOrigin::RelateTraitRefs(_) => { - "traits are compatible" - } - TypeOrigin::RelateSelfType(_) => { - "self type matches impl self type" - } - TypeOrigin::RelateOutputImplTypes(_) => { - "trait type parameters matches those \ - specified on the impl" - } - TypeOrigin::MatchExpressionArm(_, _, _) => { - "match arms have compatible types" - } - TypeOrigin::IfExpression(_) => { - "if and else have compatible types" - } - TypeOrigin::IfExpressionWithNoElse(_) => { - "if may be missing an else clause" - } - TypeOrigin::RangeExpression(_) => { - "start and end of range have compatible types" - } - TypeOrigin::EquatePredicate(_) => { - "equality where clause is satisfied" - } - TypeOrigin::MainFunctionType(_) => { - "the `main` function has the correct type" - } - TypeOrigin::StartFunctionType(_) => { - "the `start` function has the correct type" - } - TypeOrigin::IntrinsicType(_) => { - "the intrinsic has the correct type" - } - }; + if let Some((expected, found)) = self.values_str(&trace.values) { + // FIXME: do we want a "the" here? + err.span_note( + trace.origin.span(), + &format!("...so that {} (expected {}, found {})", + trace.origin.as_requirement_str(), expected, found)); + } else { + // FIXME: this really should be handled at some earlier stage. Our + // handling of region checking when type errors are present is + // *terrible*. - match self.values_str(&trace.values) { - Some((expected, found)) => { - err.span_note( - trace.origin.span(), - &format!("...so that {} (expected {}, found {})", - desc, expected, found)); - } - None => { - // Really should avoid printing this error at - // all, since it is derived, but that would - // require more refactoring than I feel like - // doing right now. - nmatsakis - err.span_note( - trace.origin.span(), - &format!("...so that {}", desc)); - } + err.span_note( + trace.origin.span(), + &format!("...so that {}", + trace.origin.as_requirement_str())); } } infer::Reborrow(span) => { @@ -1813,32 +1768,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } -pub trait Resolvable<'tcx> { - fn resolve<'a, 'gcx>(&self, infcx: &InferCtxt<'a, 'gcx, 'tcx>) -> Self; -} - -impl<'tcx> Resolvable<'tcx> for Ty<'tcx> { - fn resolve<'a, 'gcx>(&self, infcx: &InferCtxt<'a, 'gcx, 'tcx>) -> Ty<'tcx> { - infcx.resolve_type_vars_if_possible(self) - } -} - -impl<'tcx> Resolvable<'tcx> for ty::TraitRef<'tcx> { - fn resolve<'a, 'gcx>(&self, infcx: &InferCtxt<'a, 'gcx, 'tcx>) - -> ty::TraitRef<'tcx> { - infcx.resolve_type_vars_if_possible(self) - } -} - -impl<'tcx> Resolvable<'tcx> for ty::PolyTraitRef<'tcx> { - fn resolve<'a, 'gcx>(&self, - infcx: &InferCtxt<'a, 'gcx, 'tcx>) - -> ty::PolyTraitRef<'tcx> - { - infcx.resolve_type_vars_if_possible(self) - } -} - fn lifetimes_in_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, scope_id: ast::NodeId) -> Vec { diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index dc262e61dd0..fd65e06ea97 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -231,7 +231,7 @@ pub enum TypeOrigin { } impl TypeOrigin { - fn as_str(&self) -> &'static str { + fn as_failure_str(&self) -> &'static str { match self { &TypeOrigin::Misc(_) | &TypeOrigin::RelateSelfType(_) | @@ -252,11 +252,26 @@ impl TypeOrigin { &TypeOrigin::IntrinsicType(_) => "intrinsic has wrong type", } } -} -impl fmt::Display for TypeOrigin { - fn fmt(&self, f: &mut fmt::Formatter) -> Result<(),fmt::Error> { - fmt::Display::fmt(self.as_str(), f) + fn as_requirement_str(&self) -> &'static str { + match self { + &TypeOrigin::Misc(_) => "types are compatible", + &TypeOrigin::MethodCompatCheck(_) => "method type is compatible with trait", + &TypeOrigin::ExprAssignable(_) => "expression is assignable", + &TypeOrigin::RelateTraitRefs(_) => "traits are compatible", + &TypeOrigin::RelateSelfType(_) => "self type matches impl self type", + &TypeOrigin::RelateOutputImplTypes(_) => { + "trait type parameters matches those specified on the impl" + } + &TypeOrigin::MatchExpressionArm(_, _, _) => "match arms have compatible types", + &TypeOrigin::IfExpression(_) => "if and else have compatible types", + &TypeOrigin::IfExpressionWithNoElse(_) => "if missing an else returns ()", + &TypeOrigin::RangeExpression(_) => "start and end of range have compatible types", + &TypeOrigin::EquatePredicate(_) => "equality where clause is satisfied", + &TypeOrigin::MainFunctionType(_) => "`main` function has the correct type", + &TypeOrigin::StartFunctionType(_) => "`start` function has the correct type", + &TypeOrigin::IntrinsicType(_) => "intrinsic has the correct type", + } } } @@ -1489,22 +1504,20 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { pub fn type_error_message(&self, sp: Span, mk_msg: M, - actual_ty: Ty<'tcx>, - err: Option<&TypeError<'tcx>>) + actual_ty: Ty<'tcx>) where M: FnOnce(String) -> String, { - self.type_error_struct(sp, mk_msg, actual_ty, err).emit(); + self.type_error_struct(sp, mk_msg, actual_ty).emit(); } pub fn type_error_struct(&self, sp: Span, mk_msg: M, - actual_ty: Ty<'tcx>, - err: Option<&TypeError<'tcx>>) + actual_ty: Ty<'tcx>) -> DiagnosticBuilder<'tcx> where M: FnOnce(String) -> String, { - debug!("type_error_struct({:?}, {:?}, {:?})", sp, actual_ty, err); + debug!("type_error_struct({:?}, {:?})", sp, actual_ty); let actual_ty = self.resolve_type_vars_if_possible(&actual_ty); @@ -1513,21 +1526,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { return self.tcx.sess.diagnostic().struct_dummy(); } - let error_str = err.map_or("".to_string(), |t_err| { - format!(" ({})", t_err) - }); - let msg = mk_msg(self.ty_to_string(actual_ty)); // FIXME: use an error code. - let mut db = self.tcx.sess.struct_span_err( - sp, &format!("{} {}", msg, error_str)); - - if let Some(err) = err { - self.tcx.note_and_explain_type_err(&mut db, err, sp); - } - - db + self.tcx.sess.struct_span_err(sp, &msg) } pub fn report_mismatched_types(&self, diff --git a/src/librustc/macros.rs b/src/librustc/macros.rs index 76dca1bb5b6..52420475db1 100644 --- a/src/librustc/macros.rs +++ b/src/librustc/macros.rs @@ -59,3 +59,15 @@ macro_rules! span_bug { $crate::session::span_bug_fmt(file!(), line!(), $span, format_args!($($message)*)) }) } + +#[macro_export] +macro_rules! type_err { + ($infcx:expr, $trace: expr, $terr: expr, $code:ident, $($message:tt)*) => ({ + __diagnostic_used!($code); + $infcx.report_and_explain_type_error_with_code( + $trace, + $terr, + &format!($($message)*), + stringify!($code)) + }) +} diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index 2c7e7d284fa..9c6727ebbfc 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -216,7 +216,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { _ => { let mut err = self.type_error_struct(call_expr.span, |actual| { format!("expected function, found `{}`", actual) - }, callee_ty, None); + }, callee_ty); if let hir::ExprCall(ref expr, _) = call_expr.node { let tcx = self.tcx; diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs index 22ac8bc5690..7a4cc09a7d5 100644 --- a/src/librustc_typeck/check/cast.rs +++ b/src/librustc_typeck/check/cast.rs @@ -149,7 +149,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> { format!("casting `{}` as `{}` is invalid", actual, fcx.ty_to_string(self.cast_ty)) - }, self.expr_ty, None) + }, self.expr_ty) .help(&format!("cast through {} first", match e { CastError::NeedViaPtr => "a raw pointer", CastError::NeedViaThinPtr => "a thin pointer", @@ -167,35 +167,35 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> { CastError::CastToChar => { fcx.type_error_message(self.span, |actual| { format!("only `u8` can be cast as `char`, not `{}`", actual) - }, self.expr_ty, None); + }, self.expr_ty); } CastError::NonScalar => { fcx.type_error_message(self.span, |actual| { format!("non-scalar cast: `{}` as `{}`", actual, fcx.ty_to_string(self.cast_ty)) - }, self.expr_ty, None); + }, self.expr_ty); } CastError::IllegalCast => { fcx.type_error_message(self.span, |actual| { format!("casting `{}` as `{}` is invalid", actual, fcx.ty_to_string(self.cast_ty)) - }, self.expr_ty, None); + }, self.expr_ty); } CastError::SizedUnsizedCast => { fcx.type_error_message(self.span, |actual| { format!("cannot cast thin pointer `{}` to fat pointer `{}`", actual, fcx.ty_to_string(self.cast_ty)) - }, self.expr_ty, None) + }, self.expr_ty) } CastError::DifferingKinds => { fcx.type_error_struct(self.span, |actual| { format!("casting `{}` as `{}` is invalid", actual, fcx.ty_to_string(self.cast_ty)) - }, self.expr_ty, None) + }, self.expr_ty) .note("vtable kinds may not match") .emit(); } @@ -213,7 +213,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> { let tstr = fcx.ty_to_string(self.cast_ty); let mut err = fcx.type_error_struct(self.span, |actual| { format!("cast to unsized type: `{}` as `{}`", actual, tstr) - }, self.expr_ty, None); + }, self.expr_ty); match self.expr_ty.sty { ty::TyRef(_, ty::TypeAndMut { mutbl: mt, .. }) => { let mtstr = match mt { @@ -484,4 +484,3 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { traits::type_known_to_meet_builtin_bound(self, ty, ty::BoundSized, span) } } - diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index f20dcdc35ae..346449d0a51 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -160,8 +160,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { item_name, actual) }, - rcvr_ty, - None); + rcvr_ty); // If the item has the name of a field, give a help note if let (&ty::TyStruct(def, substs), Some(expr)) = (&rcvr_ty.sty, rcvr_expr) { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fc1d2236f3f..7076b6a2a90 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2541,21 +2541,21 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.type_error_message(arg.span, |t| { format!("can't pass an `{}` to variadic \ function, cast to `c_double`", t) - }, arg_ty, None); + }, arg_ty); } ty::TyInt(ast::IntTy::I8) | ty::TyInt(ast::IntTy::I16) | ty::TyBool => { self.type_error_message(arg.span, |t| { format!("can't pass `{}` to variadic \ function, cast to `c_int`", t) - }, arg_ty, None); + }, arg_ty); } ty::TyUint(ast::UintTy::U8) | ty::TyUint(ast::UintTy::U16) => { self.type_error_message(arg.span, |t| { format!("can't pass `{}` to variadic \ function, cast to `c_uint`", t) - }, arg_ty, None); + }, arg_ty); } ty::TyFnDef(_, _, f) => { let ptr_ty = self.tcx.mk_fn_ptr(f); @@ -2564,7 +2564,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { |t| { format!("can't pass `{}` to variadic \ function, cast to `{}`", t, ptr_ty) - }, arg_ty, None); + }, arg_ty); } _ => {} } @@ -2908,9 +2908,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.type_error_struct(field.span, |actual| { format!("attempted to take value of method `{}` on type \ `{}`", field.node, actual) - }, expr_t, None) - .help( - "maybe a `()` to call it is missing? \ + }, expr_t) + .help("maybe a `()` to call it is missing? \ If not, try an anonymous function") .emit(); self.write_error(expr.id); @@ -2919,7 +2918,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { format!("attempted access of field `{}` on type `{}`, \ but no field with that name was found", field.node, actual) - }, expr_t, None); + }, expr_t); if let ty::TyStruct(def, _) = expr_t.sty { Self::suggest_field_names(&mut err, def.struct_variant(), field, vec![]); } @@ -3019,7 +3018,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { actual) } }, - expr_t, None); + expr_t); self.write_error(expr.id); } @@ -3038,8 +3037,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { format!("structure `{}` has no field named `{}`", actual, field.name.node) }, - ty, - None); + ty); // prevent all specified fields from being suggested let skip_fields = skip_fields.iter().map(|ref x| x.name.node.as_str()); Self::suggest_field_names(&mut err, variant, &field.name, skip_fields.collect()); @@ -3272,7 +3270,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.type_error_message(expr.span, |actual| { format!("type `{}` cannot be \ dereferenced", actual) - }, oprnd_t, None); + }, oprnd_t); oprnd_t = tcx.types.err; } } @@ -3647,8 +3645,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { format!("cannot index a value of type `{}`", actual) }, - base_t, - None); + base_t); // Try to give some advice about indexing tuples. if let ty::TyTuple(_) = base_t.sty { let mut needs_note = true; @@ -4523,7 +4520,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if !self.is_tainted_by_errors() { self.type_error_message(sp, |_actual| { "the type of this value must be known in this context".to_string() - }, ty, None); + }, ty); } self.demand_suptype(sp, self.tcx.types.err, ty); ty = self.tcx.types.err; diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 8604dadf46d..d02f87d0b9c 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -239,7 +239,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.type_error_message(ex.span, |actual| { format!("cannot apply unary operator `{}` to type `{}`", op_str, actual) - }, operand_ty, None); + }, operand_ty); self.tcx.types.err } }