-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add compiler support for namespaced crates #140271
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Eric Holk <eric.holk@gmail.com>
@@ -118,6 +119,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { | |||
return module.copied(); | |||
} | |||
|
|||
//if def_id.is_crate_root() && !def_id.is_local() { |
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.
Expected this to hold, but it fails when compiling libc. Likely here: https://github.com/rust-lang/libc/blob/e8b01525c30545041af1ad7ba24c05ee3251016b/src/lib.rs#L33-L37
I've reviewed the implementation, perhaps it can be made a bit differently, but and I don't think it can be made significantly less hacky, so I'm against the direction in general - #122349 (comment). |
Thanks for looking at the PR. I disagree with your conclusion though. The changes that this PR introduces are fairly isolated from the existing name resolution functionality. We only ever try to resolve to a namespaced crate when If we do resolve to a namespaced crate it's simply treated as a As far as I can tell there's very little maintainability burden introduced by this solution. If there's a large change in the future in terms of how name resolution is supposed to work, I do agree that this is an additional road block that could pose a problem. I kind of imagine that to be manageable, even though it's impossible to tell really without a concrete change in mind. So I can understand you in your wish that this feature wouldn't touch the compiler at all, the fact is that this RFC has already been accepted. |
This is a bit similar to
Until it's stable it can be unaccepted. While it's unstable I think I'm fine with gradually merging parts that do fit into the current model, and then thinking how to model the remaining parts. |
Attempt to implement the remaining changes for RFC 3243. This is joint work with @eholk.
Opening a draft PR to get feedback from @petrochenkov on the viability of the approach.
r? @petrochenkov