-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix: Allow public re-export of extern crate
import
#18413
Conversation
crates/hir-def/src/nameres/tests.rs
Outdated
pub struct Exported; | ||
struct NotExported1; | ||
struct NotExported2; | ||
struct NotExported3; |
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.
Oh, these three items should be public visibility 😅
I'll fix this tomorrow
I reviewed this when I was quite tired and there's no such bug 😅 |
6c84aa8
to
c8d3cd9
Compare
let Some(ImportType::Import(id)) = def_import_type else { | ||
return false; | ||
}; | ||
let source = id.import.child_source(self.db); |
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.
We can't use that here, this draws a dependency edge between the defmap query and the AST causing any AST change to invalidate the def map. We need to derive the relevant info from the item tree somehow
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.
Okay, I'll try different approach
c8d3cd9
to
eb2a50e
Compare
MCVE:
It seems that I should exempt min visibility for
extern crate
, as it seems to be exceptional in rustc 🤔Originally posted by @ShoyuVanilla in #18390 (comment)
Should fix the regression made by #18390
Thank you @lnicola for pointing out this regression 👍