Skip to content

Commit

Permalink
Unrolled build for rust-lang#133767
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#133767 - estebank:multiple-dep-version-tests, r=Nadrieril

Add more info on type/trait mismatches for different crate versions

When encountering a type or trait mismatch for two types coming from two different crates with the same name, detect if it is either mixing two types/traits from the same crate on different versions:

```
error[E0308]: mismatched types
  --> replaced
   |
LL |     do_something_type(Type);
   |     ----------------- ^^^^ expected `dependency::Type`, found `dep_2_reexport::Type`
   |     |
   |     arguments to this function are incorrect
   |
note: two different versions of crate `dependency` are being used; two types coming from two different versions of the same crate are different types even if they look the same
  --> replaced
   |
LL | pub struct Type(pub i32);
   | ^^^^^^^^^^^^^^^ this is the expected type `dependency::Type`
   |
  ::: replaced
   |
LL | pub struct Type;
   | ^^^^^^^^^^^^^^^ this is the found type `dep_2_reexport::Type`
   |
  ::: replaced
   |
LL | extern crate dep_2_reexport;
   | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo`
LL | extern crate dependency;
   | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate
   = help: you can use `cargo tree` to explore your dependency tree
note: function defined here
  --> replaced
   |
LL | pub fn do_something_type(_: Type) {}
   |        ^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
  --> replaced
   |
LL |     do_something_trait(Box::new(Type) as Box<dyn Trait2>);
   |     ------------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected trait `dependency::Trait2`, found trait `dep_2_reexport::Trait2`
   |     |
   |     arguments to this function are incorrect
   |
note: two different versions of crate `dependency` are being used; two types coming from two different versions of the same crate are different types even if they look the same
  --> replaced
   |
LL | pub trait Trait2 {}
   | ^^^^^^^^^^^^^^^^ this is the expected trait `dependency::Trait2`
   |
  ::: replaced
   |
LL | pub trait Trait2 {}
   | ^^^^^^^^^^^^^^^^ this is the found trait `dep_2_reexport::Trait2`
   |
  ::: replaced
   |
LL | extern crate dep_2_reexport;
   | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo`
LL | extern crate dependency;
   | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate
   = help: you can use `cargo tree` to explore your dependency tree
note: function defined here
  --> replaced
   |
LL | pub fn do_something_trait(_: Box<dyn Trait2>) {}
   |        ^^^^^^^^^^^^^^^^^^
```

or if it is different crates that were renamed to the same name:

```
error[E0308]: mismatched types
  --> $DIR/type-mismatch-same-crate-name.rs:21:20
   |
LL |         a::try_foo(foo2);
   |         ---------- ^^^^ expected `main::a::Foo`, found a different `main::a::Foo`
   |         |
   |         arguments to this function are incorrect
   |
note: two types coming from two different crates are different types even if they look the same
  --> $DIR/auxiliary/crate_a2.rs:1:1
   |
LL | pub struct Foo;
   | ^^^^^^^^^^^^^^ this is the found type `crate_a2::Foo`
   |
  ::: $DIR/auxiliary/crate_a1.rs:1:1
   |
LL | pub struct Foo;
   | ^^^^^^^^^^^^^^ this is the expected type `crate_a1::Foo`
   |
  ::: $DIR/type-mismatch-same-crate-name.rs:13:17
   |
LL |     let foo2 = {extern crate crate_a2 as a; a::Foo};
   |                 --------------------------- one type comes from crate `crate_a2` is used here, which is renamed locally to `a`
...
LL |         extern crate crate_a1 as a;
   |         --------------------------- one type comes from crate `crate_a1` is used here, which is renamed locally to `a`
note: function defined here
  --> $DIR/auxiliary/crate_a1.rs:10:8
   |
LL | pub fn try_foo(x: Foo){}
   |        ^^^^^^^

error[E0308]: mismatched types
  --> $DIR/type-mismatch-same-crate-name.rs:27:20
   |
LL |         a::try_bar(bar2);
   |         ---------- ^^^^ expected trait `main::a::Bar`, found a different trait `main::a::Bar`
   |         |
   |         arguments to this function are incorrect
   |
note: two types coming from two different crates are different types even if they look the same
  --> $DIR/auxiliary/crate_a2.rs:3:1
   |
LL | pub trait Bar {}
   | ^^^^^^^^^^^^^ this is the found trait `crate_a2::Bar`
   |
  ::: $DIR/auxiliary/crate_a1.rs:3:1
   |
LL | pub trait Bar {}
   | ^^^^^^^^^^^^^ this is the expected trait `crate_a1::Bar`
   |
  ::: $DIR/type-mismatch-same-crate-name.rs:13:17
   |
LL |     let foo2 = {extern crate crate_a2 as a; a::Foo};
   |                 --------------------------- one trait comes from crate `crate_a2` is used here, which is renamed locally to `a`
...
LL |         extern crate crate_a1 as a;
   |         --------------------------- one trait comes from crate `crate_a1` is used here, which is renamed locally to `a`
note: function defined here
  --> $DIR/auxiliary/crate_a1.rs:11:8
   |
LL | pub fn try_bar(x: Box<Bar>){}
   |        ^^^^^^^
```

