Supress some lookup errors if a module contains compile_error!
#149484
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@rustbot reroll |
|
r? petrochenkov |
| 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()); |
There was a problem hiding this comment.
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.
when looking for foo::bar::baz() the test bellow only starts when checking for bar in foo, but without this test here, we would still get the errors if foo is not found and that the current module contains a compile_error!
There was a problem hiding this comment.
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, foo may 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.
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_errors set, 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.
petrochenkov
added
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
The problem is that when a macro expand to `compile_error!` because its input is malformed, the actual error message from the `compile_error!` might be hidden in a long list of other messages about using items that should have otherwise been generated by the macro. So suppress error about missing items in that module.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
petrochenkov
added
S-waiting-on-review
| } | ||
| _ => Err(Determinacy::determined(finalize.is_some())), | ||
| _ => { | ||
| for rib in &ribs[ns] { |
There was a problem hiding this comment.
Let's just keep the previous version, it's better to have something simpler, than to copy even more logic from resolve_ident_in_(scope_set,lexical_scope) to here.
petrochenkov
added
S-waiting-on-author
6 participants
The problem is that when a macro expand to
compile_error!because its input is malformed, the actual error message from thecompile_error!might be hidden in a long list of other messages about using items that should have otherwise been generated by the macro.So suppress error about missing items in that module.
Fixes #68838