Resolve $crate at the expansion-local crate
Fixes #99035. The glued $crate token now gets the resolution context of the local crate when gluing $crate into a single token. This is almost certainly what is intended by a macro author.
This does change the behavior of stable code. Specifically, previously $crate would resolve to the crate of the crate token, and if a macro uses a tt binder that expands to $, it is possible for it to replicate the behavior made much more accessible by $$crate.
However, this behavior really isn't all that desirable. If a crate token is passed in, it already resolves to its local crate, so turning it into a $crate token is a resolution no-op. If a macro_rules! definition wants a def-site crate, it can just write $crate. The current behavior of $crate becoming a def-site version of the crate token is not useful; $crate resolving to the gluing crate is.
Except... it looks like this is allowing proc-macro-hacked proc-macros to use $crate to refer to the declaration crate...
r? @estebank
(rust-highfive has picked a reviewer for you, use r? to override)
Note: #99035 is marked as a regression because $$crate "regressed" from not working to problematic semantics. The revert is the proper fix to the "regression;" this fixes the underlying issue in a way that should allow the feature to re-stabilize.
That's odd that I didn't run into this error locally. I'm worried that this means something is relying on $crate resolving to crate's def-site rather than the macro def-site...
The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Compiling rustc_macros v0.1.0 (/checkout/compiler/rustc_macros)
error[E0433]: failed to resolve: could not find `LanguageIdentifier` in the crate root
--> compiler/rustc_macros/src/diagnostics/fluent.rs:91:45
|
91 | let mut bundle = FluentBundle::new(vec![langid!("en-US")]);
| ^^^^^^^^^^^^^^^^ not found in `$crate`
= note: this error originates in the macro `proc_macro_call` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this struct
|
1 | use unic_langid::LanguageIdentifier;
1 | use unic_langid::LanguageIdentifier;
|
help: if you import `LanguageIdentifier`, refer to it directly
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
error[E0433]: failed to resolve: unresolved import
--> compiler/rustc_macros/src/diagnostics/fluent.rs:91:45
|
|
91 | let mut bundle = FluentBundle::new(vec![langid!("en-US")]);
| ^^^^^^^^^^^^^^^^ not found in `$crate::subtags`
= note: this error originates in the macro `proc_macro_call` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this struct
|
|
1 | use unic_langid::subtags::Language;
|
help: if you import `Language`, refer to it directly
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
error[E0433]: failed to resolve: unresolved import
--> compiler/rustc_macros/src/diagnostics/fluent.rs:91:45
|
|
91 | let mut bundle = FluentBundle::new(vec![langid!("en-US")]);
| ^^^^^^^^^^^^^^^^ not found in `$crate::subtags`
= note: this error originates in the macro `proc_macro_call` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this struct
|
1 | use unic_langid::subtags::Region;
1 | use unic_langid::subtags::Region;
|
help: if you import `Region`, refer to it directly
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
Compiling tracing-tree v0.2.0
Compiling chalk-solve v0.80.0
Compiling rustc_log v0.0.0 (/checkout/compiler/rustc_log)
Okay, I was able to reproduce locally, so I'll at least be able to take a look into this.
I was able to confirm that something interesting is going on here: #[proc_macro_hack] allows the proc macro's expansion to use $crate, and unic-langid's langid! is using that.
I'm not exactly sure how this ever worked in the first place tbqh. Some part of the proc-macro-hack is giving the emitted tokens a span in the declaration crate. With this change, as the proc_macro_call!() is generated in the caller's crate, the $crate is glued there and refers to the caller's crate, not the declaration crate.
macro tracing
#![feature(trace_macros)]
trace_macros!(true);
fn main() {
let _ = unic_langid::langid!("en-us");
}
With current nightly:
❯ cargo +nightly expand
Checking playground v0.0.0 (D:\git\cad97\playground)
warning: some trace filter directives would enable traces that are disabled statically
| `rustc_expand::mbe::quoted=debug` would enable the DEBUG level for the `rustc_expand::mbe::quoted` target
= note: the static max level is `info`
= help: to enable DEBUG logging, remove the `max_level_info` feature
note: trace_macro
--> src\main.rs:6:13
|
6 | let _ = unic_langid::langid!("en-us");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: expanding `langid! { "en-us" }`
= note: to `{
#[derive($crate :: _proc_macro_hack_langid)] #[allow(dead_code)] enum
ProcMacroHack { Value = (stringify! { "en-us" }, 0).1, } proc_macro_call!
()
}`
= note: expanding `proc_macro_call! { }`
= note: to `unsafe
{
$crate :: LanguageIdentifier ::
from_raw_parts_unchecked(unsafe
{ $crate :: subtags :: Language :: from_raw_unchecked(28261u64) }, None,
Some(unsafe
{ $crate :: subtags :: Region :: from_raw_unchecked(21333u32) }), None)
}`
Finished dev [unoptimized + debuginfo] target(s) in 0.14s
#![feature(prelude_import)]
#![feature(trace_macros)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
fn main() {
let _ = {
#[allow(dead_code)]
enum ProcMacroHack {
Value = ("\"en-us\"", 0).1,
}
unsafe {
::unic_langid_macros::LanguageIdentifier::from_raw_parts_unchecked(
unsafe { ::unic_langid_macros::subtags::Language::from_raw_unchecked(28261u64) },
None,
Some(unsafe {
::unic_langid_macros::subtags::Region::from_raw_unchecked(21333u32)
}),
None,
)
}
};
}
With this patch:
❯ cargo +stage1 expand
Checking playground v0.0.0 (D:\git\cad97\playground)
DEBUG rustc_expand::mbe::quoted $crate changed its target crate from DefId(19:0) to DefId(0:0)
note: trace_macro
--> src\main.rs:6:13
|
6 | let _ = unic_langid::langid!("en-us");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: expanding `langid! { "en-us" }`
= note: to `{
#[derive($crate :: _proc_macro_hack_langid)] #[allow(dead_code)] enum
ProcMacroHack { Value = (stringify! { "en-us" }, 0).1, } proc_macro_call!
()
}`
= note: expanding `proc_macro_call! { }`
= note: to `unsafe
{
$crate :: LanguageIdentifier ::
from_raw_parts_unchecked(unsafe
{ $crate :: subtags :: Language :: from_raw_unchecked(28261u64) }, None,
Some(unsafe
{ $crate :: subtags :: Region :: from_raw_unchecked(21333u32) }), None)
}`
error[E0433]: failed to resolve: could not find `LanguageIdentifier` in the crate root
--> src\main.rs:6:13
|
6 | let _ = unic_langid::langid!("en-us");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `$crate`
|
= note: this error originates in the macro `proc_macro_call` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this struct
|
3 | use unic_langid::LanguageIdentifier;
|
help: if you import `LanguageIdentifier`, refer to it directly
--> D:\.rust\cargo\registry\src\github.com-1ecc6299db9ec823\unic-langid-macros-0.9.0\src\lib.rs:4:1
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
|
error[E0433]: failed to resolve: unresolved import
--> src\main.rs:6:13
|
6 | let _ = unic_langid::langid!("en-us");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `$crate::subtags`
|
= note: this error originates in the macro `proc_macro_call` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this struct
|
3 | use unic_langid::subtags::Language;
|
help: if you import `Language`, refer to it directly
--> D:\.rust\cargo\registry\src\github.com-1ecc6299db9ec823\unic-langid-macros-0.9.0\src\lib.rs:4:1
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
|
error[E0433]: failed to resolve: unresolved import
--> src\main.rs:6:13
|
6 | let _ = unic_langid::langid!("en-us");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `$crate::subtags`
|
= note: this error originates in the macro `proc_macro_call` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this struct
|
3 | use unic_langid::subtags::Region;
|
help: if you import `Region`, refer to it directly
--> D:\.rust\cargo\registry\src\github.com-1ecc6299db9ec823\unic-langid-macros-0.9.0\src\lib.rs:4:1
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
|
For more information about this error, try `rustc --explain E0433`.
#![feature(prelude_import)]
#![feature(trace_macros)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
fn main() {
let _ = {
#[allow(dead_code)]
enum ProcMacroHack {
Value = ("\"en-us\"", 0).1,
}
unsafe {
crate::LanguageIdentifier::from_raw_parts_unchecked(
unsafe { crate::subtags::Language::from_raw_unchecked(28261u64) },
None,
Some(unsafe { crate::subtags::Region::from_raw_unchecked(21333u32) }),
None,
)
}
};
}
The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Compiling rustc_macros v0.1.0 (/checkout/compiler/rustc_macros)
error[E0433]: failed to resolve: could not find `LanguageIdentifier` in the crate root
--> compiler/rustc_macros/src/diagnostics/fluent.rs:91:45
|
91 | let mut bundle = FluentBundle::new(vec![langid!("en-US")]);
| ^^^^^^^^^^^^^^^^ not found in `$crate`
|
= note: this error originates in the macro `proc_macro_call` which comes from the expansion of the macro `langid` (in Nightly builds, run with -Z macro-backtrace for more info)
|
1 | use unic_langid::LanguageIdentifier;
|
|
help: if you import `LanguageIdentifier`, refer to it directly
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
error[E0433]: failed to resolve: unresolved import
--> compiler/rustc_macros/src/diagnostics/fluent.rs:91:45
|
|
91 | let mut bundle = FluentBundle::new(vec![langid!("en-US")]);
| ^^^^^^^^^^^^^^^^ not found in `$crate::subtags`
|
= note: this error originates in the macro `proc_macro_call` which comes from the expansion of the macro `langid` (in Nightly builds, run with -Z macro-backtrace for more info)
|
|
1 | use unic_langid::subtags::Language;
|
help: if you import `Language`, refer to it directly
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
error[E0433]: failed to resolve: unresolved import
--> compiler/rustc_macros/src/diagnostics/fluent.rs:91:45
|
|
91 | let mut bundle = FluentBundle::new(vec![langid!("en-US")]);
| ^^^^^^^^^^^^^^^^ not found in `$crate::subtags`
|
= note: this error originates in the macro `proc_macro_call` which comes from the expansion of the macro `langid` (in Nightly builds, run with -Z macro-backtrace for more info)
|
1 | use unic_langid::subtags::Region;
|
|
help: if you import `Region`, refer to it directly
4 - #[proc_macro_hack]
4 + #[proc_macro_hack]
Compiling tracing-tree v0.2.0
Compiling chalk-solve v0.80.0
Compiling rustc_log v0.0.0 (/checkout/compiler/rustc_log)
r? @petrochenkov
This is a tricky area and I need to look at this, but I won't be able to do it very soon, maybe in a week or two.
I just want to say that while (I think) this approach works as I desired, this isn't the "ideal" way to handle $crate. Namely, because since $crate is glued eagerly, it's turned into the $crate token in macro patterns as well.
The "perfect world semantics" of $crate would be as a reserved binder name. The token in the binder would just be a def-site crate.
@CAD97 do you think we should hold off on looking at this until after the work on #99447 is finished?
I need to refresh hygiene details in my memory before reviewing this PR and https://github.com/rust-lang/rust/pull/100387, it will require at least one full working day. I've been very busy the last couple of months so now I'm very tired and on vacation, I'll find the day some time after the return.
:umbrella: The latest upstream changes (presumably #102644) made this pull request unmergeable. Please resolve the merge conflicts.
FWIW, I really dislike what this PR does.
https://github.com/rust-lang/rust/issues/99035 implies that $$ has anything to do with changing hygiene, but it does not!
Imagine that you have the same situation with macros 2.0 and proper def-site crate:
macro outer {
macro inner {
crate::Struct // outer_crate::Struct
}
}
What would you do to switch crate from outer's crate root to inner's crate root? There aren't any $s here.
The answer is you'd use a hygiene opt-out like https://github.com/rust-lang/rfcs/pull/2498 or similar:
macro outer {
macro inner {
#crate::Struct // inner_crate::Struct
}
}
, and same with $crate
macro outer {
macro inner {
$#crate::Struct // inner_crate::Struct
}
}
Switching to call-site hygiene is a separate (not yet implemented) feature, it doesn't seem right to me to bolt it on $$.
(I'm interested in someone pursuing https://github.com/rust-lang/rust/pull/99447#issuecomment-1204120473 tho.)
https://github.com/rust-lang/rust/issues/99035 implies that $$ has anything to do with changing hygiene, but it does not!
So, I both agree and disagree with this interpretation. I agree that this PR's implementation is suboptimal, but I disagree that $crate should use the def site of the crate token.
My model is that $crate is an expansion metavariable the same as any other $binder. The difference is that $crate is a predefined binder, and the expansion of that binder is a def-site crate token for the macro which contains the $crate.
There is no "$$ impacts hygiene" involved here. The hygiene result is from "expanding" $crate. The difference is in pseudocode
impl MacroExpansionContext {
fn expand(binder: Ident) -> TokenStream {
if (binder == kw::crate) {
#[cfg(current-behavior)] {
Ident::new(kw::crate, Span::def_site_at(binder))
}
#[cfg(expander-model)] {
Ident::new(kw::crate, Span::def_site_at(self))
}
} else {
self.resolve_binding(binder)
}
}
}
@CAD97 ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!
This is pending deciding what the correct behavior is, as far as I'm aware. I opened a Zulip thread to discuss.
Zulip thread opened for T-lang (I'll add the relevant label to remind me of that)
@rustbot label +t-lang
Copypasting the status from Zulip thread:
I want to look at that PR again (and marked it with waiting on review), but it is time consuming and low priority, so no specific timeline. I think my general stance is still to not touch it, and keep it as is, and instead pursue the hygienic
crate, which is not a weird accidental amalgamation of two tokens.
I was writing a (probably overly long) post about my opinion/status here when I reached a conclusion I hadn't before: macro_rules! metavars do hygienic (def_site) name lookup. I guess I just never properly tested it, and [$]crate feels more like an item/path (which aren't hygienic) than a variable name lookup.
Thus, $crate (in expansion position) can be logically explained as a def_site lookup of a pre-defined, ambiently-available metavar. (The gluing in pattern position still makes no sense.) This makes me no longer actively dislike the current $crate semantics. In fact, despite still holding that it's not particularly intuitive nor useful for $$crate and $crate to double-expand identically, I actually agree that it's the correct behavior given that keywords aren't reserved names for macro binders.
As such, I'm going to close this PR since I now agree that this isn't the correct approach, at least not on its own. The ability to name the crate of a macro expanded macro definition is still desirable, but that can be filed into the same expressive gap as naming the defining module.
If we can at least make attempts to use $$crate warn, I'll be happy to remove my concern about and retry stabilization of $$. I'd prefer to make all $keyword metavars into span independent metavar information based on the containing macro definition site, but getting $$ is better than nothing (so long as current-semantics $$crate results in a warning).