Rollup merge of rust-lang#133304 - lqd:issue-132920, r=estebank

Revert diagnostics hack to fix ICE 132920

This reverts 8a568d9 from rust-lang#128849 to fix the diagnostics ICE in rust-lang#132920.

The hack mentioned in that commit was supposed to be tailored to E277, but that codepath is used actually shared with other errors, e.g. at least the E283 from the linked issue.

We may have to eat the slightly worse diagnostics until a non-hacky way to make this error less verbose is implemented (or I guess a different hack specializing even more to E277's structure).

Sorry `@estebank` 🙏. I can close this if you'd prefer to fix it in a different way.

Since it seems unexpected that rust-lang#128849 would impact the repro, here's how the error used to look before that PR.

```console
warning: unused import: `minirapier::Ray`
 --> src/main.rs:2:5
  |
2 | use minirapier::Ray;
  |     ^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error[E0283]: type annotations needed
  --> src/main.rs:10:5
   |
10 |     insert_resource(Res.into());
   |     ^^^^^^^^^^^^^^^ ---------- type must be known at this point
   |     |
   |     cannot infer type of the type parameter `R` declared on the function `insert_resource`
   |
   = note: cannot satisfy `_: Resource`
   = help: the trait `Resource` is implemented for `Res`
note: required by a bound in `insert_resource`
  --> src/main.rs:4:23
   |
4  | fn insert_resource<R: Resource>(_resource: R) {}
   |                       ^^^^^^^^ required by this bound in `insert_resource`
help: consider specifying the generic argument
   |
10 |     insert_resource::<R>(Res.into());
   |                    +++++
help: consider removing this method call, as the receiver has type `Res` and `Res: Resource` trivially holds
   |
10 -     insert_resource(Res.into());
10 +     insert_resource(Res);
```

And how it looks now without the ICE.

```console
warning: unused import: `minirapier::Ray`
 --> src/main.rs:2:5
  |
2 | use minirapier::Ray;
  |     ^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error[E0283]: type annotations needed
  --> src/main.rs:10:5
   |
10 |     insert_resource(Res.into());
   |     ^^^^^^^^^^^^^^^ ---------- type must be known at this point
   |     |
   |     cannot infer type of the type parameter `R` declared on the function `insert_resource`
   |
   = note: cannot satisfy `_: Resource`
note: there are multiple different versions of crate `minibevy` in the dependency graph
  --> /home/lqd/rust/tmp/minimization/issue-132920/rustc-ice-version-conflict/minibevy_b/src/lib.rs:1:1
   |
1  | pub trait Resource {}
   | ^^^^^^^^^^^^^^^^^^ this is the required trait
   |
  ::: src/main.rs:1:5
   |
1  | use minibevy::Resource;
   |     -------- one version of crate `minibevy` is used here, as a direct dependency of the current crate
2  | use minirapier::Ray;
   |     ---------- one version of crate `minibevy` is used here, as a dependency of crate `minirapier`
   |
  ::: /home/lqd/rust/tmp/minimization/issue-132920/rustc-ice-version-conflict/minibevy_a/src/lib.rs:1:1
   |
1  | pub trait Resource {}
   | ------------------ this is the found trait
   = help: you can use `cargo tree` to explore your dependency tree
note: required by a bound in `insert_resource`
  --> src/main.rs:4:23
   |
4  | fn insert_resource<R: Resource>(_resource: R) {}
   |                       ^^^^^^^^ required by this bound in `insert_resource`
help: consider specifying the generic argument
   |
10 |     insert_resource::<R>(Res.into());
   |                    +++++
help: consider removing this method call, as the receiver has type `Res` and `Res: Resource` trivially holds
   |
10 -     insert_resource(Res.into());
10 +     insert_resource(Res);
   |
```

The improvements from rust-lang#128849 are still present and the note about the trait coming from the 2 versions of bevy is more explanatory/helpful than before, albeit a bit verbosely.

Fixes rust-lang#132920.

Author: jhpratt

compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

@@ -1803,24 +1803,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                 StringPart::highlighted("cargo tree".to_string()),
                 StringPart::normal("` to explore your dependency tree".to_string()),
             ]);
