goncalerta / proc-quote Goto Github PK

This issue tracks the follow up of dtolnay/quote#7.

My concern about this feature is that it would make interpolation of types that implement both ToTokens and Iterator<Item=ToTokens> ambiguous. (example: TokenStream, a type that will probably come up alot in interpolations).

In fact, I think the most common use for TokenStreams would be to be used as a ToTokens rather than an iterator.

As a rough made up example:

let path = quote!{ ::some::path::to::an::object };
let construct_fn = Ident::new("new", Span::call_site());
let i = 0..5;
let output = quote!{
    #(object[#i] = #path::#construct_fn();)*
}

As TokenStream implements Iterator<Item=TokenTree>, this would (understandably) give a result that wasn't expected from the user. I fear this could become confusing in much more complex macros.

By not allowing non-repeating variables altogether, users would be forced to explicitly turn every item they want to use as ToTokens before passing them to the quote

let path = quote!{ ::some::path::to::an::object };
let construct_fn = Ident::new("new", Span::call_site());
let i = 0..5;

let path = std::iter::repeat_with(|| &path);
let construct_fn = std::iter::repeat_with(|| &construct_fn);
let output = quote!{
    #(object[#i] = #path::#construct_fn();)*
}

This case is not ambiguous and less error prone, as everything inside #(...)* must be an iterator and the variables that would intended to be used as ToTokens are explicitly stated as so with the std::iter::repeat_with before the quote!.

Unfortunately on crates.io, tarball which contains proc-quote-impl does not contain LICENSE files which is a problem from licensing POV. Also it block from packaging proc-quote in Fedora.

Thanks!

I just investigated a performance bug in a simple proc macro: illicitonion/num_enum#13

It generates a simple function which declares 7000 consts, and a match with 7000 branches.

If I hand-code the generated code as a string, cargo expand takes 6 seconds.
If I use proc-quote, cargo expand takes 94 seconds.
The only difference is how the TokenStream is generated.

There's a repro here: https://github.com/illicitonion/num_enum_regression_13 - the Cargo.toml has two options for where to get the num_enum dep switching between the implementations. This is the pull request switching from proc-quote to raw String generation: illicitonion/num_enum#14

This is the code that gets generated: https://gist.github.com/illicitonion/53d68708a1c4057ccdc7190d642d44f6

Are we using proc-quote wrong in some way? We'd love to get back to using it, as it's a really handy crate!

Thanks!

The following works in quote but fails in proc_quote:

