-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Supress some lookup errors if a module contains compile_error!
#149484
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
Open
ogoffart
wants to merge
4
commits into
rust-lang:main
Choose a base branch
from
ogoffart:fix-68838
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+122
−5
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
5869e5c
Supress some lookup errors if a module contains `compile_error!`
ogoffart 12ec9f2
Don't put non-module ids into the table
ogoffart 1472a97
Attempt to be more precise when looking for poinsoned module
ogoffart 9b59a87
Bless after rebase
ogoffart File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1718,7 +1718,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { | |
| diag_metadata: Option<&DiagMetadata<'_>>, | ||
| ) -> PathResult<'ra> { | ||
| let mut module = None; | ||
| let mut module_had_parse_errors = false; | ||
| let mut module_had_parse_errors = !self.mods_with_parse_errors.is_empty() | ||
| && self.mods_with_parse_errors.contains(&parent_scope.module.nearest_parent_mod()); | ||
| let mut allow_super = true; | ||
| let mut second_binding = None; | ||
|
|
||
|
|
@@ -1834,6 +1835,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { | |
| } | ||
|
|
||
| let binding = if let Some(module) = module { | ||
| if !self.mods_with_parse_errors.is_empty() | ||
| && let ModuleOrUniformRoot::Module(m) = module | ||
| && m.res() | ||
| .and_then(|r| r.module_like_def_id()) | ||
| .is_some_and(|def_id| self.mods_with_parse_errors.contains(&def_id)) | ||
| { | ||
| module_had_parse_errors = true; | ||
| } | ||
| self.reborrow().resolve_ident_in_module( | ||
| module, | ||
| ident, | ||
|
|
@@ -1866,7 +1875,19 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { | |
| path.len() - 1, | ||
| )); | ||
| } | ||
| _ => Err(Determinacy::determined(finalize.is_some())), | ||
| _ => { | ||
| for rib in &ribs[ns] { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's just keep the previous version, it's better to have something simpler, than to copy even more logic from |
||
| if let RibKind::Module(module) = rib.kind | ||
| && module.res().and_then(|r| r.module_like_def_id()).is_some_and( | ||
| |def_id| self.mods_with_parse_errors.contains(&def_id), | ||
| ) | ||
| { | ||
| module_had_parse_errors = true; | ||
| break; | ||
| } | ||
| } | ||
| Err(Determinacy::determined(finalize.is_some())) | ||
| } | ||
| } | ||
| } else { | ||
| self.reborrow().resolve_ident_in_scope_set( | ||
|
|
@@ -1903,9 +1924,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { | |
|
|
||
| let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(res); | ||
| if let Some(def_id) = binding.res().module_like_def_id() { | ||
| if self.mods_with_parse_errors.contains(&def_id) { | ||
| module_had_parse_errors = true; | ||
| } | ||
| module = Some(ModuleOrUniformRoot::Module(self.expect_module(def_id))); | ||
| record_segment_res(self.reborrow(), finalize, res, id); | ||
| } else if res == Res::ToolMod && !is_last && opt_ns.is_some() { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| pub mod some_module { | ||
| compile_error!("Error in a module"); //~ ERROR: Error in a module | ||
|
|
||
| fn abc() -> Hello { | ||
| let _: self::SomeType = self::Hello::new(); | ||
| let _: SomeType = Hello::new(); | ||
| } | ||
|
|
||
| mod inner_module { | ||
| use super::Hello; | ||
| use crate::another_module::NotExist; //~ ERROR: unresolved import `crate::another_module::NotExist` | ||
| use crate::some_module::World; | ||
| struct Foo { | ||
| bar: super::Xyz, | ||
| error: self::MissingType, //~ ERROR: cannot find type `MissingType` in module `self` | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub mod another_module { | ||
| use crate::some_module::NotExist; | ||
| fn error_in_this_function() { | ||
| compile_error!("Error in a function"); //~ ERROR: Error in a function | ||
| } | ||
| } | ||
|
|
||
| fn main() { | ||
| // these errors are suppressed because of the compile_error! macro | ||
|
|
||
| let _ = some_module::some_function(); | ||
| let _: some_module::SomeType = some_module::Hello::new(); | ||
|
|
||
| // these errors are not suppressed | ||
|
|
||
| let _ = another_module::some_function(); | ||
| //~^ ERROR: cannot find function `some_function` in module `another_module` | ||
| let _: another_module::SomeType = another_module::Hello::new(); | ||
| //~^ ERROR: cannot find type `SomeType` in module `another_module` | ||
| //~^^ ERROR: failed to resolve: could not find `Hello` in `another_module` | ||
| } |
46 changes: 46 additions & 0 deletions
46
tests/ui/macros/compile_error_macro-suppress-errors.stderr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| error: Error in a module | ||
| --> $DIR/compile_error_macro-suppress-errors.rs:2:5 | ||
| | | ||
| LL | compile_error!("Error in a module"); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error: Error in a function | ||
| --> $DIR/compile_error_macro-suppress-errors.rs:23:9 | ||
| | | ||
| LL | compile_error!("Error in a function"); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error[E0432]: unresolved import `crate::another_module::NotExist` | ||
| --> $DIR/compile_error_macro-suppress-errors.rs:11:13 | ||
| | | ||
| LL | use crate::another_module::NotExist; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `NotExist` in `another_module` | ||
|
|
||
| error[E0433]: failed to resolve: could not find `Hello` in `another_module` | ||
| --> $DIR/compile_error_macro-suppress-errors.rs:37:55 | ||
| | | ||
| LL | let _: another_module::SomeType = another_module::Hello::new(); | ||
| | ^^^^^ could not find `Hello` in `another_module` | ||
|
|
||
| error[E0425]: cannot find type `MissingType` in module `self` | ||
| --> $DIR/compile_error_macro-suppress-errors.rs:15:26 | ||
| | | ||
| LL | error: self::MissingType, | ||
| | ^^^^^^^^^^^ not found in `self` | ||
|
|
||
| error[E0425]: cannot find function `some_function` in module `another_module` | ||
| --> $DIR/compile_error_macro-suppress-errors.rs:35:29 | ||
| | | ||
| LL | let _ = another_module::some_function(); | ||
| | ^^^^^^^^^^^^^ not found in `another_module` | ||
|
|
||
| error[E0425]: cannot find type `SomeType` in module `another_module` | ||
| --> $DIR/compile_error_macro-suppress-errors.rs:37:28 | ||
| | | ||
| LL | let _: another_module::SomeType = another_module::Hello::new(); | ||
| | ^^^^^^^^ not found in `another_module` | ||
|
|
||
| error: aborting due to 7 previous errors | ||
|
|
||
| Some errors have detailed explanations: E0425, E0432, E0433. | ||
| For more information about an error, try `rustc --explain E0425`. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary?
The
nearest_parent_mod()part doesn't look right, and I'm not sure what effect this has in general, because a similar check already exists below.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when looking for
foo::bar::baz()the test bellow only starts when checking forbarinfoo, but without this test here, we would still get the errors iffoois not found and that the current module contains acompile_error!Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why I dislike random diagnostic changes like this, the added logic partially repeats a piece of logic that normally happens in an entirely different function (
resolve_ident_in_lexical_scope/resolve_ident_in_scope_set).Also,
foomay be "not found" in multiple other locations, not just the current module (local variables, unnamed blocks, various preludes).I guess it's ok to suppress the error if a poisoned module was found in at least one of those location during the search.
And
parent_scope.module.nearest_parent_mod()will always be in the list.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your review.
Indeed, the function scope are not added to the poisoned modules. (If I use invalid symbols in
error_in_this_function, the error is not suppressed as it should)I am not very familiar with the rustc codebase, so let me know if there is a way to solve that. (Adding non-module NodeId in the set?)
I was just re-using the existing
module_had_parse_errorsset, but maybe there is a better option?I tried to change a bit the logic to be more robust, but I'm not sure what logic you mean is repeated.