Don't print name resolution errors if a crate fails to load

[jyn514]

Given the following code: 7342286

The current output is: several hundred thousand lines long. https://github.com/rust-lang/rust/runs/6331341497?check_suite_focus=true#step:26:783

Ideally the output should look like: not that. Just the errors about crate loading:

error[E0464]: multiple matching crates for `core`
   --> /cargo/registry/src/github.com-1ecc6299db9ec823/scopeguard-1.1.0/src/lib.rs:192:1
    |
192 | extern crate core as std;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: candidates:
            crate `core`: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-19a6c7cb160f2c97.rmeta
            crate `core`: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-7d5425aa48f72471.rlib

The failures from name resolution are almost always because of unresolved imports from the missing crate, and showing them hurts more than it helps.

@rustbot label +A-resolve

[jyn514]

I tried this diff:

Details

diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index 62485beac47..f1900700946 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -1537,19 +1537,26 @@ fn macro_def(&self, mut ctxt: SyntaxContext) -> DefId {
     }
 
     /// Entry point to crate resolution.
-    pub fn resolve_crate(&mut self, krate: &Crate) {
+    pub fn resolve_crate(&mut self, krate: &Crate) -> Result<(), ErrorGuaranteed> {
         self.session.time("resolve_crate", || {
             self.session.time("finalize_imports", || ImportResolver { r: self }.finalize_imports());
             self.session.time("resolve_access_levels", || {
                 AccessLevelsVisitor::compute_access_levels(self, krate)
             });
             self.session.time("finalize_macro_resolutions", || self.finalize_macro_resolutions());
+            // If we can't find all imports, we run into cascading errors where all imported items are marked as unresolved.
+            // Avoid that by aborting early if we can't resolve all imports.
+            if let Some(reported) = self.session.has_errors() {
+                return Err(reported);
+            }
             self.session.time("late_resolve_crate", || self.late_resolve_crate(krate));
             self.session.time("resolve_main", || self.resolve_main());
             self.session.time("resolve_check_unused", || self.check_unused(krate));
             self.session.time("resolve_report_errors", || self.report_errors(krate));
             self.session.time("resolve_postprocess", || self.crate_loader.postprocess(krate));
-        });
+
+            Ok(())
+        })
     }
 
     pub fn traits_in_scope(

which I think would work, and additionally have the benefit of aborting early soon after compile_error!s are emitted and before type checking / late resolution. But fixing all 200 tests that fail (and making sure they keep the original meaning) is going to be a pain.

cc @petrochenkov - before I spend more time on this, do you think it's a good idea?

[petrochenkov]

I don't think that's a good idea.

In cases like

use my::import; // unresolved import

let x = import::y;

we should already skip the error on import::y specifically.
If that doesn't happens in some cases then we should fix it.

Unresolved names in https://github.com/rust-lang/rust/runs/6331341497?check_suite_focus=true#step:26:783 mostly come from the standard library prelude.
It means the standard library specifically is not found, not some random crate, and I don't think we should optimize diagnostics for this case.

[jyn514]

@petrochenkov here is a more realistic case:

use regx::*;

fn main() {
    Regex::new("[0-9]+").unwrap();
}

That gives an error about code that would be correct if the import were resolved:

error[E0433]: failed to resolve: use of undeclared type `Regex`
 --> src/main.rs:4:5
  |
4 |     Regex::new("[0-9]+").unwrap();
  |     ^^^^^ not found in this scope

I've seen this happen for things besides glob imports in the past but I don't remember them off the top of my head; I'll post here if I come across one.

[jyn514]

Oh, here's another good one: when macros fail to expand, you get cascading errors about the items that they fail to define. https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b1a05f74b54686ec591b0782ce07a4ac

macro_rules! m {
    () => {
        cost X: &str = "hi";
        const Y: &str = "hello";
    }
}

m!();

fn main() {
    println!("{} {}", X, Y);
}

[jyn514]

Another cascading error from type check: the quiche crate fails to load and for some reason it breaks unrelated code.

error[E0433]: failed to resolve: use of undeclared crate or module `quiche`
  --> src/config.rs:56:30
   |
56 |     let mut config = quiche::Config::new(quiche::PROTOCOL_VERSION).unwrap();
   |                              ^^^^^^ not found in `quiche`
   |
help: consider importing this struct
   |
1  | use crate::Config;
   |
help: if you import `Config`, refer to it directly
   |
56 -     let mut config = quiche::Config::new(quiche::PROTOCOL_VERSION).unwrap();
56 +     let mut config = Config::new(quiche::PROTOCOL_VERSION).unwrap();
   | 

error: future cannot be sent between threads safely
   --> src/connection.rs:78:22
    |
78  |           tokio::spawn(WriteWorker::run(
    |  ______________________^
79  | |             Arc::clone(&conn.transport),
80  | |             flush_notify,
81  | |             writer_cfg,
82  | |             conns,
83  | |             audit_log_stats,
84  | |         ));
    | |_________^ future returned by `run` is not `Send`
    |
    = help: within `ConnectionMap<A>`, the trait `std::marker::Send` is not implemented for `NonNull<u8>`
note: future is not `Send` as this value is used across an await
   --> src/io_workers.rs:109:37
    |
87  |         conns: ConnectionPool<A>,
    |         ----- has type `Arc<std::sync::RwLock<ConnectionMap<A>>>` which is not `Send`
...
109 |         worker.write_loop(transport).await;
    |                                     ^^^^^^ await occurs here, with `conns` maybe used later
110 |     }
    |     - `conns` is later dropped here
note: required by a bound in `tokio::spawn`
   --> /home/jnelson/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.18.2/src/task/spawn.rs:127:21
    |
127 |         T: Future + Send + 'static,
    |                     ^^^^ required by this bound in `tokio::spawn`

[Nemo157]

It means the standard library specifically is not found, not some random crate, and I don't think we should optimize diagnostics for this case.

This is a common situation if you don't have a target installed

#0 14.21 error[E0463]: can't find crate for `core`
#0 14.21  --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/memchr-2.5.0/src/memchr/mod.rs:1:5
#0 14.21   |
#0 14.21 1 | use core::iter::Rev;
#0 14.21   |     ^^^^ can't find crate
#0 14.21   |
#0 14.21   = note: the `wasm32-unknown-unknown` target may not be installed
#0 14.21   = help: consider downloading the target with `rustup target add wasm32-unknown-unknown`
#0 14.21   = help: consider building the standard library from source with `cargo build -Zbuild-std`
... 214 multiline name resolution errors referring to things like `Some` ...