From 8b16eb832596d305f7da3f88c64aa35762fcba1e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 8 Sep 2015 19:48:56 +0530 Subject: [PATCH 1/6] Add note for when a type error comes from similarly named objects from two different crate of the same name (#22750) --- src/librustc/middle/infer/error_reporting.rs | 37 ++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 8197ccf4be7d3..67357ab239f53 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -226,6 +226,8 @@ pub trait ErrorReporting<'tcx> { fn report_type_error(&self, trace: TypeTrace<'tcx>, terr: &ty::TypeError<'tcx>); + fn check_and_note_conflicting_crates(&self, terr: &ty::TypeError<'tcx>, sp: Span); + fn report_and_explain_type_error(&self, trace: TypeTrace<'tcx>, terr: &ty::TypeError<'tcx>); @@ -484,6 +486,8 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { expected_found_str, terr); + self.check_and_note_conflicting_crates(terr, trace.origin.span()); + match trace.origin { infer::MatchExpressionArm(_, arm_span) => self.tcx.sess.span_note(arm_span, "match arm with an incompatible type"), @@ -491,6 +495,39 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { } } + /// Adds a note if the types come from similarly named crates + fn check_and_note_conflicting_crates(&self, terr: &ty::TypeError<'tcx>, sp: Span) { + match *terr { + ty::TypeError::Sorts(ref exp_found) => { + // if they are both "path types", there's a chance of ambiguity + // due to different versions of the same crate + match (&exp_found.expected.sty, &exp_found.found.sty) { + (&ty::TyEnum(ref exp_adt, _), &ty::TyEnum(ref found_adt, _)) | + (&ty::TyStruct(ref exp_adt, _), &ty::TyStruct(ref found_adt, _)) | + (&ty::TyEnum(ref exp_adt, _), &ty::TyStruct(ref found_adt, _)) | + (&ty::TyStruct(ref exp_adt, _), &ty::TyEnum(ref found_adt, _)) => { + // Only external crates, if either is from a local + // module we could have false positives + if exp_adt.did.is_local() || found_adt.did.is_local() { + return + } + let exp_path = self.tcx.with_path(exp_adt.did, + |p| p.collect::>()); + let found_path = self.tcx.with_path(exp_adt.did, + |p| p.collect::>()); + if exp_path == found_path { + self.tcx.sess.span_note(sp, &format!("Perhaps two different versions \ + of crate `{}` are being used?", + exp_path[0])); + } + }, + _ => () + } + } + _ => () // FIXME(Manishearth) handle traits and stuff + } + } + fn report_and_explain_type_error(&self, trace: TypeTrace<'tcx>, terr: &ty::TypeError<'tcx>) { From 89af15322dbca73b098e55bbd283a2d8a254571b Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 8 Sep 2015 22:21:20 +0530 Subject: [PATCH 2/6] Handle trait objects --- src/librustc/middle/infer/error_reporting.rs | 41 +++++++++++++------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 67357ab239f53..8f929d9b19c2e 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -78,6 +78,7 @@ use rustc_front::hir; use rustc_front::print::pprust; use middle::def; +use middle::def_id::DefId; use middle::infer; use middle::region; use middle::subst; @@ -497,6 +498,25 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { /// Adds a note if the types come from similarly named crates fn check_and_note_conflicting_crates(&self, terr: &ty::TypeError<'tcx>, sp: Span) { + let report_path_match = |did1: DefId, did2: DefId| { + // Only external crates, if either is from a local + // module we could have false positives + if !(did1.is_local() || did2.is_local()) { + let exp_path = self.tcx.with_path(did1, + |p| p.map(|x| x.to_string()) + .collect::>()); + let found_path = self.tcx.with_path(did2, + |p| p.map(|x| x.to_string()) + .collect::>()); + // We compare strings because PathMod and PathName can be different + // for imported and non-imported crates + if exp_path == found_path { + self.tcx.sess.span_note(sp, &format!("Perhaps two different versions \ + of crate `{}` are being used?", + exp_path[0])); + } + } + }; match *terr { ty::TypeError::Sorts(ref exp_found) => { // if they are both "path types", there's a chance of ambiguity @@ -506,24 +526,15 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { (&ty::TyStruct(ref exp_adt, _), &ty::TyStruct(ref found_adt, _)) | (&ty::TyEnum(ref exp_adt, _), &ty::TyStruct(ref found_adt, _)) | (&ty::TyStruct(ref exp_adt, _), &ty::TyEnum(ref found_adt, _)) => { - // Only external crates, if either is from a local - // module we could have false positives - if exp_adt.did.is_local() || found_adt.did.is_local() { - return - } - let exp_path = self.tcx.with_path(exp_adt.did, - |p| p.collect::>()); - let found_path = self.tcx.with_path(exp_adt.did, - |p| p.collect::>()); - if exp_path == found_path { - self.tcx.sess.span_note(sp, &format!("Perhaps two different versions \ - of crate `{}` are being used?", - exp_path[0])); - } + report_path_match(exp_adt.did, found_adt.did); }, _ => () } - } + }, + ty::TypeError::Traits(ref exp_found) => { + self.tcx.sess.note("errrr0"); + report_path_match(exp_found.expected, exp_found.found); + }, _ => () // FIXME(Manishearth) handle traits and stuff } } From b48ffa073fe81706a0416c6aaf70bf96d380465e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 8 Sep 2015 23:50:48 +0530 Subject: [PATCH 3/6] Use 'a different' for trait object mismatches too --- src/librustc/middle/ty.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 9f2c87b1a0a5e..99d8c5c9301ec 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -5311,6 +5311,16 @@ impl<'tcx> TyS<'tcx> { impl<'tcx> fmt::Display for TypeError<'tcx> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use self::TypeError::*; + fn report_maybe_different(f: &mut fmt::Formatter, + expected: String, found: String) -> fmt::Result { + // A naive approach to making sure that we're not reporting silly errors such as: + // (expected closure, found closure). + if expected == found { + write!(f, "expected {}, found a different {}", expected, found) + } else { + write!(f, "expected {}, found {}", expected, found) + } + } match *self { CyclicTy => write!(f, "cyclic type of infinite size"), @@ -5371,20 +5381,15 @@ impl<'tcx> fmt::Display for TypeError<'tcx> { found bound lifetime parameter {}", br) } Sorts(values) => tls::with(|tcx| { - // A naive approach to making sure that we're not reporting silly errors such as: - // (expected closure, found closure). - let expected_str = values.expected.sort_string(tcx); - let found_str = values.found.sort_string(tcx); - if expected_str == found_str { - write!(f, "expected {}, found a different {}", expected_str, found_str) - } else { - write!(f, "expected {}, found {}", expected_str, found_str) - } + report_maybe_different(f, values.expected.sort_string(tcx), + values.found.sort_string(tcx)) }), Traits(values) => tls::with(|tcx| { - write!(f, "expected trait `{}`, found trait `{}`", - tcx.item_path_str(values.expected), - tcx.item_path_str(values.found)) + report_maybe_different(f, + format!("trait `{}`", + tcx.item_path_str(values.expected)), + format!("trait `{}`", + tcx.item_path_str(values.found))) }), BuiltinBoundsMismatch(values) => { if values.expected.is_empty() { From a3f05f6acec1de07d507f2e26af57522c2335ed7 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 8 Sep 2015 23:57:24 +0530 Subject: [PATCH 4/6] Fix fixme, add crate check --- src/librustc/middle/infer/error_reporting.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 8f929d9b19c2e..363af87b70ba0 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -501,7 +501,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { let report_path_match = |did1: DefId, did2: DefId| { // Only external crates, if either is from a local // module we could have false positives - if !(did1.is_local() || did2.is_local()) { + if !(did1.is_local() || did2.is_local()) && did1.krate != did2.krate { let exp_path = self.tcx.with_path(did1, |p| p.map(|x| x.to_string()) .collect::>()); @@ -535,7 +535,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { self.tcx.sess.note("errrr0"); report_path_match(exp_found.expected, exp_found.found); }, - _ => () // FIXME(Manishearth) handle traits and stuff + _ => () // FIXME(#22750) handle traits and stuff } } From 00e70051dc6fc65eba67b9f7e2368adfdf08f0a9 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 9 Sep 2015 00:34:55 +0530 Subject: [PATCH 5/6] Add test --- src/test/auxiliary/crate_a1.rs | 21 ++++++++++++ src/test/auxiliary/crate_a2.rs | 17 ++++++++++ .../type-mismatch-same-crate-name.rs | 33 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 src/test/auxiliary/crate_a1.rs create mode 100644 src/test/auxiliary/crate_a2.rs create mode 100644 src/test/compile-fail/type-mismatch-same-crate-name.rs diff --git a/src/test/auxiliary/crate_a1.rs b/src/test/auxiliary/crate_a1.rs new file mode 100644 index 0000000000000..70f7cac94de6c --- /dev/null +++ b/src/test/auxiliary/crate_a1.rs @@ -0,0 +1,21 @@ +// Copyright 2015 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. + +pub struct Foo; + +pub trait Bar{} + +pub fn bar() -> Box { + unimplemented!() +} + + +pub fn try_foo(x: Foo){} +pub fn try_bar(x: Box){} diff --git a/src/test/auxiliary/crate_a2.rs b/src/test/auxiliary/crate_a2.rs new file mode 100644 index 0000000000000..d801f25ba2ee0 --- /dev/null +++ b/src/test/auxiliary/crate_a2.rs @@ -0,0 +1,17 @@ +// Copyright 2015 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. + +pub struct Foo; + +pub trait Bar{} + +pub fn bar() -> Box { + unimplemented!() +} diff --git a/src/test/compile-fail/type-mismatch-same-crate-name.rs b/src/test/compile-fail/type-mismatch-same-crate-name.rs new file mode 100644 index 0000000000000..69ef31e90e47c --- /dev/null +++ b/src/test/compile-fail/type-mismatch-same-crate-name.rs @@ -0,0 +1,33 @@ +// 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. + +// aux-build:crate_a1.rs +// aux-build:crate_a2.rs + +// This tests the extra note reported when a type error deals with +// seemingly identical types. +// The main use case of this error is when there are two crates +// (generally different versions of the same crate) with the same name +// causing a type mismatch. Here, we simulate that error using block-scoped +// aliased `extern crate` declarations. + +fn main() { + let foo2 = {extern crate crate_a2 as a; a::Foo}; + let bar2 = {extern crate crate_a2 as a; a::bar()}; + { + extern crate crate_a1 as a; + a::try_foo(foo2); //~ ERROR mismatched types + //~^ HELP run + //~^^ NOTE Perhaps two different versions of crate `main` + a::try_bar(bar2); //~ ERROR mismatched types + //~^ HELP run + //~^^ NOTE Perhaps two different versions of crate `main` + } +} From c65d33819ccf141d40bd9bf30784b36bf83c124b Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 9 Sep 2015 01:22:03 +0530 Subject: [PATCH 6/6] Print correct crate name --- src/librustc/middle/infer/error_reporting.rs | 4 +++- src/test/compile-fail/type-mismatch-same-crate-name.rs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 363af87b70ba0..7ec39ac851574 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -511,9 +511,11 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { // We compare strings because PathMod and PathName can be different // for imported and non-imported crates if exp_path == found_path { + let crate_name = self.tcx.sess.cstore + .get_crate_data(did1.krate).name(); self.tcx.sess.span_note(sp, &format!("Perhaps two different versions \ of crate `{}` are being used?", - exp_path[0])); + crate_name)); } } }; diff --git a/src/test/compile-fail/type-mismatch-same-crate-name.rs b/src/test/compile-fail/type-mismatch-same-crate-name.rs index 69ef31e90e47c..014fa35c309e6 100644 --- a/src/test/compile-fail/type-mismatch-same-crate-name.rs +++ b/src/test/compile-fail/type-mismatch-same-crate-name.rs @@ -25,9 +25,9 @@ fn main() { extern crate crate_a1 as a; a::try_foo(foo2); //~ ERROR mismatched types //~^ HELP run - //~^^ NOTE Perhaps two different versions of crate `main` + //~^^ NOTE Perhaps two different versions of crate `crate_a1` a::try_bar(bar2); //~ ERROR mismatched types //~^ HELP run - //~^^ NOTE Perhaps two different versions of crate `main` + //~^^ NOTE Perhaps two different versions of crate `crate_a1` } }