Open alexcrichton opened 5 years ago
Ah, and this is also tracking the proc_macro_quote
feature.
We need to avoid .clone()
-ing the variables interpolated by quote!
, we should have &T -> TokenStream
(or, better, an append taking &T, &mut TokenStream
, like the quote
crate).
EDIT: thinking more about it, I think we should support TokenStream
, all the proc_macro
types that convert to TokenStream
, including references of all of those, and, on top of that, anything that .extend(...)
on a TokenStream
would take (I haven't checked, do we implement Extend
for it?).
I don't like $$
used for escaping $
, I'd prefer if \$
was used (we could make $$
an error for now).
We should make sure we error on $(...)
even if we don't start supporting repetition syntax right away, to reserve the right to do so in the future.
And maybe we should consider ${...}
for nesting expressions, but, again, as long as any unsupported syntax that starts with $
errors, we can extend it later.
There's also the question of what to do about spans. The quote
crate has a version of the macro that also takes Span
, but I'd prefer pointing to the proc macro source where quote!
was used.
For this, we can use an unstable Span
constructor that takes a SourceFile
reference and a byte range into that, and an unstable constructor for SourceFile
that provides all the per-file SourceMap
information (file path, expected content hash, line map, etc.) the compiler needs.
Making def-site hygiene also work would be trickier, but not impossible AFAIK.
cc @petrochenkov
I just tried this out and $a
quoting doesn't seem to work at all. It doesn't even error on $#
like I'd expect from the source.
Oh nevermind, I was using it wrong.
I don't like
$$
used for escaping$
, I'd prefer if\$
was used (we could make$$
an error for now).
Yes please.
I'm wondering what the reasoning is behind the preference for \$
over $$
.
Is it purely a syntactic style preference? Or is there more to it?
I ask because as I see it (but without announcing a preference either way), $$
has the advantage of being easier to type for pretty much everyone, while it has the disadvantage of being less easy to read for e.g. people with dyslexia.
@jjpe As I see, there are two main reasons: 1) easier to read (can be picked out at a glance with less difficulty), 2) agrees with long-established convention of backslash escapes (beginning with C I think, but existing in most current languages for strings, including Rust).
Furthermore, it will probably be more common to escape $
than it will be to escape \
in macros.
@alexreg, main advantage of $$
is that it does not need another special character. In this case there is only one, so adding another doubles the complexity. If there were more special characters, a common escape would make sense, but in this case there is only one and most of the languages that have only one special character escape it by doubling it.
@jan-hudec That's not a real advantage to me. There's no additional complexity, when you already have all of those chars in the syntax. And I think the two reasons I gave above are more pertinent.
is this quote!
as fully-featured as the one in crates.io quote ? I cannot figure out how to do e.g. repetition with it
@pnkfelix It's not. See also https://github.com/dtolnay/request-for-implementation/issues/8#issuecomment-458760269.
I'm kind of confused here: why would we want to pursue adding an inferior solution to the stdlib if a superior one exists as a crate?
One other difference besides repetitions and $
vs #
is that the external quote is unit testable -- the proc_macro quote produces a proc_macro::TokenStream which can only exist within the context of a procedural macro, so any functions or helper libraries that you write using proc_macro quote will panic when called from a unit test.
(I don't have an opinion about whether proc_macro needs to provide a quote macro, just pointing out one other consideration that has not been brought up yet. It would be nice to fix this before stabilizing something.)
I see. I struggle to understand why the proc_macro
crate is only accessible from within proc macro crates though -- it gives rise to what I see as an unfortunate need for the proc_macro2 crate, and creates a problem with testing, not to mention the issue @dtolnay just pointed out above.
It's accessible in other crates as a library, see also #57288. The attributes to define proc macros are not.
On Wed., Jan. 30, 2019, 23:35 Alexander Regueiro <notifications@github.com wrote:
I see. I struggle to understand why the proc_macro crate is only accessible from within proc macro crates though -- it gives rise to what I see as an unfortunate need for the proc_macro2 crate, and creates a problem with testing, not to mention the issue @dtolnay https://github.com/dtolnay just pointed out above.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/54722#issuecomment-459211850, or mute the thread https://github.com/notifications/unsubscribe-auth/AT4HVWZ8uxcYVj2UiLTmrMkWt01HuVWKks5vInKKgaJpZM4XCjad .
I believe @dtolnay is referring to the fact that the API itself will panic if used outside of the execution of a proc macro by a compiler, because it doesn't contain an implementation of the API, only the logic to talk to a "server" of such an implementation.
Some have suggested having a default "server" (loopback, if you will), which behaves just like proc-macro2's polyfills and allows using proc_macro
at all times, and I'm not really opposed to that.
@eddyb Yes, that’s what I’d like to see too. @acrichton thoughts on the above?
Ah, I see. It surprises me somewhat that all of the implementation is provided by ABI rather than a more detailed in-Rust implementation with less reliance on the ABI.
@alercah (Sorry for delayed response, I'm clearing out my notifications)
If we were to provide an implementation of the proc_macro
API in proc_macro
itself, we would have to move a significant chunk of the compiler frontend in there, and expose a lot of the fiddly details.
And it would be fine-tuned for rustc in particular, while at the same time forcing the proc macro token model onto rustc (might not be a bad thing, but we're not there yet).
With the RPC(-like) approach, a frontend can supply its own implementation, for any particular invocation. Some projects are already using this capability to run proc macros outside of rustc.
Aside from the issue regarding interpolation noted above, what's holding up stabilization here?
quote!
currently uses macro 2.0 hygiene, so either macro 2.0 hygiene needs to be stabilized first, or quote!
needs to be migrated to weaker hygiene (e.g. Span::mixed_site
).
Regarding the hygiene, there has just been an example of a user being confused by this never-seen-elsewhere Span::def_site()
: https://stackoverflow.com/a/66302653/10776437
I don't know if weakening the hygiene is necessary, but at the very least a way to choose another transparency (e.g., opt into ::quote::quote_spanned!(Span::mixed_site() =>
-like behavior) would be desirable.
How does this interact with macro_rules!
interpolation? Specifically, both use $
, so which is given precedence? I haven't tried this, but it's something that should be clearly documented either way.
Is there any progress on this issue? It seems like a good candidate given recent efforts to reduce compile times due to proc macros and extremely complicated inline macros (of which quote::quote!
is definitely a major example).
Have we considered making a min_proc_macro_quote
issue that could get implemented without having to solve hygiene?
Lang briefly discussed this in backlog bonanza today and we'd like to see a summary of the outstanding blockers or expectations here.
I think the thread so far has pointed out:
But it'd be useful to get a summary from folks more familiar on the blockers for stabilization (or removal, if we feel that's better).
I’ve stumbled upon this interesting case:
fn main() {
let expr: syn::Expr = syn::parse_str("2 + 2").unwrap();
let quoted = quote::quote! {
let foo = &#expr;
};
println!("{quoted}"); // & 2 + 2
}
I think that’s not ideal (e.g. it differs from macro_rules!
behaviour) and that if quote!()
is to be added to the standard library, this issue warrants some consideration.
UPD:
Since proc_macro
doesn’t have anything like Expr
, it’s probably a documentation issue. Downstream implementors of types that can be used inside of quote!()
choose their token representation.
Can proc_macro::quote
always quote proc_macro2::TokenStream
s? From a quick test it looks like the answer is yes, but I'm not sure whether this is guaranteed or if there are limitations.
Looks like it only returns proc_macro::TokenStream
, which isn't unexpected. It would be really nice, and probably more efficient, if there was a way for it to do TokenStream2
-> TokenStream2
quoting. But this of course probably isn't possible.
Edit: or, I guess that the nicer long term solution would be to make proc_macro
usable outside of proc macros, which woulld eliminate the need for proc_macro2
I think proc_macro::quote seems like it has been stable for a while. Rust team, the time has come to stabilize this.
This will be useful for everyone. Do it, for me and for every Rustacean out there.
I commented this on zulip before, but for visibility:
The current impl of the quote! macro in rustc forwards hygiene info from compile time of the proc macro into runtime. This means that this hygiene info has to be embedded into the proc macro and I believe loading a proc macro requires all the compile time dependencies of the proc macro too in case hygiene info originates from them. IMO we should rip out this functionality and just use the same hygiene as would be used when manually creating the token stream.
@bjorn3
IMO we should rip out this functionality and just use the same hygiene as would be used when manually creating the token stream.
Verifying: would that change make proc_macro::quote behave like the one from the crates.io ecosystem (proc_macro2
/quote
)?
We discussed this in today's @rust-lang/lang meeting. Given the widespread use of proc_macro
/quote
in the ecosystem, and the libs-api ACP to make proc_macro
work outside of a proc macro (which libs-api currently expects to accept once updated), we're enthusiastic about seeing proc_macro::quote!
brought to a point where we can stabilize it.
What would it take to get quote!
into a shape to stabilize, with roughly the functionality currently present in the crates.io version of quote!
?
If https://github.com/rust-lang/rust/pull/125721 lands proc_macro::quote!{}
should have almost the same expressive power as quote::quote!{}
. (The only difference being that the current state of that PR uses unstable def-site spans by default rather than call-site spans like the quote crate does.) It currently regresses diagnostics in a way that matches how the quote crate already behaves, but in https://github.com/rust-lang/rust/pull/125721#issuecomment-2147941185 I suggested a potential way of fixing just the diagnostic problems.
Assorted things we may want to figure out before stabilization, mostly related to parity with quote
:
It seems like only Into<proc_macro::TokenStream>
things can be interpolated. Is this enough? This seems unfortunate because you can't quote tokens directly:
// `pm` = `proc_macro`, `pm2` = `proc_macro2`
// proc_macro2 / quote::quote works fine
let x2 = pm2::Ident::new("foo", pm2::Span::call_site());
let _ = quote::quote! {
let #x2 = 199;
};
// proc_macro::quote errors
let x = pm::Ident::new("foo", pm::Span::call_site());
let _ = pm::quote! {
// ERROR: `From<proc_macro::Ident>` is not implemented for `proc_macro::TokenStream`
let $x = 199;
};
Maybe we could consider implementing Into<pm::TokenStream>
for the individual token types like Ident
, Literal
, etc? Or the quote::quote!
approach of using a specialized trait, which also allows quoting primitives https://docs.rs/quote/latest/quote/trait.ToTokens.html. nevermind, ToTokens
is also more efficient since it uses &mut
to append to a single TokenStream
. (Currently we create a TokenStream
for each token and collect them all into a new TokenStream
. We might be able to improve our expansion.)quote::ToTokens
winds up cloning to append too.
Whatever the decision, this needs documentation.
Special case of the above, we can't quote any references, e.g. a &TokenTree
needs to be cloned first. quote::ToTokens
allows this, which is a nice convenience.
We should add repetition, I don't think there is any point in stabilizing without since it's a pretty basic feature for parity. For the record, here is the current error:
let v = vec![];
let _ = pm::quote! { $($v)* };
error: proc macro panicked
help: message: `$` must be followed by an ident or `$` in `quote!`
Tests are kind of scattered and mostly relate to hygiene, we may want to just pull in `quote's test suite. Which will also be good to check other features that diverge.
Related to all of the above, documentation is pretty empty and doesn't have any examples. quote::quote!
has a nicer reference.
To comment with my rust-analyzer hat on, I'd like it if we could support tracking the def site spans to the concrete tokens somehow (for rust-analyzer at least) in the future. As being able to use go to def and landing within the proc-macro source (where relevant) sounds really neat. So it would be nice if we don't close the doors to that possibility when stabilizing the macro.
quote!
should not be stabilized unless either Span::def_site
is stabilized (which is hard), or quote!
is migrated to Span::mixed_site
/Span::call_site
(which is easier).
Do you want me to change my PR to use Span::call_site
?
Rough thought from the lang team meeting:
Making proc_macro
a drop-in replacement for proc_macro2
seems like a real win.
I realize that will take away the purported benefit of quote!
here (i.e., using $
instead of #
).
But maybe that's a sign that we should be pursuing a different syntax, not sure.
@rustbot labels -I-lang-nominated
We talked about this today. There seems to be enthusiasm for making proc_macro
more generally useful. It seems there's some known further work needed here. Please renominate for lang with any specific questions or with any particular things we might be able to unblock. If there are many things to work through that could be assembled into a design document, perhaps a design meeting would be in order.
Making
proc_macro
a drop-in replacement forproc_macro2
seems like a real win.
Did you mean proc_macro::quote
as a drop-in for quote::quote
, i.e. with #
and ToTokens
? Or for proc_macro
to be usable outside of proc macro crates like proc_macro2
is? I think the latter is definitely a goal but it is a difficult one, I don't necessarily think that needs to block moving forward with proc_macro::quote
.
I realize that will take away the purported benefit of
quote!
here (i.e., using$
instead of#
).But maybe that's a sign that we should be pursuing a different syntax, not sure.
I wondered about this, is $
just considered a benefit for consistency?
I’d argue that #
is better, since it lets you generate declmacros without escaping. I’m not sure what are the advantages of $
here.
So concretely, the hardest blockers seem to be:
Span::def_site()
OR make quote!
use Span::{call,mixed}_site()
OR otherwise address the span issue so that we don't compromise diagnosticsAt least personally, I consider "feature-parity with the crate" to be a huge "nice-to-have" and something that's easy to use as a block to ever shipping this. We should focus mainly on those that we expect to be essentially impossible to simply tack on later, which will probably include...
I'm splitting this off https://github.com/rust-lang/rust/issues/38356 to track the
quote!
macro specifically contained inproc_macro
. This macro has different syntax than thequote
crate on crates.io but I believe similar functionality. At this time it's not clear (to me at least) what the status of this macro is in terms of how much we actually want to stabilize it or what would block its stabilization. Others are welcome to fill this in though!Steps / History
Unresolved questions
$
to match other macros or#
to drop in withquote::quote
? https://github.com/rust-lang/rust/issues/54722#issuecomment-2302411103From https://github.com/rust-lang/rust/issues/54722#issuecomment-2290103770:
quote::ToTokens
rather than relying onFrom
?