-
-            // FIXME: this is a giant hack for the benefit of this specific diagnostic. Because
-            // we're so nested in method calls before the error gets emitted, bubbling a single bit
-            // flag informing the top level caller to stop adding extra detail to the diagnostic,
-            // would actually be harder to follow. So we do something naughty here: we consume the
-            // diagnostic, emit it and leave in its place a "delayed bug" that will continue being
-            // modified but won't actually be printed to end users. This *is not ideal*, but allows
-            // us to reduce the verbosity of an error that is already quite verbose and increase its
-            // specificity. Below we modify the main message as well, in a way that *could* break if
-            // the implementation of Diagnostics change significantly, but that would be caught with
-            // a make test failure when this diagnostic is tested.
-            err.primary_message(format!(
-                "{} because the trait comes from a different crate version",
-                err.messages[0].0.as_str().unwrap(),
-            ));
-            let diag = err.clone();
-            err.downgrade_to_delayed_bug();
-            self.tcx.dcx().emit_diagnostic(diag);
             return true;
         }
 

tests/run-make/crate-loading/rmake.rs

@@ -18,31 +18,37 @@ fn main() {
         .extern_("dependency", rust_lib_name("dependency"))
         .extern_("dep_2_reexport", rust_lib_name("foo"))
         .run_fail()
-        .assert_stderr_contains(r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied because the trait comes from a different crate version
- --> multiple-dep-versions.rs:7:18
-  |
-7 |     do_something(Type);
-  |                  ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type`
-  |
+        .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
+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"#)
+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"#)
+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: Trait>(_: 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
   |

tests/run-make/diagnostics-traits-from-duplicate-crates/minibevy.rs

@@ -0,0 +1,2 @@
+pub trait Resource {}
+pub struct Ray2d;

tests/run-make/diagnostics-traits-from-duplicate-crates/minirapier.rs

@@ -0,0 +1 @@
+pub type Ray = minibevy::Ray2d;

tests/run-make/diagnostics-traits-from-duplicate-crates/repro.rs

@@ -0,0 +1,14 @@
+extern crate minibevy;
+extern crate minirapier;
+
+use minibevy::Resource;
+use minirapier::Ray;
+
+fn insert_resource<R: Resource>(_resource: R) {}
+
+struct Res;
+impl Resource for Res {}
+
+fn main() {
+    insert_resource(Res.into());
+}

tests/run-make/diagnostics-traits-from-duplicate-crates/rmake.rs

@@ -0,0 +1,45 @@
+// Non-regression test for issue #132920 where multiple versions of the same crate are present in
+// the dependency graph, and an unexpected error in a dependent crate caused an ICE in the
+// unsatisfied bounds diagnostics for traits present in multiple crate versions.
+//
+// Setup:
+// - two versions of the same crate: minibevy_a and minibevy_b
+// - minirapier: depends on minibevy_a
+// - repro: depends on minirapier and minibevy_b
+
+use run_make_support::rustc;
+
+fn main() {
+    // Prepare dependencies, mimicking a check build with cargo.
+    rustc()
+        .input("minibevy.rs")
+        .crate_name("minibevy")
+        .crate_type("lib")
+        .emit("metadata")
+        .metadata("a")
+        .extra_filename("-a")
+        .run();
+    rustc()
+        .input("minibevy.rs")
+        .crate_name("minibevy")
+        .crate_type("lib")
+        .emit("metadata")
+        .metadata("b")
+        .extra_filename("-b")
+        .run();
+    rustc()
+        .input("minirapier.rs")
+        .crate_name("minirapier")
+        .crate_type("lib")
+        .emit("metadata")
+        .extern_("minibevy", "libminibevy-a.rmeta")
+        .run();
+
+    // Building the main crate used to ICE here when printing the `type annotations needed` error.
+    rustc()
+        .input("repro.rs")
+        .extern_("minibevy", "libminibevy-b.rmeta")
+        .extern_("minirapier", "libminirapier.rmeta")
+        .run_fail()
+        .assert_stderr_not_contains("error: the compiler unexpectedly panicked. this is a bug");
+}