-
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? 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: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Eric Holk <[email protected]>
@@ -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