From 3699c2497bc6ff74126ae8c56a06905f49d0300c Mon Sep 17 00:00:00 2001 From: mejrs <> Date: Mon, 12 Sep 2022 22:17:06 +0200 Subject: [PATCH] Address feedback --- .../src/infer/error_reporting/mod.rs | 177 ++++++++++++++---- compiler/rustc_infer/src/lib.rs | 2 + .../fully-qualified-type-name2.stderr | 4 +- src/test/ui/issues/issue-56943.stderr | 12 -- src/test/ui/mismatched_types/show_module.rs | 18 ++ .../ui/mismatched_types/show_module.stderr | 23 +++ .../ui/mismatched_types/similar_paths.stderr | 4 +- .../similar_paths_primitive.rs | 10 + .../similar_paths_primitive.stderr | 24 +++ .../type/type-mismatch-same-crate-name.stderr | 4 +- 10 files changed, 226 insertions(+), 52 deletions(-) create mode 100644 src/test/ui/mismatched_types/show_module.rs create mode 100644 src/test/ui/mismatched_types/show_module.stderr create mode 100644 src/test/ui/mismatched_types/similar_paths_primitive.rs create mode 100644 src/test/ui/mismatched_types/similar_paths_primitive.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index ab76b4884781..d4e8e267babe 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -1654,47 +1654,156 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ), Mismatch::Fixed(s) => (s.into(), s.into(), None), }; - let looks_similar = |e: ExpectedFound>| { - // We're only interested in adts - if let (Some(e), Some(f)) = (e.expected.ty_adt_def(), e.found.ty_adt_def()) { - // Only compare the last parts of the path. - // `whatever::Foo` is pretty similar to `blah::Foo` - let e_path = self.tcx.def_path(e.did()).data; - let f_path = self.tcx.def_path(f.did()).data; - if let (Some(e), Some(f)) = (e_path.last(), f_path.last()) { - return e.data == f.data; + + enum Similar<'tcx> { + Adts(ty::AdtDef<'tcx>, ty::AdtDef<'tcx>), + PrimitiveFound(Ty<'tcx>, ty::AdtDef<'tcx>), + PrimitiveExpected(ty::AdtDef<'tcx>, Ty<'tcx>), + } + + let primitive_sym = |kind: &_| match kind { + ty::Bool => Some(sym::bool), + ty::Char => Some(sym::char), + ty::Float(f) => match f { + ty::FloatTy::F32 => Some(sym::f32), + ty::FloatTy::F64 => Some(sym::f64), + }, + ty::Int(f) => match f { + ty::IntTy::Isize => Some(sym::isize), + ty::IntTy::I8 => Some(sym::i8), + ty::IntTy::I16 => Some(sym::i16), + ty::IntTy::I32 => Some(sym::i32), + ty::IntTy::I64 => Some(sym::i64), + ty::IntTy::I128 => Some(sym::i128), + }, + ty::Uint(f) => match f { + ty::UintTy::Usize => Some(sym::usize), + ty::UintTy::U8 => Some(sym::u8), + ty::UintTy::U16 => Some(sym::u16), + ty::UintTy::U32 => Some(sym::u32), + ty::UintTy::U64 => Some(sym::u64), + ty::UintTy::U128 => Some(sym::u128), + }, + _ => None, + }; + + let similarity = |e: ExpectedFound>| { + let (fk, ek) = (e.found.kind(), e.expected.kind()); + match (fk, ek) { + ( + ty::Adt(adt, _), + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_), + ) => { + let path = self.tcx.def_path(adt.did()).data; + let name = path.last().unwrap().data.get_opt_name(); + let prim_sym = primitive_sym(ek); + + if name == prim_sym { + return Some(Similar::PrimitiveExpected(*adt, e.expected)); + } + None } + ( + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_), + ty::Adt(adt, _), + ) => { + let path = self.tcx.def_path(adt.did()).data; + let name = path.last().unwrap().data.get_opt_name(); + let prim_sym = primitive_sym(fk); + + if name == prim_sym { + return Some(Similar::PrimitiveFound(e.expected, *adt)); + } + None + } + (ty::Adt(f, _), ty::Adt(e, _)) => { + if !f.did().is_local() && f.did().krate == e.did().krate { + // Most likely types from different versions of the same crate + // are in play, in which case this message isn't so helpful. + // A "perhaps two different versions..." error is already emitted for that. + return None; + } + let e_path = self.tcx.def_path(e.did()).data; + let f_path = self.tcx.def_path(f.did()).data; + if let (Some(e_last), Some(f_last)) = (e_path.last(), f_path.last()) && e_last == f_last { + return Some(Similar::Adts(*f, *e)); + } + None + } + _ => None, } - false }; match terr { // If two types mismatch but have similar names, mention that specifically. - TypeError::Sorts(values) if looks_similar(values) => { - let found_adt = values.found.ty_adt_def().unwrap(); - let expected_adt = values.expected.ty_adt_def().unwrap(); - - let found_name = values.found.sort_string(self.tcx); - let expected_name = values.expected.sort_string(self.tcx); - - diag.note(format!("{found_name} and {expected_name} have similar names, but are actually distinct types")); - - for (adt, name) in [(found_adt, found_name), (expected_adt, expected_name)] { - let defid = adt.did(); - let def_span = self.tcx.def_span(defid); - - let msg = if defid.is_local() { - format!("{name} is defined in the current crate.") - } else if self.tcx.all_diagnostic_items(()).id_to_name.get(&defid).is_some() - { - // if it's a diagnostic item, it's definitely defined in std/core/alloc - // otherwise might be, might not be. - format!("{name} is defined in the standard library.") - } else { - let crate_name = self.tcx.crate_name(defid.krate); - format!("{name} is defined in crate `{crate_name}`.") + TypeError::Sorts(values) if let Some(s) = similarity(values) => { + let diagnose_primitive = + |prim: Ty<'tcx>, + shadow: Ty<'tcx>, + defid: DefId, + diagnostic: &mut Diagnostic| { + let name = shadow.sort_string(self.tcx); + diagnostic.note(format!( + "{prim} and {name} have similar names, but are actually distinct types" + )); + diagnostic + .note(format!("{prim} is a primitive defined by the language")); + let def_span = self.tcx.def_span(defid); + let msg = if defid.is_local() { + format!("{name} is defined in the current crate") + } else { + let crate_name = self.tcx.crate_name(defid.krate); + format!("{name} is defined in crate `{crate_name}") + }; + diagnostic.span_note(def_span, msg); }; - diag.span_note(def_span, msg); + + let diagnose_adts = + |found_adt: ty::AdtDef<'tcx>, + expected_adt: ty::AdtDef<'tcx>, + diagnostic: &mut Diagnostic| { + let found_name = values.found.sort_string(self.tcx); + let expected_name = values.expected.sort_string(self.tcx); + + let found_defid = found_adt.did(); + let expected_defid = expected_adt.did(); + + diagnostic.note(format!("{found_name} and {expected_name} have similar names, but are actually distinct types")); + for (defid, name) in + [(found_defid, found_name), (expected_defid, expected_name)] + { + let def_span = self.tcx.def_span(defid); + + let msg = if found_defid.is_local() && expected_defid.is_local() { + let module = self + .tcx + .parent_module_from_def_id(defid.expect_local()) + .to_def_id(); + let module_name = + self.tcx.def_path(module).to_string_no_crate_verbose(); + format!( + "{name} is defined in module {module_name} of the current crate" + ) + } else if defid.is_local() { + format!("{name} is defined in the current crate") + } else { + let crate_name = self.tcx.crate_name(defid.krate); + format!("{name} is defined in crate `{crate_name}`") + }; + diagnostic.span_note(def_span, msg); + } + }; + + match s { + Similar::Adts(found_adt, expected_adt) => { + diagnose_adts(found_adt, expected_adt, diag) + } + Similar::PrimitiveFound(prim, e) => { + diagnose_primitive(prim, values.expected, e.did(), diag) + } + Similar::PrimitiveExpected(f, prim) => { + diagnose_primitive(prim, values.found, f.did(), diag) + } } } TypeError::Sorts(values) => { diff --git a/compiler/rustc_infer/src/lib.rs b/compiler/rustc_infer/src/lib.rs index ef60d2c91884..5b14e5bdb78c 100644 --- a/compiler/rustc_infer/src/lib.rs +++ b/compiler/rustc_infer/src/lib.rs @@ -20,6 +20,8 @@ #![cfg_attr(bootstrap, feature(label_break_value))] #![feature(let_chains)] #![cfg_attr(bootstrap, feature(let_else))] +#![feature(let_else)] +#![feature(if_let_guard)] #![feature(min_specialization)] #![feature(never_type)] #![feature(try_blocks)] diff --git a/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr b/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr index 8729ea1740ce..94c34cf9d04f 100644 --- a/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr +++ b/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr @@ -7,12 +7,12 @@ LL | return x; | ^ expected enum `y::Foo`, found enum `x::Foo` | = note: enum `x::Foo` and enum `y::Foo` have similar names, but are actually distinct types -note: enum `x::Foo` is defined in the current crate. +note: enum `x::Foo` is defined in module ::x of the current crate --> $DIR/fully-qualified-type-name2.rs:4:5 | LL | pub enum Foo { } | ^^^^^^^^^^^^ -note: enum `y::Foo` is defined in the current crate. +note: enum `y::Foo` is defined in module ::y of the current crate --> $DIR/fully-qualified-type-name2.rs:8:5 | LL | pub enum Foo { } diff --git a/src/test/ui/issues/issue-56943.stderr b/src/test/ui/issues/issue-56943.stderr index 3efa5f6d0409..74ed5ec0fb6f 100644 --- a/src/test/ui/issues/issue-56943.stderr +++ b/src/test/ui/issues/issue-56943.stderr @@ -5,18 +5,6 @@ LL | let _: issue_56943::S = issue_56943::S2; | -------------- ^^^^^^^^^^^^^^^ expected struct `S`, found struct `S2` | | | expected due to this - | - = note: struct `S2` and struct `S` have similar names, but are actually distinct types -note: struct `S2` is defined in crate `issue_56943`. - --> $DIR/auxiliary/issue-56943.rs:2:9 - | -LL | mod m { pub struct S; } - | ^^^^^^^^^^^^ -note: struct `S` is defined in crate `issue_56943`. - --> $DIR/auxiliary/issue-56943.rs:1:1 - | -LL | pub struct S; - | ^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/mismatched_types/show_module.rs b/src/test/ui/mismatched_types/show_module.rs new file mode 100644 index 000000000000..97d45b377bcf --- /dev/null +++ b/src/test/ui/mismatched_types/show_module.rs @@ -0,0 +1,18 @@ +pub mod blah{ + pub mod baz{ + pub struct Foo; + } +} + +pub mod meh{ + pub struct Foo; +} + +pub type Foo = blah::baz::Foo; + +fn foo() -> Foo { + meh::Foo + //~^ ERROR mismatched types [E0308] +} + +fn main(){} diff --git a/src/test/ui/mismatched_types/show_module.stderr b/src/test/ui/mismatched_types/show_module.stderr new file mode 100644 index 000000000000..6d8986d1cad2 --- /dev/null +++ b/src/test/ui/mismatched_types/show_module.stderr @@ -0,0 +1,23 @@ +error[E0308]: mismatched types + --> $DIR/show_module.rs:14:5 + | +LL | fn foo() -> Foo { + | --- expected `baz::Foo` because of return type +LL | meh::Foo + | ^^^^^^^^ expected struct `baz::Foo`, found struct `meh::Foo` + | + = note: struct `meh::Foo` and struct `baz::Foo` have similar names, but are actually distinct types +note: struct `meh::Foo` is defined in module ::meh of the current crate + --> $DIR/show_module.rs:8:5 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^ +note: struct `baz::Foo` is defined in module ::blah::baz of the current crate + --> $DIR/show_module.rs:3:9 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/mismatched_types/similar_paths.stderr b/src/test/ui/mismatched_types/similar_paths.stderr index c12afd20b9cf..16f33e55fea0 100644 --- a/src/test/ui/mismatched_types/similar_paths.stderr +++ b/src/test/ui/mismatched_types/similar_paths.stderr @@ -7,12 +7,12 @@ LL | Some(42_u8) | ^^^^^^^^^^^ expected enum `Option`, found enum `std::option::Option` | = note: enum `std::option::Option` and enum `Option` have similar names, but are actually distinct types -note: enum `std::option::Option` is defined in the standard library. +note: enum `std::option::Option` is defined in crate `core` --> $SRC_DIR/core/src/option.rs:LL:COL | LL | pub enum Option { | ^^^^^^^^^^^^^^^^^^ -note: enum `Option` is defined in the current crate. +note: enum `Option` is defined in the current crate --> $DIR/similar_paths.rs:1:1 | LL | enum Option{ diff --git a/src/test/ui/mismatched_types/similar_paths_primitive.rs b/src/test/ui/mismatched_types/similar_paths_primitive.rs new file mode 100644 index 000000000000..8f5b7cce4690 --- /dev/null +++ b/src/test/ui/mismatched_types/similar_paths_primitive.rs @@ -0,0 +1,10 @@ +#![allow(non_camel_case_types)] + +struct bool; + +fn foo(_: bool) {} + +fn main() { + foo(true); + //~^ ERROR mismatched types [E0308] +} diff --git a/src/test/ui/mismatched_types/similar_paths_primitive.stderr b/src/test/ui/mismatched_types/similar_paths_primitive.stderr new file mode 100644 index 000000000000..8a2f73945e84 --- /dev/null +++ b/src/test/ui/mismatched_types/similar_paths_primitive.stderr @@ -0,0 +1,24 @@ +error[E0308]: mismatched types + --> $DIR/similar_paths_primitive.rs:8:9 + | +LL | foo(true); + | --- ^^^^ expected struct `bool`, found `bool` + | | + | arguments to this function are incorrect + | + = note: bool and struct `bool` have similar names, but are actually distinct types + = note: bool is a primitive defined by the language +note: struct `bool` is defined in the current crate + --> $DIR/similar_paths_primitive.rs:3:1 + | +LL | struct bool; + | ^^^^^^^^^^^ +note: function defined here + --> $DIR/similar_paths_primitive.rs:5:4 + | +LL | fn foo(_: bool) {} + | ^^^ ------- + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/type/type-mismatch-same-crate-name.stderr b/src/test/ui/type/type-mismatch-same-crate-name.stderr index 38a36e8940fa..fcafd315ebf5 100644 --- a/src/test/ui/type/type-mismatch-same-crate-name.stderr +++ b/src/test/ui/type/type-mismatch-same-crate-name.stderr @@ -7,12 +7,12 @@ LL | a::try_foo(foo2); | arguments to this function are incorrect | = note: struct `main::a::Foo` and struct `main::a::Foo` have similar names, but are actually distinct types -note: struct `main::a::Foo` is defined in crate `crate_a2`. +note: struct `main::a::Foo` is defined in crate `crate_a2` --> $DIR/auxiliary/crate_a2.rs:1:1 | LL | pub struct Foo; | ^^^^^^^^^^^^^^ -note: struct `main::a::Foo` is defined in crate `crate_a1`. +note: struct `main::a::Foo` is defined in crate `crate_a1` --> $DIR/auxiliary/crate_a1.rs:1:1 | LL | pub struct Foo;