Changes to name resolution by nrc · Pull Request #1560 · rust-lang/rfcs (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation70 Commits4 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Member
• Loading
Some internal and language-level changes to name resolution.
Internally, name resolution will be split into two parts - import resolution and
name lookup. Import resolution is moved forward in time to happen in the same
phase as parsing and macro expansion. Name lookup remains where name resolution
currently takes place (that may change in the future, but is outside the scope
of this RFC). However, name lookup can be done earlier if required (importantly
it can be done during macro expansion to allow using the module system for
macros, also outside the scope of this RFC). Import resolution will use a new
algorithm.
The observable effects of this RFC (i.e., language changes) are some increased
flexibility in the name resolution rules, especially around globs and shadowing.
glaebhoerl, ticki, jseyfried, eddyb, ozkriff, Nemikolh, isomorpheme, Kimundi, birkenfeld, tikue, and 2 more reacted with heart emoji
Some internal and language-level changes to name resolution.
Internally, name resolution will be split into two parts - import resolution and name lookup. Import resolution is moved forward in time to happen in the same phase as parsing and macro expansion. Name lookup remains where name resolution currently takes place (that may change in the future, but is outside the scope of this RFC). However, name lookup can be done earlier if required (importantly it can be done during macro expansion to allow using the module system for macros, also outside the scope of this RFC). Import resolution will use a new algorithm.
The observable effects of this RFC (i.e., language changes) are some increased flexibility in the name resolution rules, especially around globs and shadowing.
I am especially excited about the implications for macro resolution, since these are, as of now, really annoying. Having them fitting better into the module system would be an awesome change.
This RFC, specifically the ability to import the same item multiple times without conflict, solves one of the bigger unresolved pains in Diesel. The ability to have a local name shadow an import feels like a loss to me, though. When you take macros into account, it's possible for defining a struct to silently break something, with a fairly non-local error. I'd much prefer something akin to hiding from Haskell.
mod foo {
pub struct Bar;
}
use foo::* hiding Bar;
struct Bar;
Would this be legal as a result of this RFC?
mod foo { pub mod apple { use super::; pub type Carrot = u32; } pub mod banana { use super::; pub type Carrot = u32; } pub use self::apple::; pub use self:🍌:; } fn main() { let x: foo::Carrot = 5; }
@retep998
No, types are still processed after name resolution, so apple::Carrot and banana::Carrot are considered different items.
Instead you can use primitive types like u32 in imports through one more level of indirection:
mod primitive {
pub type u32_ = u32;
}
pub mod apple {
pub use primitive::u32_ as Carrot;
}
pub mod banana {
pub use primitive::u32_ as Carrot;
}
(Or even nicer, on >=1.9 with two levels of indirection: gist)
| ### Privacy |
| In order to resolve imports (and in the future for macro privacy), we must be |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, this RFC doesn't propose the ability to leak private items through macros, right? I.e.
struct S;
pub macro s {
// Still an error, privacy is checked in the context
// of macro expansion and not macro definition,
// private `S` cannot be leaked.
() => (S)
}
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there are no user-visible changes to privacy or macros in this RFC. I do plan to propose this later though.
@petrochenkov The reason I ask is due to winapi, where sometimes two (or more!) headers will both define the same typedef, and I'd rather not have to pick a header to be the canonical one (nevermind that if I want to be able to cfg out some modules, I'd have really complex cfg conditions to re-enable the non-canonical typedefs). If I define a typedef via type or use in a crate, is there a difference between the two for downstream consumers (assuming it is a primitive type or a non-tuple struct)?
Relevant to the language team, which will review and decide on the RFC.
label
@retep998 There is a difference between type and use - the former are processed later in the compiler and are not part of the module system. At name resolution time two different aliases to the same type are considered different items. Two different use imports of the same item are considered the same for the purposes of 'overlapping' imports.
One could imagine erasing aliases earlier in the compilation process and thus making them transparent to name resolution (and the shadowing rules). I think that should be a separate RFC and I'm not sure if it is a good idea or not.
I suspect that would be a bad idea if extended to generic type aliases.
| the lowering from AST to HIR, so the HIR is a name-resolved data structure. Or, |
|---|
| name lookup could be done lazily (probably with some caching) so no tables |
| binding names to definitions are kept. I prefer the first option, but this is |
| not really in scope for this RFC. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When in doubt, combine your options: lazy lowering (similar to HIR+Ty -> HAIR for MIR building), triggered by type collection (top-level items) or type inference (expressions inside functions) that uses binding tables to determine what nodes to produce.
| where name resolution currently takes place. |
|---|
| To resolve imports, the algorithm proceeds as follows: we start by parsing as |
| much of the program as we can; like today we don't parse macros. When we find |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly does this mean? Don't parse macro definitions? invocations? the result of macro expansion?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't attempt to parse the invocations of a macro in any way (other than storing them as TTs).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that of the 3 makes the most sense but just wanted to check :)
In order to keep macro expansion comprehensible to programmers, we must enforce that all macro uses resolve to the same binding at the end of resolution as they do when they were resolved.
I think this is required for basic soundness, not just peace of mind. Say two macro use sites, a and b, expand to a macro definition used at a third use site, c. Expanding a then c vs b then c leads to a different result, yet at each expansion of c the binding for c's macro was hitherto unambiguous. The only red flag at the end is the overlapping macro bindings once a and b are expanded.
Worse yet, if a and b expand to glob imports providing binding the macro expanded at c, there isn't actually an error at the end of the [a, c, b] or [b, a, c] expansions (in the final states themselves), because overlapping glob imports are not a problem.
| The macro changes are not backwards compatible, which means having a macro |
| system 2.0. If users are reluctant to use that, we will have two macro systems |
| forever. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are you planning to mitigate this risk?
@Ericson2314 Perhaps you are using stable? The fix is on beta but hasn't landed on stable yet. If it doesn't work for you on beta or nightly, could you provide an example?
extern crates are currently treated the way that section proposes we treat use imports, so it doesn't really apply.
@jseyfried edited my comment but my mistake it does indeed work; for the record I am on nightly.
Ah and yes I somehow also missed the current semantics of extern crate are indeed that, so this + your fix unifies everything as is desired. Yay!
I'm finding it hard to reason about the precise model proposed here, I admit. I wonder if there is a way to make the write up a bit more declarative.
In any case, I think the only change here that I found problematic when working on my name resolution prototype was this one:
Note that shadowing is namespace specific. I believe this is consistent with our general approach to name spaces.
I think the problem had to do with the possibility that, in some round, we would bring in a name from a glob that would later have to be shadowed, but we didn't know at the time whether it ought to be shadowed or not. I'll try to see if I can come up with a precise example.
I think the problem had to do with the possibility that, in some round, we would bring in a name from a glob that would later have to be shadowed, but we didn't know at the time whether it ought to be shadowed or not.
This is primarily why we currently distinguish between Indeterminate and Failed imports, which adds quite a bit of complexity (cf. this code and this code). For example,
mod bar { pub fn foo() {} pub use baz::; } mod baz { pub use bar::; }
pub mod foo {}
mod quux {
pub use super::*; // This defines quux::foo in the type namespace ...
pub use bar::foo;
//^ ... but we can't know that until we know that bar::foo fails in the type namespace,
//| which requires us to detect that bar -> baz -> bar -> ... is a glob cycle.
}
If we changed the semantics so that a single import (and perhaps also an ordinary item for consistency) always shadowed glob imports in both namespaces (even if it only defined one of them), then we wouldn't need to distinguish between failing and indeterminate resolutions.
I think this change would cause too much breakage to be practical, but if you'd like I could implement it and we could do a crater run to see if doing a warning cycle would be feasible.
Also, while the complexity is unfortunate, it does generalize well in the presence of unexpanded macros -- we would just have to report that an otherwise failing name in a module with unexpanded macros is indeterminate.
Oh, it just occurred to me that this should mean that a single glob use at the top-level module should suffice for desguaring an automatic prelude import, right?
| ### Multiple imports of the same binding |
|---|
| A name may be imported multiple times and used if both names bind to the same |
| item. E.g., |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to mention a drawback - names (and paths) don't have a definitive "point of introduction" anymore.
I.e. you can't say "the name was defined/imported here", there is always can be >1 of such definition/import points.
Unless all of these points are tracked, this complicates, for example, stability checking for reexports or error reporting if errors refer to these points.
This was referenced
Aug 22, 2016
bors added a commit to rust-lang/rust that referenced this pull request
Implement RFC 1560 behind #![feature(item_like_imports)]
This implements rust-lang/rfcs#1560 (cc #35120) behind the item_like_imports feature gate.
The RFC text describes the changes to name resolution enabled by #
6 tasks
homu added a commit to gfx-rs/gfx that referenced this pull request
Fix pipeline definition macro
use $crate; is not allowed due to recent restrictions and use super::*; won't import the gfx crate. (allowed in future but feature gated rust-lang/rfcs#1560)
Closes #1070
Labels
Proposals relating to modules.
Privacy related proposals & ideas
Proposals relating to name resolution.
Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised.
Relevant to the language team, which will review and decide on the RFC.