This new output unifies the E0308 errors detail with the pre-existing E0277 errors, and better differentiates the "`extern crate` renamed" and "same crate, different versions" cases.
  • Loading branch information
rust-timer authored Dec 8, 2024
2 parents f415c07 + 16bf722 commit 5e34106
Show file tree
Hide file tree
Showing 15 changed files with 281 additions and 76 deletions.
134 changes: 116 additions & 18 deletions compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ use std::{cmp, fmt, iter};

use rustc_abi::ExternAbi;
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_errors::{Applicability, Diag, DiagStyledString, IntoDiagArg, StringPart, pluralize};
use rustc_errors::{
Applicability, Diag, DiagStyledString, IntoDiagArg, MultiSpan, StringPart, pluralize,
};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::Visitor;
Expand All @@ -67,6 +69,7 @@ use rustc_middle::ty::{
self, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable,
TypeVisitableExt,
};
use rustc_span::def_id::LOCAL_CRATE;
use rustc_span::{BytePos, DesugaringKind, Pos, Span, sym};
use tracing::{debug, instrument};

Expand Down Expand Up @@ -211,7 +214,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}

/// Adds a note if the types come from similarly named crates
fn check_and_note_conflicting_crates(&self, err: &mut Diag<'_>, terr: TypeError<'tcx>) {
fn check_and_note_conflicting_crates(&self, err: &mut Diag<'_>, terr: TypeError<'tcx>) -> bool {
// FIXME(estebank): unify with `report_similar_impl_candidates`. The message is similar,
// even if the logic needed to detect the case is very different.
use hir::def_id::CrateNum;
use rustc_hir::definitions::DisambiguatedDefPathData;
use ty::GenericArg;
Expand Down Expand Up @@ -285,7 +290,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}
}

