Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::{fmt, iter};
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::unord::UnordSet;
use rustc_errors::{Applicability, Diag, E0038, E0276, MultiSpan, struct_span_code_err};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
use rustc_hir::intravisit::Visitor;
use rustc_hir::{self as hir, AmbigArg};
use rustc_infer::traits::solve::Goal;
Expand All @@ -22,7 +22,8 @@ use rustc_infer::traits::{
};
use rustc_middle::ty::print::{PrintTraitRefExt as _, with_no_trimmed_paths};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt as _};
use rustc_span::{DesugaringKind, ErrorGuaranteed, ExpnKind, Span, Symbol};
use rustc_session::cstore::{ExternCrate, ExternCrateSource};
use rustc_span::{DesugaringKind, ErrorGuaranteed, ExpnKind, Span};
use tracing::{info, instrument};

pub use self::overflow::*;
Expand Down Expand Up @@ -353,14 +354,37 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}
}

fn get_extern_crate_renamed_symbol(&self, trait_def_id: DefId) -> Option<Symbol> {
if !trait_def_id.is_local()
&& let Some(data) = self.tcx.extern_crate(trait_def_id.krate)
&& let rustc_session::cstore::ExternCrateSource::Extern(def_id) = data.src
{
self.tcx.opt_item_name(def_id)
} else {
None
/// If the crates of `expected_def_id` and `trait_def_id` are imported as extern crate
/// under the same name (`extern crate foo as a` and `extern crate bar as a`) returns true,
/// otherwise returns false.
fn extern_crates_with_the_same_name(
&self,
expected_def_id: DefId,
trait_def_id: DefId,
) -> bool {
if expected_def_id.is_local() || trait_def_id.is_local() {
return false;
}
// We only compare direct dependencies of the current crate, so it avoids unnecessary
// processing and excludes indirect dependencies, like `std` or `core`. In such a case
// both would be imported under the same name `std`.
match (
self.tcx.extern_crate(expected_def_id.krate),
self.tcx.extern_crate(trait_def_id.krate),
) {
(
Some(ExternCrate {
src: ExternCrateSource::Extern(expected_def_id),
dependency_of: LOCAL_CRATE,
..
}),
Some(ExternCrate {
src: ExternCrateSource::Extern(trait_def_id),
dependency_of: LOCAL_CRATE,
..
}),
) => self.tcx.item_name(expected_def_id) == self.tcx.item_name(trait_def_id),
_ => false,
}
}

Expand All @@ -377,13 +401,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
{
let krate = self.tcx.crate_name(expected_did.krate);
let name = self.tcx.item_name(expected_did);
let locally_renamed_krate = self
.get_extern_crate_renamed_symbol(expected_did)
.map_or(None, |s| if s != krate { Some(s) } else { None });
let definitions_with_same_path: UnordSet<_> = found_dids
.filter(|def_id| {
def_id.krate != expected_did.krate
&& (locally_renamed_krate == self.get_extern_crate_renamed_symbol(*def_id)
&& (self.extern_crates_with_the_same_name(expected_did, *def_id)
|| self.tcx.crate_name(def_id.krate) == krate)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that || still necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it's still necessary. Because you want to detect different crates imported as the same name and different crates with the same crate name (typically the same crate in a different version).

Required by the following tests:

  • tests/incremental/circular-dependencies
  • tests/run-make/crate-loading
  • tests/run-make/crate-loading-crate-depends-on-itself
  • tests/run-make/type-mismatch-same-crate-name

&& self.tcx.item_name(def_id) == name
})
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/traits/auxiliary/crate1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//@ aux-crate:crate2=crate2.rs

pub trait Trait {}

pub fn foo(_arg: impl Trait) {}

pub fn bar(_arg: impl crate2::Trait) {}
1 change: 1 addition & 0 deletions tests/ui/traits/auxiliary/crate2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub trait Trait {}
12 changes: 12 additions & 0 deletions tests/ui/traits/wrong-multiple-different-versions-of-a-crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Test that we do not report false positives of a crate as being found multiple times in the
// dependency graph with different versions, when this crate is only present once. In this test,
// this was happening for `crate1` when two different crates in the dependencies were imported
// as ExternCrateSource::Path.
// Issue #148892.
//@ aux-crate:crate1=crate1.rs

struct MyStruct; //~ HELP the trait `Trait` is not implemented for `MyStruct`

fn main() {
crate1::foo(MyStruct); //~ ERROR the trait bound `MyStruct: Trait` is not satisfied
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0277]: the trait bound `MyStruct: Trait` is not satisfied
--> $DIR/wrong-multiple-different-versions-of-a-crate.rs:11:17
|
LL | crate1::foo(MyStruct);
| ----------- ^^^^^^^^ unsatisfied trait bound
| |
| required by a bound introduced by this call
|
help: the trait `Trait` is not implemented for `MyStruct`
--> $DIR/wrong-multiple-different-versions-of-a-crate.rs:8:1
|
LL | struct MyStruct;
| ^^^^^^^^^^^^^^^
note: required by a bound in `foo`
--> $DIR/auxiliary/crate1.rs:5:23
|
LL | pub fn foo(_arg: impl Trait) {}
| ^^^^^ required by this bound in `foo`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
Loading