#[test]
fn test_raw_ident() {
    quote!( r#the );
}

with the error

thread 'main' panicked at '"r#the" is not a valid Ident'
stack backtrace:
   7: proc_macro2::fallback::validate_term
   8: proc_macro2::fallback::Ident::_new
   9: proc_macro2::fallback::Ident::new
  10: proc_macro2::imp::Ident::new
  11: proc_macro2::Ident::new
  12: proc_quote::__rt::append_ident

Tests should be created to ensure that this crate behaves as expected.

After implementing tests, travis could also be set up.

Currently, this crate follows the syntax of dtolnay/quote, where the symbol # is used for interpolations. I wonder if this symbol was chosen because $ would clash with macro_rules!.

The (unstable) libproc_macro::quote uses $ rather than #. Would allowing this symbol as an alias to # be beneficial? Using $ would allow a smoother transition to libproc once quote is stable.

Although __proc_macro__as_repeat is a very unique name, in theory it can collide if the struct being interpolated in a repetition also implements some function with that name.

Trying to solve this with the fully qualified syntax Repeat::__proc_macro__as_repeat(something), would raise a new issue.

While iterators have to be consumed to be used (transfer ownership), slices and ToTokens shouldn't (borrow reference). On the other hand, the macro must generate code that simultaneously can work in both situations. This means Repeat::__proc_macro__as_repeat(&something) couldn't be generated, otherwise iterators wouldn't work. On the other hand, using Repeat::__proc_macro__as_repeat(something), the compiler doesn't infer the reference for some reason (while something.__proc_macro__as_repeat() does), so slices and ToTokens wouldn't work directly.

The user would need to borrow the variables before passing them into the macro, which could get very annoying:

let a = vec!["a", "b", "c"];
let b = [5,6,7];
let c = 5;
let d = "a";
let e = X;

let a = &a;
let b = &b;
let c = &c;
let d = &d;
let e = &e;
let q = quote!{ #(#a #b #c #d #e)* };

Allow nested iterators like the quote crate.
Example:

let nested = vec![vec!['a', 'b', 'c'], vec!['x', 'y', 'z']];

let tokens = quote! {
    #(
        #(#nested)*
    ),*
};

let expected = "'a' 'b' 'c' , 'x' 'y' 'z'";
assert_eq!(expected, tokens.to_string());

From quote test cases:

#[test]
fn test_empty_repetition() {
    let tokens = quote!(#(a b)* #(c d),*);
    assert_eq!("", tokens.to_string());
}

On one side, I feel inclined to allow this just to keep conformant behavior with the original quote. However, one the other side, I feel like this doesn't add anything to the crate and could even be masking an error, as a iteratorless repetition is probably just a mistake anyway.

Bringing @eddyb's comment on #4 to a new issue so as not to lose track of it.

There is a problem here, AFAICT something like this will repeat ad infinitum without warning (and end up allocating GBs of RAM until stopped by the OOM killer):

let x = quote!(_);
quote(#(#x)*);

If iter::repeat is involved, then its implementation of size_hint suggests this could be detected at runtime by asserting that combined_iterator.size_hint().0 < usize::MAX (but I'm not familiar enough with the implementation to be sure).

---- test_char stdout ----
thread 'test_char' panicked at 'assertion failed: `(left == right)`
  left: `"\'\\u{0}\' \'#\' \'\"\' \'\\\'\' \'\\n\' \'\\u{2764}\'"`,
 right: `"\'\\u{0}\' \'#\' \'\"\' \'\\\'\' \'\\n\' \'❤\'"`', tests/types.rs:70:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Not sure what other info you need.

Currently, #(#var)#var* is allowed, even though it doesn't really make sense. The current behaviour is outputing something like #var0 #var1 #var1 #var2 #var2 #var3 #var3.

It would be nice to keep a list of interpolations used between ) and *, compare them against the ones inside #(...) and raise an error if they are the same (so as to forbid this case).

The example @siavash-hamedani gave in dtolnay/quote#99:

let trait_name = ...
quote! {
    #[proc_macro_derive(#trait_name)]
    ...
    let derivee = ...
    ...
    quote!{
        impl #trait_name for ## derivee {
            ...
        }
    }
}

From looking at the source, it doesn't seem like adding this would be extremely difficult, however I'm not sure if there's some subtle effect that actually would make this impossible.

I'm also not sure if you'd be adding features that diverge from the existing quote! macro.

I don't think this could be done in a way that is 100% compatible with quote!, but one approach would be to a separate macro that allows some extended features. Maybe something like xquote!

Here's an example use case:

struct Blah { x: Ident, y: Ident }

impl ToTokens for Blah {
  fn to_tokens(&self, tokens: &mut TokenStream) {
    let x = self.x;
    let y = self.y;
    // Ignore that the actual generated code is nonsense, just an example.
    tokens.append_all(quote!( #x #y ))
  }
}

Instead of having to create a bunch of bindings before calling quote!:

struct Blah { x: Ident, y: Ident }

impl ToTokens for Blah {
  fn to_tokens(&self, tokens: &mut TokenStream) {
    tokens.append_all(xquote!( #$(self.x) #$(self.y) ))
  }
}

Another common case is having something that implements IntoIterator like an Option, but you have to create bindings with let x = x.into_iter() or to use an iterator multiple times it's necessary to do some something like:

let my_iter1 = blah.iter();
let my_iter2 = blah.iter();
quote!( #( #my_iter1 )* #( #my_iter2 )* )

Instead:

xquote!( #( #$( blah.iter() ) )* #( #$( blah.iter() ) )* )

Of course, it doesn't have to be #$( ... ) for the expression interpolation syntax.

Anyway, if this isn't something you'd be opposed to on principle then I'd probably look into working on this and submitting a PR if I get it to a workable state.

Some of the readme no longer applies now with quote 1.0. It would be good to focus the readme on the remaining advantages.

proc-macro2 introduced some breaking changes in their latest release. this crate recently automatically updated all dependencies and it seems to be incompatible w/this crate 😄

d262bd9#diff-80398c5faae3c069e4e6aa2ed11b28c0R20

example failed install log

Compiling liquid-derive v0.19.0
error[E0053]: method `to_tokens` has an incompatible type for trait
  --> /Users/me/.cargo/registry/src/github.com-xxxxxxxx/liquid-derive-0.19.0/src/filter/display.rs:34:5
   |
34 |     fn to_tokens(&self, tokens: &mut TokenStream) {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `proc_quote::__rt::TokenStream`, found struct `proc_macro2::TokenStream`
   |
   = note: expected type `fn(&filter::display::Parameters<'a>, &mut proc_quote::__rt::TokenStream)`
              found type `fn(&filter::display::Parameters<'a>, &mut proc_macro2::TokenStream)`
note: Perhaps two different versions of crate `proc_macro2` are being used?
  --> /Users/me/.cargo/registry/src/github.com-xxxxxxxxxx/liquid-derive-0.19.0/src/filter/display.rs:34:5
   |
34 |     fn to_tokens(&self, tokens: &mut TokenStream) {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0053]: method `to_tokens` has an incompatible type for trait
   --> /Users/me/.cargo/registry/src/github.com-xxxxxxx/liquid-derive-0.19.0/src/filter_parameters.rs:300:5
    |
300 |     fn to_tokens(&self, tokens: &mut TokenStream) {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `proc_quote::__rt::TokenStream`, found struct `proc_macro2::TokenStream`
    |
    = note: expected type `fn(&filter_parameters::FilterParameter<'a>, &mut proc_quote::__rt::TokenStream)`
               found type `fn(&filter_parameters::FilterParameter<'a>, &mut proc_macro2::TokenStream)`
note: Perhaps two different versions of crate `proc_macro2` are being used?
   --> /Users/me/.cargo/registry/src/github.com-xxxxxxx/liquid-derive-0.19.0/src/filter_parameters.rs:300:5
    |
300 |     fn to_tokens(&self, tokens: &mut TokenStream) {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0053`.
error: Could not compile `liquid-derive`.
warning: build failed, waiting for other jobs to finish...
error: failed to compile `cargo-generate v0.4.0`, intermediate artifacts can be found at `/var/folders/xx/xxxxxxxxxxx/T/cargo-installaxxxxx`