Skip to content
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

Tracking issue for the quote! macro in proc_macro #54722

Open
2 of 3 tasks
alexcrichton opened this issue Oct 1, 2018 · 44 comments
Open
2 of 3 tasks

Tracking issue for the quote! macro in proc_macro #54722

alexcrichton opened this issue Oct 1, 2018 · 44 comments
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

alexcrichton commented Oct 1, 2018

I'm splitting this off #38356 to track the quote! macro specifically contained in proc_macro. This macro has different syntax than the quote 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

From #54722 (comment):

  • Do we want our own version of quote::ToTokens rather than relying on From?
  • Should we add repetition before stabilization?
@alexcrichton alexcrichton added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Oct 1, 2018
@alexcrichton
Copy link
Member Author

Ah, and this is also tracking the proc_macro_quote feature.

@alexcrichton alexcrichton added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Oct 1, 2018
alexcrichton added a commit to alexcrichton/rust that referenced this issue Oct 1, 2018
@eddyb
Copy link
Member

eddyb commented Nov 3, 2018

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

@alercah

This comment has been minimized.

@alercah

This comment has been minimized.

@alexreg
Copy link
Contributor

alexreg commented Jan 9, 2019

I don't like $$ used for escaping $, I'd prefer if \$ was used (we could make $$ an error for now).

Yes please.

@jjpe
Copy link

jjpe commented Jan 9, 2019

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.

@alexreg
Copy link
Contributor

alexreg commented Jan 9, 2019

@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.

@jan-hudec
Copy link

@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.

@alexreg
Copy link
Contributor

alexreg commented Jan 10, 2019

@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.

@pnkfelix
Copy link
Member

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

@eddyb
Copy link
Member

eddyb commented Jan 30, 2019

@pnkfelix It's not. See also dtolnay/request-for-implementation#8 (comment).

@alexreg
Copy link
Contributor

alexreg commented Jan 31, 2019

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?

@dtolnay
Copy link
Member

dtolnay commented Jan 31, 2019

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.)

@alexreg
Copy link
Contributor

alexreg commented Jan 31, 2019

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.

@alercah
Copy link
Contributor

alercah commented Jan 31, 2019 via email

@eddyb
Copy link
Member

eddyb commented Jan 31, 2019

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.

@alexreg
Copy link
Contributor

alexreg commented Jan 31, 2019

@eddyb Yes, that’s what I’d like to see too. @acrichton thoughts on the above?

@alercah
Copy link
Contributor

alercah commented Jan 31, 2019

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.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Mar 26, 2019
@eddyb
Copy link
Member

eddyb commented May 1, 2019

@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.

@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 31, 2020
@PoignardAzur
Copy link
Contributor

PoignardAzur commented Mar 8, 2022

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?

@joshtriplett joshtriplett added the S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. label Jun 1, 2022
@Mark-Simulacrum
Copy link
Member

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:

  • def_site/macros 2.0 hygiene
  • Do we actually need quote in the in-tree proc_macro?

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).

@GoldsteinE
Copy link
Contributor

GoldsteinE commented Jan 5, 2023

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.

@tgross35
Copy link
Contributor

tgross35 commented Apr 14, 2023

Can proc_macro::quote always quote proc_macro2::TokenStreams? 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

@gorentbarak
Copy link

I think proc_macro::quote seems like it has been stable for a while. Rust team, the time has come to stabilize this.

@gorentbarak

This comment was marked as spam.

@bjorn3
Copy link
Member

bjorn3 commented Nov 30, 2023

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.

@traviscross
Copy link
Contributor

@rustbot labels +I-lang-nominated

This issue came up in the context of RFC 3607, so let's nominate to check in on this and whether there's anything we can help to unblock here.

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Jul 12, 2024
@joshtriplett
Copy link
Member

@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)?

@joshtriplett
Copy link
Member

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!?

@bjorn3
Copy link
Member

bjorn3 commented Aug 14, 2024

If #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 #125721 (comment) I suggested a potential way of fixing just the diagnostic problems.

@tgross35
Copy link
Contributor

tgross35 commented Aug 15, 2024

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. 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.) nevermind, 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.

@Veykril
Copy link
Member

Veykril commented Aug 15, 2024

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.

@petrochenkov
Copy link
Contributor

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).

@bjorn3
Copy link
Member

bjorn3 commented Aug 15, 2024

Do you want me to change my PR to use Span::call_site?

@nikomatsakis
Copy link
Contributor

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.

@traviscross
Copy link
Contributor

@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.

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Aug 21, 2024
@tgross35
Copy link
Contributor

Making proc_macro a drop-in replacement for proc_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?

@GoldsteinE
Copy link
Contributor

I’d argue that # is better, since it lets you generate declmacros without escaping. I’m not sure what are the advantages of $ here.

@traviscross
Copy link
Contributor

traviscross commented Aug 21, 2024

Apropos of proposed RFC 3607, we're considering an .#tag syntax for accessing the discriminant of an enum. See here.

More generally, we find ourselves wanting to use the # syntax space (e.g.), and so an interpolation syntax that doesn't conflict with that has some appeal.

@workingjubilee
Copy link
Member

workingjubilee commented Aug 30, 2024

So concretely, the hardest blockers seem to be:

  1. Stabilize Span::def_site() OR make quote! use Span::{call,mixed}_site() OR otherwise address the span issue so that we don't compromise diagnostics
  2. (ahem) hash out any details about the unquoting syntax
  3. bjorn3's remark about hygiene which might be 1 again (I haven't expanded the tokens fully yet)

At 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...

  1. deciding on including repetition or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests