-
Notifications
You must be signed in to change notification settings - Fork 557
Expand name resolution stub #2055
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -398,6 +398,13 @@ r[items.use.ambiguities] | |
| r[items.use.ambiguities.intro] | ||
| Some situations are an error when there is an ambiguity as to which name a `use` declaration refers. This happens when there are two name candidates that do not resolve to the same entity. | ||
|
|
||
| * except where shadowing is allowed | ||
| r[names.resolution.early.imports.errors.ambiguity.globvsglob] | ||
| * it is an error to name an item through ambiguous use declarations | ||
| * two globs imports which both have an item matching that name where the items are different | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| * this is still an error even if there is a third non glob binding resolution to an item with the same name | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. citation needed I expected this to be dependent upon ordering. Much of the ambiguity checking logic saves the first resolution result and checks that against subsequent results, which can end up suppressing errors that would have occured had the 2nd and 3rd results been compared against each other. I'm honestly surprised this isn't one of those cases, gonna check into why, but iirc globvsglob might be one of the few ambiguity errors that is checked outside of edit: yeah this is handled by I feel like this should still be possible to trigger but I can't figure out how and afaict this claim came from my read of the logic in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Resolution results are supposed to never depend on the item ordering (besides There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I guess this is worth putting into the reference as an axiom / expressed intent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
you're of course right, I went back and revisited the example that I was thinking of when I made this comment, it has to do with how macro_rules! m {
() => {}
}
fn foo() {
use bar::m2 as m; // remove `m` decl below and this becomes an ambiguity error
macro_rules! m {
() => {}
}
m!();
}
mod bar {
macro_rules! m2 {
() => {}
}
pub(crate) use m2;
}This isn't necessarily a bug or anything but its where I got the ordering dependent thought. The order in which these are visited does impact whether errors get emitted, but that's the idea, and changing the order involves changing the code semantically, not reordering things lexically. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright so I dug more into the bit of code which I think I have an incomplete understanding of that the initial comment was based off of. Here's the codepath I am trying to trigger and describe
I think my problem is better understanding how try_define_local fits into the rest of import resolution, and when it will get called and when the different members get filled, gonna keep investigating Edit: best I can tell it doesn't matter if that codepath gets executed. I only vaguely remember this but I recall us talking about how these ambiguity errors are attached to resolutions and only get emitted when we actually use an import. The fact that there exists a shadowed glob import is irrelevant because it is shadowed, so the non glob import will always be selected when resolving the associated name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The thing I don't understand is, when will the ambiguity errors from this codepath ever see the light of day? As far as I can tell there are two scenarios where it gets triggered
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went ahead and added a bomb field to that associated ambiguity error to make it panic if it ever observed an error from that codepath and found the culprits |
||
| * it is not an error to have two glob imports which include items which would be ambiguous so long as you do not name one of those items through the ambiguous glob imports | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. documented |
||
|
|
||
| r[items.use.ambiguities.glob] | ||
| Glob imports are allowed to import conflicting names in the same namespace as long as the name is not used. | ||
| For example: | ||
|
|
@@ -442,6 +449,146 @@ fn main() { | |
| } | ||
| ``` | ||
|
|
||
| r[names.resolution.early.imports.errors.ambiguity.builtin-attr] | ||
| * it is an error to use a user defined attribute or derive macro with the same name as a builtin attribute (e.g. inline) | ||
| * I think we may special case this one and allow certain kinds of | ||
| ambiguities where the builtin-attr is shadowed by a user attribute (not | ||
| sure if this actually exists or is just proposed, TODO investigate) | ||
|
|
||
| ```rust | ||
| use derive as inline; // OK | ||
|
|
||
| #[inline] // Not OK, ambiguity at use time | ||
| fn main() {} | ||
| ``` | ||
|
|
||
| <!-- ignore: test doesn't support proc-macro --> | ||
| ```rust,ignore | ||
| // myinline/src/lib.rs | ||
| use proc_macro::TokenStream; | ||
|
|
||
| #[proc_macro_attribute] | ||
| pub fn inline(_attr: TokenStream, item: TokenStream) -> TokenStream { | ||
| item | ||
| } | ||
| ``` | ||
|
|
||
| <!-- ignore: requires external crates --> | ||
| ```rust,ignore | ||
| // src/lib.rs | ||
| use myinline::inline; | ||
| use myinline::inline as myinline; | ||
|
|
||
| #[myinline::inline] | ||
| pub fn foo() {} | ||
|
|
||
| #[crate::inline] | ||
| pub fn bar() {} | ||
|
|
||
| #[myinline] | ||
| pub fn baz() {} | ||
|
|
||
| #[inline] // ERROR `inline` is ambiguous | ||
| pub fn quix() {} | ||
| ``` | ||
|
|
||
| r[names.resolution.early.imports.errors.ambiguity.textualvspathbasedscope] | ||
| * path-based scope bindings for macros may not shadow textual scope bindings to macros | ||
| * This is sort of an intersection between macros and imports, because at | ||
| least in stable rust you can only get path-based macro resolutions from | ||
| imports of mbe macros (and presumably from proc macro crates), but you | ||
| can only get textual scope of macros from macro declarations | ||
| * https://doc.rust-lang.org/nightly/reference/names/namespaces.html#r-names.namespaces.sub-namespaces.use-shadow | ||
| * [macro.decl.scope.path.ambiguity] | ||
| r[names.resolution.early.imports.errors.ambiguity.globvsouter] | ||
| * it is an error to shadow an outer name binding with a glob import | ||
| * This seems to only apply to early resolution (duh, I documented this as part of an early resolution codepath) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the new text here need to be moved to name resolution, IMO. I'd use src/items/use-declarations.md for things that happen at import definition time. Only some situations create inherently ambiguous import definitions, like glob-vs-glob conflicts, and even then they are not reported at definition time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't mind moving this but the reason I put them alongside their items was because that seemed most consistent with how we documented other parts of name resolution, specifically how the names that are introduced by each item are documented next to their items and then theres an aggregated list linking to all those sections in the namespaces chapter. My plan was to put ambiguity and shadowing sections alongside their items and aggregate links to all those sections in name-resolution.md. You can see where I discussed this plan with eric starting here: #t-lang-docs/reference > name resolution @ 💬 |
||
| * // Below we report various ambiguity errors. | ||
| // We do not need to report them if we are either in speculative resolution, | ||
| // or in late resolution when everything is already imported and expanded | ||
| // and no ambiguities exist. | ||
| * I attempted to produce an example using structs and it allowed the outer import to shadow the inner glob just fine | ||
|
|
||
| ```rust | ||
| mod bar { | ||
| pub struct Name; | ||
| } | ||
|
|
||
| mod baz { | ||
| pub struct Name; | ||
| } | ||
|
|
||
| use baz::Name; | ||
|
|
||
| pub fn foo() { | ||
| use bar::*; | ||
| Name; | ||
| } | ||
| ``` | ||
|
|
||
| * I'd like to have a better understanding of why this doesn't trigger ambiguity errors. | ||
| * I'm taking a guess but I think it has to do with how and when we resolve | ||
| names during early resolution. We resolve all the imports but ambiguities | ||
| only occur when observed, so we'd need to try to resolve Name during | ||
| early resolution which simply won't happen because it is a struct so it | ||
| will never be visited for resolution during expansion. | ||
| * We will end up resolving the imports themselves, but they'll resolve fine | ||
| because the imports themselves aren't ambiguous | ||
| * By the time we get to late resolution we no longer expect there to be any | ||
| ambiguities, so we will happily return the first resolution result and | ||
| never search for additional ambiguities, so we resolve directly to | ||
| `bar::Name` through the glob import | ||
|
|
||
| * doing it with macros produced the expected error | ||
| ```rust | ||
| mod bar { | ||
| macro_rules! name { | ||
| () => {} | ||
| } | ||
| pub(crate) use name; | ||
| } | ||
|
|
||
| mod baz { | ||
| macro_rules! name { | ||
| () => {} | ||
| } | ||
| pub(crate) use name; | ||
| } | ||
|
|
||
| use baz::name; | ||
|
|
||
| pub fn foo() { | ||
| use bar::*; | ||
| name!(); // ERROR `name` is ambiguous | ||
| } | ||
| ``` | ||
|
|
||
| * how does it work with imports? The same as macros, same error during early resolution | ||
|
|
||
| ```rust | ||
| mod bar { | ||
| pub mod foo { | ||
| pub struct Name; | ||
| } | ||
| } | ||
|
|
||
| mod baz { | ||
| pub mod foo { | ||
| pub struct Name; | ||
| } | ||
| } | ||
|
|
||
| use baz::foo; | ||
|
|
||
| pub fn foo() { | ||
| use bar::*; | ||
| use foo::Name; // `foo` is ambiguous | ||
| } | ||
| ``` | ||
|
|
||
| r[names.resolution.early.imports.errors.ambiguity.globvsexpanded] | ||
| * Grey Area | ||
|
|
||
| [`extern crate`]: extern-crates.md | ||
| [`macro_rules`]: ../macros-by-example.md | ||
| [`self`]: ../paths.md#self | ||
|
|
@@ -460,3 +607,4 @@ fn main() { | |
| [tool attributes]: ../attributes.md#tool-attributes | ||
| [type alias]: type-aliases.md | ||
| [type namespace]: ../names/namespaces.md | ||
| [macro.decl.scope.path.ambiguity]: ../macros-by-example.md#macro.decl.scope.path.ambiguity | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,111 @@ | ||||||
| r[names.resolution] | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. duplicate of a section in names.md, not sure which to change or how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's just delete the duplicates: #2060 |
||||||
| # Name resolution | ||||||
|
|
||||||
| > [!NOTE] | ||||||
| > This is a placeholder for future expansion. | ||||||
| r[names.resolution.intro] | ||||||
|
|
||||||
| _Name resolution_ is the process of tying paths and other identifiers to the | ||||||
| declarations of those entities. Names are segregated into different | ||||||
| [namespaces], allowing entities in different namespaces to share the same name | ||||||
| without conflict. Each name is valid within a [scope], or a region of source | ||||||
| text where that name may be referenced. Access to certain names may be | ||||||
| restricted based on their [visibility]. | ||||||
|
|
||||||
| * Names are resolved at three different stages of compilation. | ||||||
| * [Macros] and [use declarations] are resolved during macro expansion. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like we're probably going to want to craft new terms of art for the various stages of name resolution. I think early resolution should be named in a way that indicates its required for macro expansion, type dependent name resolution is something I haven't really seen documented anywhere else but follows the same pattern, these are the names that depend on type information to be fully resolved. Everything else is called "late resolution" but I hate that and would prefer to drop the late if possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. currently going with |
||||||
| * This stage of resolution is known as "Early Resolution". | ||||||
| * `Type::assoc_item`, `<Type>::assoc_item`, `<Enum>::Variant` and `EnumTyAlias::Variant` are resolved during type checking | ||||||
| * `Trait::assoc_item`, `<Type as Trait>::assoc_item` and `Enum::Variant` are resolved during late resolution | ||||||
| * This stage of resolution is known as type-relative resolution. | ||||||
| * in reality this is never talked about so I doubt it has a name yet. | ||||||
| * All other names are resolved during AST lowering. | ||||||
| * This stage of resolution is known as "Late Resolution". | ||||||
| * Note, late resolution occurs before type dependent resolution. | ||||||
|
|
||||||
| r[names.resolution.early] | ||||||
| ## Early name resolution | ||||||
|
|
||||||
| r[names.resolution.early.intro] | ||||||
|
|
||||||
| * early name resolution is the part of name resolution that happens during macro expansion | ||||||
| * early name resolution includes the resolution of imports and macros | ||||||
| * early name resolution is the minimum amount of resolution required to resolve macro invocations so they can be expanded. | ||||||
| * resolving imports is necessary to resolve macro invocations | ||||||
| * specifically for path-based scope macro resolutions, this can occur | ||||||
| either because of a `#[macro_export]`, a proc macro definition, or a | ||||||
| reexport of a macro in 2018 or later code. | ||||||
| * resolving macro invocations and tying them to macro declarations is necessary so they can be expanded | ||||||
| * this process is iterative and repeats until there are no remaining unexpanded macro invocations (fixed point algorithm) | ||||||
| * Post expansion these resolutions are checked again to ensure no new ambiguities were introduced by the expansion process | ||||||
| * This causes so called time traveling ambiguities, such as when a glob import introduces an item that is ambiguous with its own base path. | ||||||
|
|
||||||
| TODO Document some time traveling ambiguitie examples, place in relevant ambiguity section | ||||||
|
|
||||||
| r[names.resolution.early.imports] | ||||||
|
|
||||||
| * All imports are fully resolved at this point. | ||||||
| * imports of names that cannot be fully resolved during macro expansion, such as those depending on type information, are not supported and will produce an error. | ||||||
|
|
||||||
| TODO example of unsupported type dependent import | ||||||
|
|
||||||
| r[names.resolution.early.imports.shadowing] | ||||||
|
|
||||||
| The following is a list of situations where shadowing of use declarations is permitted: | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the original text is correct. Use glob shadowing: https://doc.rust-lang.org/nightly/reference/items/use-declarations.html#r-items.use.glob.shadowing In my mind the "not permitted" set is the list of ambiguity errors below |
||||||
|
|
||||||
| * [use glob shadowing] | ||||||
| * [macro textual scope shadowing] | ||||||
|
|
||||||
| r[names.resolution.early.imports.errors] | ||||||
| r[names.resolution.early.imports.errors.ambiguity] | ||||||
|
|
||||||
| * shadowing and ambiguity may or may not represent the same section or one may be a subsection of the other | ||||||
|
|
||||||
| * Builtin Attributes | ||||||
| * Derive Helpers | ||||||
| * Textual Vs Path-based Scope | ||||||
| * Glob vs Outer | ||||||
| * Glob vs Glob | ||||||
| * ~~Glob vs Expanded~~ pretty certain we don't want to mention this one | ||||||
| * More Expanded vs Outer | ||||||
|
|
||||||
| r[names.resolution.early.macros] | ||||||
|
|
||||||
| * .visitation-order | ||||||
| * derive helpers | ||||||
| * not visited when resolving derive macros in the parent scope (starting scope) | ||||||
| * derive helpers compat | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same concern as above about this being deprecated and removed next year. |
||||||
| * always visited | ||||||
| * macro rules bindings (textual scope macros) | ||||||
| * always visited | ||||||
| * modules (path-based scope macros) | ||||||
| * always visited | ||||||
| * macrouseprelude | ||||||
| * not visited in 2018 and later when `#[no_implicit_prelude]` is present | ||||||
| * stdlibprelude | ||||||
| * always visited for macro resolutions | ||||||
| * is it? what about no-std + no-core? | ||||||
| * builtinattrs | ||||||
| * always visited | ||||||
| * .subnamespaces | ||||||
| * macros are split into two subnamespaces, one for bang macros, and the other for attributes and derives. Resolution candidates from the incorrect subnamespace are ignored | ||||||
| * https://doc.rust-lang.org/nightly/reference/names/namespaces.html#r-names.namespaces.sub-namespaces | ||||||
|
|
||||||
| r[names.resolution.early.macros.errors.reserved-names] | ||||||
|
|
||||||
| the names cfg and cfg_attr are reserved in the macro attribute sub-namespace | ||||||
|
|
||||||
| * https://doc.rust-lang.org/nightly/reference/names/namespaces.html#r-names.namespaces.sub-namespaces | ||||||
|
|
||||||
|
|
||||||
| r[names.resolution.late] | ||||||
|
|
||||||
| r[names.resolution.type-dependent] | ||||||
|
|
||||||
| [use glob shadowing]: ../items/use-declarations.md#r-items.use.glob.shadowing | ||||||
| [Macros]: ../macros.md | ||||||
| [use declarations]: ../items/use-declarations.md | ||||||
| [macro textual scope shadowing]: ../macros-by-example.md#r-macro.decl.scope.textual.shadow | ||||||
| [`let` bindings]: ../statements.md#let-statements | ||||||
| [item definitions]: ../items.md | ||||||
| [namespaces]: ../names/namespaces.md | ||||||
| [scope]: ../names/scopes.md | ||||||
| [visibility]: ../visibility-and-privacy.md | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,6 +119,7 @@ For example, the [`cfg` attribute] and the [`cfg` macro] are two different entit | |
|
|
||
| r[names.namespaces.sub-namespaces.use-shadow] | ||
| It is still an error for a [`use` import] to shadow another macro, regardless of their sub-namespaces. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel conflicted about this piece of info being in this section. On the one hand sub-namespaces are quite similar to shadowing, though they work via an independent mechanism. On the other, I'm fairly certain this is exclusively referring to the Assuming this explanation is correct, we can maybe find a place within the use-declarations or mbe chapters to mention how imports are unaffected by subnamespaces. I'm currently wondering if it would make sense to move the subnamespace stuff to live in one of the macros chapters and link to it from here. |
||
| * TODO revisit | ||
|
|
||
| [`cfg` attribute]: ../conditional-compilation.md#the-cfg-attribute | ||
| [`cfg` macro]: ../conditional-compilation.md#the-cfg-macro | ||
|
|
||
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.
documented already