From 8574f374e2cc27b53c8b81dc4031c59ca3035284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 Nov 2024 17:55:52 +0000 Subject: [PATCH 1/3] Do not call `extern_crate` on current trait on crate mismatch errors When we encounter an error caused by traits/types of different versions of the same crate, filter out the current crate when collecting spans to add to the context so we don't call `extern_crate` on the `DefId` of the current crate, which is meaningless and ICEs. Produced output with this filter: ``` error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied --> y.rs:13:19 | 13 | check_trait::(); | ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct` | note: there are multiple different versions of crate `foo` in the dependency graph --> y.rs:7:1 | 4 | extern crate foo; | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate 5 | 6 | pub struct Struct; | ----------------- this type implements the required trait 7 | pub trait Trait {} | ^^^^^^^^^^^^^^^ this is the required trait | ::: x.rs:4:1 | 4 | pub struct Struct; | ----------------- this type doesn't implement the required trait 5 | pub trait Trait {} | --------------- this is the found trait = note: two types coming from two different versions of the same crate are different types even if they look the same = help: you can use `cargo tree` to explore your dependency tree note: required by a bound in `check_trait` --> y.rs:10:19 | 10 | fn check_trait() {} | ^^^^^ required by this bound in `check_trait` ``` Fix #133563. --- .../traits/fulfillment_errors.rs | 4 +++ .../foo-current.rs | 14 ++++++++ .../foo-prev.rs | 6 ++++ .../rmake.rs | 32 +++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 7519189452591..9b969dd3e43a6 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -1731,6 +1731,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { span.push_span_label(self.tcx.def_span(trait_def_id), "this is the required trait"); for (sp, label) in [trait_def_id, other_trait_def_id] .iter() + // The current crate-version might depend on another version of the same crate + // (Think "semver-trick"). Do not call `extern_crate` in that case for the local + // crate as that doesn't make sense and ICEs (#133563). + .filter(|def_id| !def_id.is_local()) .filter_map(|def_id| self.tcx.extern_crate(def_id.krate)) .map(|data| { let dependency = if data.dependency_of == LOCAL_CRATE { diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs b/tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs new file mode 100644 index 0000000000000..71b27cd85bfed --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs @@ -0,0 +1,14 @@ +#![crate_type = "lib"] +#![crate_name = "foo"] + +extern crate foo; + +pub struct Struct; +pub trait Trait {} +impl Trait for Struct {} + +fn check_trait() {} + +fn ice() { + check_trait::(); +} diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs b/tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs new file mode 100644 index 0000000000000..19d3f3c972b09 --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs @@ -0,0 +1,6 @@ +#![crate_type = "lib"] +#![crate_name = "foo"] + +pub struct Struct; +pub trait Trait {} +impl Trait for Struct {} diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs new file mode 100644 index 0000000000000..f52f3d791a713 --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs @@ -0,0 +1,32 @@ +//@ only-linux +//@ ignore-wasm32 +//@ ignore-wasm64 +// ignore-tidy-linelength + +// Verify that if the current crate depends on a different version of the same crate, *and* types +// and traits of the different versions are mixed, we produce diagnostic output and not an ICE. +// #133563 + +use run_make_support::{rust_lib_name, rustc}; + +fn main() { + rustc().input("foo-prev.rs").run(); + + rustc() + .extra_filename("current") + .metadata("current") + .input("foo-current.rs") + .extern_("foo", rust_lib_name("foo")) + .run_fail() + .assert_stderr_contains(r#" +note: there are multiple different versions of crate `foo` in the dependency graph + --> 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 +5 | +6 | pub struct Struct; + | ----------------- this type implements the required trait +7 | pub trait Trait {} + | ^^^^^^^^^^^^^^^ this is the required trait"#); +} From e97e15dea55d61d68732bc030be1a44d8a51a1e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 29 Nov 2024 18:31:37 +0000 Subject: [PATCH 2/3] Use rmake `diff` output in test --- .../foo.stderr | 34 +++++++++++++++++++ .../rmake.rs | 25 +++++++------- 2 files changed, 46 insertions(+), 13 deletions(-) create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr b/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr new file mode 100644 index 0000000000000..36379429530bc --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr @@ -0,0 +1,34 @@ +error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied + --> foo-current.rs:13:19 + | +13 | check_trait::(); + | ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct` + | +note: there are multiple different versions of crate `foo` in the dependency graph + --> 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 +5 | +6 | pub struct Struct; + | ----------------- this type implements the required trait +7 | pub trait Trait {} + | ^^^^^^^^^^^^^^^ this is the required trait + | + ::: foo-prev.rs:X:Y + | +4 | pub struct Struct; + | ----------------- this type doesn't implement the required trait +5 | pub trait Trait {} + | --------------- this is the found trait + = note: two types coming from two different versions of the same crate are different types even if they look the same + = help: you can use `cargo tree` to explore your dependency tree +note: required by a bound in `check_trait` + --> foo-current.rs:10:19 + | +10 | fn check_trait() {} + | ^^^^^ required by this bound in `check_trait` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. \ No newline at end of file diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs index f52f3d791a713..57e0cab92f1ef 100644 --- a/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs +++ b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs @@ -7,26 +7,25 @@ // and traits of the different versions are mixed, we produce diagnostic output and not an ICE. // #133563 -use run_make_support::{rust_lib_name, rustc}; +use run_make_support::{diff, rust_lib_name, rustc}; fn main() { rustc().input("foo-prev.rs").run(); - rustc() + let out = rustc() .extra_filename("current") .metadata("current") .input("foo-current.rs") .extern_("foo", rust_lib_name("foo")) .run_fail() - .assert_stderr_contains(r#" -note: there are multiple different versions of crate `foo` in the dependency graph - --> 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 -5 | -6 | pub struct Struct; - | ----------------- this type implements the required trait -7 | pub trait Trait {} - | ^^^^^^^^^^^^^^^ this is the required trait"#); + .stderr_utf8(); + + // We don't remap the path of the `foo-prev` crate, so we remap it here. + let mut lines: Vec<_> = out.lines().collect(); + for line in &mut lines { + if line.starts_with(" ::: ") { + *line = " ::: foo-prev.rs:X:Y"; + } + } + diff().expected_file("foo.stderr").actual_text("(rustc)", &lines.join("\n")).run(); } From 998ff2f0cd2902e86178d35b01ba78fe4633f80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 29 Nov 2024 18:43:48 +0000 Subject: [PATCH 3/3] Move the `crate-loading` test to use `diff` output --- .../src/external_deps/rustc.rs | 6 + .../multiple-dep-versions.stderr | 127 ++++++++++++++++++ tests/run-make/crate-loading/rmake.rs | 95 +++---------- 3 files changed, 152 insertions(+), 76 deletions(-) create mode 100644 tests/run-make/crate-loading/multiple-dep-versions.stderr diff --git a/src/tools/run-make-support/src/external_deps/rustc.rs b/src/tools/run-make-support/src/external_deps/rustc.rs index 494daeca96364..ffe10092cc28a 100644 --- a/src/tools/run-make-support/src/external_deps/rustc.rs +++ b/src/tools/run-make-support/src/external_deps/rustc.rs @@ -227,6 +227,12 @@ impl Rustc { self } + /// Normalize the line number in the stderr output + pub fn ui_testing(&mut self) -> &mut Self { + self.cmd.arg(format!("-Zui-testing")); + self + } + /// Specify the target triple, or a path to a custom target json spec file. pub fn target>(&mut self, target: S) -> &mut Self { let target = target.as_ref(); diff --git a/tests/run-make/crate-loading/multiple-dep-versions.stderr b/tests/run-make/crate-loading/multiple-dep-versions.stderr new file mode 100644 index 0000000000000..5888aad8f37bc --- /dev/null +++ b/tests/run-make/crate-loading/multiple-dep-versions.stderr @@ -0,0 +1,127 @@ +error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied + --> replaced + | +LL | do_something(Type); + | ------------ ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` + | | + | required by a bound introduced by this call + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub struct Type(pub i32); + | --------------- this type implements the required trait +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the required trait + | + ::: 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 + | + ::: replaced + | +LL | pub struct Type; + | --------------- this type doesn't implement the required trait +LL | pub trait Trait { + | --------------- this is the found trait + = note: two types coming from two different versions of the same crate are different types even if they look the same + = help: you can use `cargo tree` to explore your dependency tree +note: required by a bound in `do_something` + --> replaced + | +LL | pub fn do_something(_: X) {} + | ^^^^^ required by this bound in `do_something` + +error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope + --> replaced + | +LL | Type.foo(); + | ^^^ method not found in `Type` + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the trait that is needed +LL | fn foo(&self); + | -------------- the method is available for `dep_2_reexport::Type` here + | + ::: replaced + | +LL | use dependency::{Trait, do_something}; + | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency` + | + ::: replaced + | +LL | pub trait Trait { + | --------------- this is the trait that was imported + +error[E0599]: no function or associated item named `bar` found for struct `dep_2_reexport::Type` in the current scope + --> replaced + | +LL | Type::bar(); + | ^^^ function or associated item not found in `Type` + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the trait that is needed +LL | fn foo(&self); +LL | fn bar(); + | --------- the associated function is available for `dep_2_reexport::Type` here + | + ::: replaced + | +LL | use dependency::{Trait, do_something}; + | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency` + | + ::: replaced + | +LL | pub trait Trait { + | --------------- this is the trait that was imported + +error[E0277]: the trait bound `OtherType: Trait` is not satisfied + --> replaced + | +LL | do_something(OtherType); + | ------------ ^^^^^^^^^ the trait `Trait` is not implemented for `OtherType` + | | + | required by a bound introduced by this call + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the required trait + | + ::: 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 + | + ::: replaced + | +LL | pub struct OtherType; + | -------------------- this type doesn't implement the required trait + | + ::: replaced + | +LL | pub trait Trait { + | --------------- this is the found trait + = help: you can use `cargo tree` to explore your dependency tree +note: required by a bound in `do_something` + --> replaced + | +LL | pub fn do_something(_: X) {} + | ^^^^^ required by this bound in `do_something` + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0277, E0599. +For more information about an error, try `rustc --explain E0277`. \ No newline at end of file diff --git a/tests/run-make/crate-loading/rmake.rs b/tests/run-make/crate-loading/rmake.rs index 544bf9ab957a4..6ad456e3e3e57 100644 --- a/tests/run-make/crate-loading/rmake.rs +++ b/tests/run-make/crate-loading/rmake.rs @@ -3,7 +3,7 @@ //@ ignore-wasm64 // ignore-tidy-linelength -use run_make_support::{rust_lib_name, rustc}; +use run_make_support::{diff, rust_lib_name, rustc}; fn main() { rustc().input("multiple-dep-versions-1.rs").run(); @@ -13,83 +13,26 @@ fn main() { .extern_("dependency", rust_lib_name("dependency2")) .run(); - rustc() + let out = rustc() .input("multiple-dep-versions.rs") .extern_("dependency", rust_lib_name("dependency")) .extern_("dep_2_reexport", rust_lib_name("foo")) + .ui_testing() .run_fail() - .assert_stderr_contains(r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied - --> multiple-dep-versions.rs:7:18 - | -7 | do_something(Type); - | ------------ ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` - | | - | required by a bound introduced by this call - | -note: there are multiple different versions of crate `dependency` in the dependency graph"#) - .assert_stderr_contains(r#" -3 | pub struct Type(pub i32); - | --------------- this type implements the required trait -4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the required trait -"#) - .assert_stderr_contains(r#" -1 | extern crate dep_2_reexport; - | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` -2 | extern crate dependency; - | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate"#) - .assert_stderr_contains(r#" -3 | pub struct Type; - | --------------- this type doesn't implement the required trait -4 | pub trait Trait { - | --------------- this is the found trait - = note: two types coming from two different versions of the same crate are different types even if they look the same - = help: you can use `cargo tree` to explore your dependency tree"#) - .assert_stderr_contains(r#"note: required by a bound in `do_something`"#) - .assert_stderr_contains(r#" -12 | pub fn do_something(_: X) {} - | ^^^^^ required by this bound in `do_something`"#) - .assert_stderr_contains(r#"error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope - --> multiple-dep-versions.rs:8:10 - | -8 | Type.foo(); - | ^^^ method not found in `Type` - | -note: there are multiple different versions of crate `dependency` in the dependency graph"#) - .assert_stderr_contains(r#" -4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the trait that is needed -5 | fn foo(&self); - | -------------- the method is available for `dep_2_reexport::Type` here - | - ::: multiple-dep-versions.rs:4:18 - | -4 | use dependency::{Trait, do_something}; - | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#) - .assert_stderr_contains(r#" -4 | pub trait Trait { - | --------------- this is the trait that was imported"#) - .assert_stderr_contains(r#" -error[E0599]: no function or associated item named `bar` found for struct `dep_2_reexport::Type` in the current scope - --> multiple-dep-versions.rs:9:11 - | -9 | Type::bar(); - | ^^^ function or associated item not found in `Type` - | -note: there are multiple different versions of crate `dependency` in the dependency graph"#) - .assert_stderr_contains(r#" -4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the trait that is needed -5 | fn foo(&self); -6 | fn bar(); - | --------- the associated function is available for `dep_2_reexport::Type` here - | - ::: multiple-dep-versions.rs:4:18 - | -4 | use dependency::{Trait, do_something}; - | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#) - .assert_stderr_contains( - r#" -6 | pub struct OtherType; - | -------------------- this type doesn't implement the required trait"#); + .stderr_utf8(); + + // We don't remap all the paths, so we remap it here. + let mut lines: Vec<_> = out.lines().collect(); + for line in &mut lines { + if line.starts_with(" --> ") { + *line = " --> replaced"; + } + if line.starts_with(" ::: ") { + *line = " ::: replaced"; + } + } + diff() + .expected_file("multiple-dep-versions.stderr") + .actual_text("(rustc)", &lines.join("\n")) + .run(); }