Skip to content

Silence some resolve errors when there have been glob import errors #125381

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 29, 2024

Conversation

estebank
Copy link
Contributor

When encountering use foo::*; where foo fails to be found, and we later encounter resolution errors, we silence those later errors.

A single case of the above, for an existing import on a big codebase would otherwise have a huge number of knock-down spurious errors.

Ideally, instead of a global flag to silence all subsequent resolve errors, we'd want to introduce an unnameable binding in the appropriate rib as a sentinel when there's a failed glob import, so when we encounter a resolve error we can search for that sentinel and if found, and only then, silence that error. The current approach is just a quick proof of concept to iterate over.

Partially address #96799.

@rustbot
Copy link
Collaborator

rustbot commented May 21, 2024

r? @pnkfelix

rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 21, 2024
Comment on lines 550 to 553
if let ImportKind::Glob { .. } = import.kind {
self.glob_error = true;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrochenkov I've tried injecting a nameless binding as a sentinel with things like the following here:

self.per_ns(|this, ns| {
    this.define(import.parent_scope.module, Ident::with_dummy_span(kw::Empty), ns, this.dummy_binding);
});

but then I couldn't find those bindings back in smart_resolve_path_fragment. I have access to the module and turn glob_error into a FxHashSet<DefId> that I check later before emitting the subsequent resolve errors, but that sounds like a bigger hack, inaccurate at that, effectively approximating normal resolve, so I'd like to figure out a way of injecting an unnameable binding as a sentinel so that the regular machinery does its thing, and the error silencing is always accurate (a bad use foo::*; won't affect an unrelated scope).

@petrochenkov petrochenkov self-assigned this May 21, 2024
@pnkfelix
Copy link
Member

This looks fine to me, but I want to cede review authority to petrochenkov who already self-assigned.

r? @petrochenkov

@rustbot
Copy link
Collaborator

rustbot commented May 23, 2024

Could not assign reviewer from: petrochenkov.
User(s) petrochenkov are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

@pnkfelix pnkfelix removed their assignment May 23, 2024
@petrochenkov
Copy link
Contributor

I'm surprised how little effect this had on the test suite.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 24, 2024
@estebank
Copy link
Contributor Author

It now uses ErrorGuaranteed.

I'm surprised how little effect this had on the test suite.

I believe that the only situation where this is a problem is during changes to existing codebases, which is a case that is not well represented in our test suite.

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 27, 2024
@petrochenkov
Copy link
Contributor

r=me with minor cleanup (#125381 (comment)).
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 28, 2024
When encountering `use foo::*;` where `foo` fails to be found, and we later
encounter resolution errors, we silence those later errors.

A single case of the above, for an *existing* import on a big codebase would
otherwise have a huge number of knock-down spurious errors.

Ideally, instead of a global flag to silence all subsequent resolve errors,
we'd want to introduce an unameable binding in the appropriate rib as a
sentinel when there's a failed glob import, so when we encounter a resolve
error we can search for that sentinel and if found, and only then, silence
that error. The current approach is just a quick proof of concept to
iterate over.

Partially address rust-lang#96799.
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented May 28, 2024

📌 Commit 37c54db has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 28, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request May 28, 2024
Silence some resolve errors when there have been glob import errors

When encountering `use foo::*;` where `foo` fails to be found, and we later encounter resolution errors, we silence those later errors.

A single case of the above, for an *existing* import on a big codebase would otherwise have a huge number of knock-down spurious errors.

Ideally, instead of a global flag to silence all subsequent resolve errors, we'd want to introduce an unnameable binding in the appropriate rib as a sentinel when there's a failed glob import, so when we encounter a resolve error we can search for that sentinel and if found, and only then, silence that error. The current approach is just a quick proof of concept to iterate over.

Partially address rust-lang#96799.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 29, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#124251 (Add an intrinsic for `ptr::metadata`)
 - rust-lang#124320 (Add `--print=check-cfg` to get the expected configs)
 - rust-lang#125226 (Make more of the test suite run on Mac Catalyst)
 - rust-lang#125381 (Silence some resolve errors when there have been glob import errors)
 - rust-lang#125633 (miri: avoid making a full copy of all new allocations)
 - rust-lang#125638 (Rewrite `lto-smoke`, `simple-rlib` and `mixing-deps` `run-make` tests in `rmake.rs` format)
 - rust-lang#125639 (Support `./x doc run-make-support --open`)
 - rust-lang#125664 (Tweak relations to no longer rely on `TypeTrace`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 29, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#124251 (Add an intrinsic for `ptr::metadata`)
 - rust-lang#124320 (Add `--print=check-cfg` to get the expected configs)
 - rust-lang#125226 (Make more of the test suite run on Mac Catalyst)
 - rust-lang#125381 (Silence some resolve errors when there have been glob import errors)
 - rust-lang#125633 (miri: avoid making a full copy of all new allocations)
 - rust-lang#125638 (Rewrite `lto-smoke`, `simple-rlib` and `mixing-deps` `run-make` tests in `rmake.rs` format)
 - rust-lang#125639 (Support `./x doc run-make-support --open`)
 - rust-lang#125664 (Tweak relations to no longer rely on `TypeTrace`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bc1a069 into rust-lang:master May 29, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 29, 2024
Rollup merge of rust-lang#125381 - estebank:issue-96799, r=petrochenkov

Silence some resolve errors when there have been glob import errors

When encountering `use foo::*;` where `foo` fails to be found, and we later encounter resolution errors, we silence those later errors.

A single case of the above, for an *existing* import on a big codebase would otherwise have a huge number of knock-down spurious errors.

Ideally, instead of a global flag to silence all subsequent resolve errors, we'd want to introduce an unnameable binding in the appropriate rib as a sentinel when there's a failed glob import, so when we encounter a resolve error we can search for that sentinel and if found, and only then, silence that error. The current approach is just a quick proof of concept to iterate over.

Partially address rust-lang#96799.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants