Document the order preludes are resolved #1765
Conversation
src/names/preludes.md
Outdated
| Names of items are resolved in the following order: | ||
|
|
||
| 1. Explicit definitions (including imports) | ||
| 2. Language and macro prelude. If an item is present in both preludes, an error is emitted. |
There was a problem hiding this comment.
This doesn't look correct, language prelude should be considered last.
In the compiler the order can be found in fn visit_scopes (it also describes the motivation behind the scope ordering).
- Type namespace: module items ("explicit definitions") -> extern prelude -> tool prelude -> stdlib prelude -> language prelude (built-in types).
- Value namespace: module items -> stdlib prelude.
- Macro namespace: derive helpers -> legacy derive helpers ->
macro_rules-> module items ->macro_useprelude -> stdlib prelude -> language prelude (built-in attributes).
Language and macro prelude. If an item is present in both preludes, an error is emitted.
Built-in attributes conflict with all other scopes (in attr sub-namespace), not just with preludes.
There was a problem hiding this comment.
(This is not relevant to preludes, but the "macro_rules -> module items" should ideally be intermixed, but it's under-implemented and stubbed by errors, so probably no need to specify it for now.)
There was a problem hiding this comment.
ok; clearly my testing was wrong and i need to think about exactly why. i have fixed the first commit (about sub-namespaces) in the meantime.
There was a problem hiding this comment.
actually i have a question for you: visit_scopes says that in the type namespace, the extern prelude takes precedence over tool modules. this makes sense by the reasoning there since tool modules are closed but the extern prelude is not (i guess in source this correponds to the fact that extern crate can be anywhere but register_tool can only be at the crate root). but for tools it's not ideal because it means that extern crate rustfmt; will break rustfmt::skip.
that doesn't matter so much today because the tool namespace is controlled. but if we want to stabilize register tool, it becomes more of a problem. if the precedence went the other way, with tools resolved before the extern prelude, there would be a workaround: use ::rustfmt to refer to the crate. but there is no such workaround to refer to the tool module.
There was a problem hiding this comment.
oh lol i didn't ask a question. my question is: do you agree that's a problem? and if so, do you have a preferred solution?
my preferred solution is to have tool modules take precedence over the extern prelude, but i'm curious to hear your thoughts.
There was a problem hiding this comment.
i thought of another corner case with register_tool. say you have the following code:
mod rustfmt {}
#[rustfmt::skip] // error: failed to resolve
fn foo() {}there is no way here to disambiguate the tool module. and we cannot change the precedence either, it is very important that explicit definitions have the highest precedence.
i am not sure what to do here. just document the conflict and tell people "don't do that"? at least the names are controlled by author, they can rename any of their explicit definitions or imports.
There was a problem hiding this comment.
oh - we could make this a hard error, and this has the benefit that authors of extern tools never need to do name resolution. either the name is a tool attribute or it's a hard error.
(note that rustfmt doesn't do name resolution today, the above code will get formatted whether or not you comment out the module.)
There was a problem hiding this comment.
it seems like a corner case that will be disallowed eventually, right?
"module items" refers to "pub macro"? i don't plan to document those either, they're still unstable.
Yes, no need to document legacy helper attributes and unstable macros.
There was a problem hiding this comment.
just document the conflict and tell people "don't do that"? at least the names are controlled by author, they can rename any of their explicit definitions or imports.
Yes, that's the status quo.
There was a problem hiding this comment.
oh - we could make this a hard error, and this has the benefit that authors of extern tools never need to do name resolution. either the name is a tool attribute or it's a hard error.
(note that rustfmt doesn't do name resolution today, the above code will get formatted whether or not you comment out the module.)
If a tool works after macro expansion, then we are mostly good already.
The only multi-segment attributes that can survive expansion are tool attributes, if it's something else it will either disappear or produce an error.
Tools working early, like rustfmt, have to accept some false positives in recognizing tool attributes.
I agree that nobody expects tools (or proc macros) to do resolution for their helper attributes, that's why derive helpers have the highest priority (#66529), but with derive helpers we could do that backward compatibly and it didn't interfere with resolution in other positions because macro namespace + attribute sub-namespace make it pretty localized.
In case of tool attributes making them top priority or ambiguous with everything else (like built-in attributes) will probably be too destructive to resolution in other locations.
E.g. in something like type MyType = clippy::Type; the prefix will resolve to a tool module and produce an error.
Unless we do some hacks like in rust-lang/rust#139095 (comment) - "yes, it resolved to a tool module, but they cannot result in anything good in this position, so let's fall back to something else".
I guess we can try starting with the most restrictive rules during stabilization and relax them later if there's breakage or complaints.
…iser rustc_resolve: Test the order that preludes are resolved This test is exhaustive. See attached truth table:  Companion PR to rust-lang/reference#1765.
Rollup merge of rust-lang#138840 - jyn514:precedence-order, r=wesleywiser rustc_resolve: Test the order that preludes are resolved This test is exhaustive. See attached truth table:  Companion PR to rust-lang/reference#1765.
|
FYI this is ready for review. Most of the discussion with petrochenkov is about future extensions; to my knowledge I have incorporated all the feedback about the docs into the PR. |
Also document the derive macro sub-namespace; apparently it was missing before.
cc rust-lang/rust#66079 (comment), rust-lang/rust#138840