let report_path_match = |err: &mut Diag<'_>, did1: DefId, did2: DefId| {
let report_path_match = |err: &mut Diag<'_>, did1: DefId, did2: DefId, ty: &str| -> bool {
// Only report definitions from different crates. If both definitions
// are from a local module we could have false positives, e.g.
// let _ = [{struct Foo; Foo}, {struct Foo; Foo}];
Expand All @@ -297,24 +302,112 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {

// We compare strings because DefPath can be different
// for imported and non-imported crates
let expected_str = self.tcx.def_path_str(did1);
let found_str = self.tcx.def_path_str(did2);
let Ok(expected_abs) = abs_path(did1) else { return false };
let Ok(found_abs) = abs_path(did2) else { return false };
let same_path = || -> Result<_, PrintError> {
Ok(self.tcx.def_path_str(did1) == self.tcx.def_path_str(did2)
|| abs_path(did1)? == abs_path(did2)?)
Ok(expected_str == found_str || expected_abs == found_abs)
};
// We want to use as unique a type path as possible. If both types are "locally
// known" by the same name, we use the "absolute path" which uses the original
// crate name instead.
let (expected, found) = if expected_str == found_str {
(expected_abs.join("::"), found_abs.join("::"))
} else {
(expected_str.clone(), found_str.clone())
};
if same_path().unwrap_or(false) {
let crate_name = self.tcx.crate_name(did1.krate);
let msg = if did1.is_local() || did2.is_local() {
// We've displayed "expected `a::b`, found `a::b`". We add context to
// differentiate the different cases where that might happen.
let expected_crate_name = self.tcx.crate_name(did1.krate);
let found_crate_name = self.tcx.crate_name(did2.krate);
let same_crate = expected_crate_name == found_crate_name;
let expected_sp = self.tcx.def_span(did1);
let found_sp = self.tcx.def_span(did2);

let both_direct_dependencies = if !did1.is_local()
&& !did2.is_local()
&& let Some(data1) = self.tcx.extern_crate(did1.krate)
&& let Some(data2) = self.tcx.extern_crate(did2.krate)
&& data1.dependency_of == LOCAL_CRATE
&& data2.dependency_of == LOCAL_CRATE
{
// If both crates are directly depended on, we don't want to mention that
// in the final message, as it is redundant wording.
// We skip the case of semver trick, where one version of the local crate
// depends on another version of itself by checking that both crates at play
// are not the current one.
true
} else {
false
};

let mut span: MultiSpan = vec![expected_sp, found_sp].into();
span.push_span_label(
self.tcx.def_span(did1),
format!("this is the expected {ty} `{expected}`"),
);
span.push_span_label(
self.tcx.def_span(did2),
format!("this is the found {ty} `{found}`"),
);
for def_id in [did1, did2] {
let crate_name = self.tcx.crate_name(def_id.krate);
if !def_id.is_local()
&& let Some(data) = self.tcx.extern_crate(def_id.krate)
{
let descr = if same_crate {
"one version of".to_string()
} else {
format!("one {ty} comes from")
};
let dependency = if both_direct_dependencies {
if let rustc_session::cstore::ExternCrateSource::Extern(def_id) =
data.src
&& let Some(name) = self.tcx.opt_item_name(def_id)
{
format!(", which is renamed locally to `{name}`")
} else {
String::new()
}
} else if data.dependency_of == LOCAL_CRATE {
", as a direct dependency of the current crate".to_string()
} else {
let dep = self.tcx.crate_name(data.dependency_of);
format!(", as a dependency of crate `{dep}`")
};
span.push_span_label(
data.span,
format!("{descr} crate `{crate_name}` used here{dependency}"),
);
}
}
let msg = if (did1.is_local() || did2.is_local()) && same_crate {
format!(
"the crate `{expected_crate_name}` is compiled multiple times, \
possibly with different configurations",
)
} else if same_crate {
format!(
"the crate `{crate_name}` is compiled multiple times, possibly with different configurations"
"two different versions of crate `{expected_crate_name}` are being \
used; two types coming from two different versions of the same crate \
are different types even if they look the same",
)
} else {
format!(
"perhaps two different versions of crate `{crate_name}` are being used?"
"two types coming from two different crates are different types even \
if they look the same",
)
};
err.note(msg);
err.span_note(span, msg);
if same_crate {
err.help("you can use `cargo tree` to explore your dependency tree");
}
return true;
}
}
false
};
match terr {
TypeError::Sorts(ref exp_found) => {
Expand All @@ -323,14 +416,15 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
if let (&ty::Adt(exp_adt, _), &ty::Adt(found_adt, _)) =
(exp_found.expected.kind(), exp_found.found.kind())
{
report_path_match(err, exp_adt.did(), found_adt.did());
return report_path_match(err, exp_adt.did(), found_adt.did(), "type");
}
}
TypeError::Traits(ref exp_found) => {
report_path_match(err, exp_found.expected, exp_found.found);
return report_path_match(err, exp_found.expected, exp_found.found, "trait");
}
_ => (), // FIXME(#22750) handle traits and stuff
}
false
}

fn note_error_origin(
Expand Down Expand Up @@ -1409,6 +1503,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
label_or_note(span, terr.to_string(self.tcx));
}

if self.check_and_note_conflicting_crates(diag, terr) {
return;
}

if let Some((expected, found, path)) = expected_found {
let (expected_label, found_label, exp_found) = match exp_found {
Mismatch::Variable(ef) => (
Expand Down Expand Up @@ -1470,15 +1568,17 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
|prim: Ty<'tcx>, shadow: Ty<'tcx>, defid: DefId, diag: &mut Diag<'_>| {
let name = shadow.sort_string(self.tcx);
diag.note(format!(
"{prim} and {name} have similar names, but are actually distinct types"
"`{prim}` and {name} have similar names, but are actually distinct types"
));
diag.note(format!(
"one `{prim}` is a primitive defined by the language",
));
diag.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")
format!("the other {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}`")
format!("the other {name} is defined in crate `{crate_name}`")
};
diag.span_note(def_span, msg);
};
Expand Down Expand Up @@ -1666,8 +1766,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}
}

self.check_and_note_conflicting_crates(diag, terr);

