Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rustc shows unnecessary big error when glob reexport failed #81413

Closed
WaffleLapkin opened this issue Jan 26, 2021 · 7 comments · Fixed by #113920
Closed

Rustc shows unnecessary big error when glob reexport failed #81413

WaffleLapkin opened this issue Jan 26, 2021 · 7 comments · Fixed by #113920
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name resolution C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@WaffleLapkin
Copy link
Member

I tried this code:

pub const ITEM: Item = Item;

pub struct Item;

pub fn item() {}

// Glob (*) is required to cause the error
pub use doesnt_exist::*;

mod a {
    use crate::{item, Item, ITEM};
}

mod b {
    use crate::item;
    use crate::Item;
    use crate::ITEM;
}

mod c {
    use crate::item;
}

(play)

I expected to see an error that only says that "doesnt_exist is unresolved".

But the error says that all uses of items from the module with glob import (crate in this case) are unresolved too:

error[E0432]: unresolved imports `doesnt_exist`, `crate::item`, `crate::Item`, `crate::ITEM`, `crate::item`, `crate::Item`, `crate::ITEM`, `crate::item`
  --> src/lib.rs:41:9
   |
41 | pub use doesnt_exist::*;
   |         ^^^^^^^^^^^^ use of undeclared crate or module `doesnt_exist`
...
44 |     use crate::{item, Item, ITEM};
   |                 ^^^^  ^^^^  ^^^^
...
48 |     use crate::item;
   |         ^^^^^^^^^^^
49 |     use crate::Item;
   |         ^^^^^^^^^^^
50 |     use crate::ITEM;
   |         ^^^^^^^^^^^
...
54 |     use crate::item;
   |         ^^^^^^^^^^^

This may lead to extremely long and unreadable errors. The issue was originally found while developing teloxide where it caused 1500+ lines error.

Meta

rustc --version --verbose:

rustc 1.51.0-nightly (c97f11af7 2021-01-10)
binary: rustc
commit-hash: c97f11af7bc4a6d3578f6a953be04ab2449a5728
commit-date: 2021-01-10
host: x86_64-unknown-linux-gnu
release: 1.51.0-nightly
@WaffleLapkin WaffleLapkin added the C-bug Category: This is a bug. label Jan 26, 2021
@SNCPlay42
Copy link
Contributor

Output on 1.38:
error[E0432]: unresolved import `doesnt_exist`
 --> <source>:8:9
  |
8 | pub use doesnt_exist::*;
  |         ^^^^^^^^^^^^ maybe a missing crate `doesnt_exist`?

warning: unused imports: `ITEM`, `Item`, `item`
  --> <source>:11:17
   |
11 |     use crate::{item, Item, ITEM};
   |                 ^^^^  ^^^^  ^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: unused import: `crate::item`
  --> <source>:15:9
   |
15 |     use crate::item;
   |         ^^^^^^^^^^^

warning: unused import: `crate::Item`
  --> <source>:16:9
   |
16 |     use crate::Item;
   |         ^^^^^^^^^^^

warning: unused import: `crate::ITEM`
  --> <source>:17:9
   |
17 |     use crate::ITEM;
   |         ^^^^^^^^^^^

warning: unused import: `crate::item`
  --> <source>:21:9
   |
21 |     use crate::item;
   |         ^^^^^^^^^^^

error: aborting due to previous error

@rustbot label A-diagnostics A-resolve regression-from-stable-to-stable

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name resolution regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 27, 2021
@WaffleLapkin
Copy link
Member Author

@SNCPlay42 why do you think it's a "regression-from-stable-to-stable"? IMO the output from 1.38 is even worse

@SNCPlay42
Copy link
Contributor

The error is much shorter, and only points at the actual problem. The unused import warnings, of course, wouldn't be there if those imports were actually used.

@SNCPlay42
Copy link
Contributor

********************************************************************************
Regression in nightly-2019-09-11
********************************************************************************
...
found 6 bors merge commits in the specified range
  commit[0] 2019-09-09UTC: Auto merge of #64313 - Centril:rollup-7w8b67g, r=Centril
  commit[1] 2019-09-10UTC: Auto merge of #64321 - Centril:rollup-jsj5tpl, r=Centril
  commit[2] 2019-09-10UTC: Auto merge of #64333 - Centril:rollup-llhhr82, r=Centril
  commit[3] 2019-09-10UTC: Auto merge of #64329 - Mark-Simulacrum:rustdoc-log, r=GuillaumeGomez
  commit[4] 2019-09-10UTC: Auto merge of #60387 - Goirad:test-expansion, r=ollie27
  commit[5] 2019-09-10UTC: Auto merge of #64354 - Centril:rollup-oaq0xoi, r=Centril

From those rollups, #64054 looks relevant.

@WaffleLapkin
Copy link
Member Author

@SNCPlay42 ah, you are right, I've misread the output thinking that the warnings were errors too.

@osa1
Copy link
Contributor

osa1 commented Jan 30, 2021

I spent some time trying to debug this last week and I'm fairly sure #64054 is the PR that introduced this. I wasn't aware of that PR at the time but I spent a lot of time trying to understand what is "determinate" vs. "indeterminate" (which is unfortunately not documented) and I remember finding out that with glob import the import crate::item is becoming "indeterminate" while it's normally (without the glob import) it is "determinate", which is one of the things we check elsewhere when generating these warnings.

@Stupremee Stupremee added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 2, 2021
@Stupremee
Copy link
Member

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@Stupremee Stupremee added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Feb 2, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 31, 2023
fix(resolve): report unresolved imports firstly

Fixes rust-lang#81413

An easy fix, r? `@petrochenkov`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 31, 2023
fix(resolve): report unresolved imports firstly

Fixes rust-lang#81413

An easy fix, r? ``@petrochenkov``
@bors bors closed this as completed in 2de51cc Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name resolution C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants