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

E0432 and E0433 are the same error, only inconsistent #137577

Open
kornelski opened this issue Feb 25, 2025 · 0 comments
Open

E0432 and E0433 are the same error, only inconsistent #137577

kornelski opened this issue Feb 25, 2025 · 0 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.

Comments

@kornelski
Copy link
Contributor

Reporting of name resolution errors in rustc_resolve is complicated (often for good/necessary reasons). The problem is, there seems to be a significant amount of organically grown near-duplicate code for the error reporting. The same error can end up with different suggestions depending on how it was passed through the codebase.

I'm thinking about refactoring this:

  • Where possible, don't emit errors immediately. Return them to the caller, and collect in Resolver until the "resolve_report_errors" pass.
  • Where possible, instead of immediately computing error message and suggestion String, keep relevant info until "resolve_report_errors" pass to compute suggestions in diagnostics.rs.
  • Where possible, use structs from rustc_errors for reporting localizable errors.
  • Perform grouping and deduplication across multiple errors, as late as possible.

Is this the right direction?

More on the current state:

For example, a path resolution in use foo::typ0; gets a "did you mean?" spelling suggestion. The same path, resolved using the same function, failing for the same reason, won't get any suggestions if it's in pub(in foo::typ0), because the original resolution error is converted to a different error type first, skipping lookup of candidates.

There are two places that can generate "could not find `foo` in the crate root" message. I think one of them is unreachable.

The same name resolution error can go through different error reporting code depending whether the first, middle, or last segment of the path failed, even if the reason is the same:

use crate::module::function::err;
// ^ expected type, found function `function` in `module`

use crate::function::err;
// ^ could not find `function` in the crate root

// the second one is wrong; it should have been "expected type, found function `function` in in the crate root",
// but the second case tries emitting another suggestion first, which suppresses the relevant suggestion

pub fn function() {
}

mod module {
    pub fn function() {
    }
}

There's PathResult::Failed, UnresolvedImportError, VisResolutionError::FailedToResolve, ResolutionError::FailedToResolve, and the same failure can end up being in any one of these.

Some errors are emitted immediately as they happen, usually via report_error(). Some are bubbled up. Some of the bubbled up ones are aggregated for batches of throw_unresolved_import_error, and some are emitted before. The same name resolution error going through the batching codepath is E0432, and reporting it earlier gives it E0433.

Some errors are buffered in Resolver to be deduped and enhanced during the "resolve_report_errors" phase, but most errors are reported earlier in "finalize_imports" phase. There's a different dedupe in finalize_imports, for a different subset of errors. There's a hack for glob import failures suppressing most errors, even syntax errors unrelated to globs.

Motivation for this:

I wanted to add a suggestion for when user writes use crate::missingmod and the file src/missingmod.rs exists on disk, but there's no mod missingmod in the code.

The problem is that the import/type paths are resolved in lots of contexts that report errors in different ways. Any segment of the import path could be a missing mod, but errors about first/middle/last segment go through very different suggestion-generating code.

It's even worse than just duplication of the code for generating the suggestion. The suggestion needs a span to point out the correct place to insert the required mod in parent module's body, but this information is not easily available. It requires walking the ast, which can't be done from most places where the suggestions are currently computed. There's code in diagnostics.rs that has already implemented finding location for suggesting use $ident, and that would work for mod $ident too, but all the relevant errors are emitted early and never make it that far.

@kornelski kornelski added the C-bug Category: This is a bug. label Feb 25, 2025
@compiler-errors compiler-errors added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. labels Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.
Projects
None yet
Development

No branches or pull requests

2 participants