self.note_and_explain_type_err(diag, terr, cause, span, cause.body_id.to_def_id());
if let Some(exp_found) = exp_found
&& let exp_found = TypeError::Sorts(exp_found)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1745,9 +1745,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
};
(
data.span,
format!(
"one version of crate `{crate_name}` is used here, as a {dependency}"
),
format!("one version of crate `{crate_name}` used here, as a {dependency}"),
)
})
{
Expand Down
16 changes: 8 additions & 8 deletions tests/incremental/circular-dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
//@ [cfail2] compile-flags: --test --extern aux={{build-base}}/circular-dependencies/auxiliary/libcircular_dependencies_aux.rmeta -L dependency={{build-base}}/circular-dependencies

pub struct Foo;
//[cfail2]~^ NOTE `Foo` is defined in the current crate
//[cfail2]~| NOTE `Foo` is defined in the current crate
//[cfail2]~| NOTE `circular_dependencies::Foo` is defined in crate `circular_dependencies`
//[cfail2]~| NOTE `circular_dependencies::Foo` is defined in crate `circular_dependencies`
//[cfail2]~^ NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations
//[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations
//[cfail2]~| NOTE this is the expected type `Foo`
//[cfail2]~| NOTE this is the expected type `circular_dependencies::Foo`
//[cfail2]~| NOTE this is the found type `Foo`
//[cfail2]~| NOTE this is the found type `circular_dependencies::Foo`

pub fn consume_foo(_: Foo) {}
//[cfail2]~^ NOTE function defined here
Expand All @@ -24,14 +26,12 @@ fn test() {
//[cfail2]~^ ERROR mismatched types [E0308]
//[cfail2]~| NOTE expected `circular_dependencies::Foo`, found `Foo`
//[cfail2]~| NOTE arguments to this function are incorrect
//[cfail2]~| NOTE `Foo` and `circular_dependencies::Foo` have similar names, but are actually distinct types
//[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations
//[cfail2]~| NOTE function defined here
//[cfail2]~| NOTE one version of crate `circular_dependencies` used here, as a dependency of crate `circular_dependencies_aux`
//[cfail2]~| NOTE one version of crate `circular_dependencies` used here, as a dependency of crate `circular_dependencies_aux`

consume_foo(aux::produce_foo());
//[cfail2]~^ ERROR mismatched types [E0308]
//[cfail2]~| NOTE expected `Foo`, found `circular_dependencies::Foo`
//[cfail2]~| NOTE arguments to this function are incorrect
//[cfail2]~| NOTE `circular_dependencies::Foo` and `Foo` have similar names, but are actually distinct types
//[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ note: there are multiple different versions of crate `foo` in the dependency gra
--> foo-current.rs:7:1
|
4 | extern crate foo;
| ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate
| ----------------- one version of crate `foo` used here, as a direct dependency of the current crate
5 |
6 | pub struct Struct;
| ----------------- this type implements the required trait
Expand Down
3 changes: 3 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ pub trait Trait {
fn foo(&self);
fn bar();
}
pub trait Trait2 {}
impl Trait for Type {
fn foo(&self) {}
fn bar() {}
}
pub fn do_something<X: Trait>(_: X) {}
pub fn do_something_type(_: Type) {}
pub fn do_something_trait(_: Box<dyn Trait2>) {}
4 changes: 4 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ pub trait Trait {
fn foo(&self);
fn bar();
}
pub trait Trait2 {}
impl Trait2 for Type {}
impl Trait for Type {
fn foo(&self) {}
fn bar() {}
}
pub fn do_something<X: Trait>(_: X) {}
pub fn do_something_type(_: Type) {}
pub fn do_something_trait(_: Box<dyn Trait2>) {}
2 changes: 1 addition & 1 deletion tests/run-make/crate-loading/multiple-dep-versions-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![crate_type = "rlib"]

extern crate dependency;
pub use dependency::Type;
pub use dependency::{Trait2, Type, do_something_trait, do_something_type};
pub struct OtherType;
impl dependency::Trait for OtherType {
fn foo(&self) {}
Expand Down
6 changes: 4 additions & 2 deletions tests/run-make/crate-loading/multiple-dep-versions.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
extern crate dep_2_reexport;
extern crate dependency;
use dep_2_reexport::{OtherType, Type};
use dependency::{Trait, do_something};
use dep_2_reexport::{OtherType, Trait2, Type};
use dependency::{Trait, do_something, do_something_trait, do_something_type};

fn main() {
do_something(Type);
Type.foo();
Type::bar();
do_something(OtherType);
do_something_type(Type);
do_something_trait(Box::new(Type) as Box<dyn Trait2>);
}
Loading

0 comments on commit 5e34106

Please sign in to comment.