-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
resolve: Avoid mutating vis for distinct glob imports #151956
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
base: main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,8 @@ | |||
| //! Regression test for <https://github.com/rust-lang/rust/issues/151124> | |||
| //@ check-pass | |||
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 building Rustc locally, ICE reproduction was possible for this code without debug-assert
| && old_import != new_import | ||
| { | ||
| return glob_decl; | ||
| } |
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 not a correct change.
Imports are always different here (due to the remove_same_import logic above), so we never update the visibility, but we must indeed update the visibility.
This is a language problem really, this should be an ambiguity error, but we cannot report it without breakage.
And this language problem turns into annoyances in the compiler as well (like #151124).
(#149596 tries to start addressing it in some cases.)
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.
In the meantime it's better to relax the assert logic in compiler\rustc_middle\src\middle\privacy.rs (preferably only for globs).
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.
Later this week I'll try to extend the ambiguous_import_visibilities (from #149596) lint to glob-vs-glob ambiguities, this issue may also get fixed in the process.
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.
Ah, I understand. Thank you for the explanation.
As you said, I modified rustc_middle/src/middle/privacy.rs to avoid asserts.
I'll check back later to see how things are progressing.
5c7b07a to
bce60d9
Compare
bce60d9 to
d26782c
Compare
|
@rustbot ready |
close: #151124
r? @petrochenkov