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 proc_macro::ToTokens #130977

Open
2 of 5 tasks
tgross35 opened this issue Sep 28, 2024 · 9 comments
Open
2 of 5 tasks

Tracking Issue for proc_macro::ToTokens #130977

tgross35 opened this issue Sep 28, 2024 · 9 comments
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. WG-macros Working group: Macros

Comments

@tgross35
Copy link
Contributor

tgross35 commented Sep 28, 2024

Feature gate: #![feature(proc_macro_totokens)]

This is a tracking issue for adding a ToTokens trait in proc_macro, which can then be used in proc_macro::quote!. See the ACP for motivation.

Public API

This will be similar to quote::ToTokens. That can be used as a reference for implementation details since it already provides all of these.

// proc_macro

pub trait ToTokens {
    fn to_tokens(&self, tokens: &mut TokenStream);
    fn to_token_stream(&self) -> TokenStream { ... }
    fn into_token_stream(self) -> TokenStream
       where Self: Sized { ... }
}

// Aggregate token types
impl ToTokens for TokenTree { /* ... */ }
impl ToTokens for TokenStream { /* ... */ }

// Single token types
impl ToTokens for Literal { /* ... */ }
impl ToTokens for Ident { /* ... */ }
impl ToTokens for Punct { /* ... */ }
impl ToTokens for Group { /* ... */ }

// Indirect types
impl<T: ToTokens + ?Sized> ToTokens for &T { /* ... */ }
impl<T: ToTokens + ?Sized> ToTokens for &mut T { /* ... */ }
impl<T: ToTokens + ?Sized> ToTokens for Box<T> { /* ... */ }
impl<T: ToTokens + ?Sized> ToTokens for Rc<T> { /* ... */ }
impl<T: ToTokens> ToTokens for Option<T> { /* ... */ }
impl<T: ToTokens + ToOwned + ?Sized> ToTokens for Cow<T> { /* ... */ }

// Types that can create `Literal`s
impl ToTokens for {u,i}{8,16,32,64,128} { /* ... */ }
impl ToTokens for f{16,32,64,128} { /* ... */ }
impl ToTokens for bool { /* ... */ }
impl ToTokens for char { /* ... */ }
impl ToTokens for str { /* ... */ }
impl ToTokens for String { /* ... */ }
impl ToTokens for CStr { /* ... */ }
impl ToTokens for CString { /* ... */ }

/* migrate the following APIs, if possible without breakage */

// currently `Extend<TokenStream>` and `Extend<TokenTree>`
impl Extend<T: ToTokens> for TokenStream { /* ... */ }

// currently `FromIterator<TokenStream>` and `FromIterator<TokenTree>`
impl FromIterator<T: ToTokens> for TokenStream { /* ... */ }

Steps / History

Unresolved Questions

  • What should this be named? ToTokens doesn't seem quite accurate, but I don't know what would be better (ToTokenStream? ExtendTokenStream? Those seem a bit clunky).
  • Considering impl<T: ToTokens> ToTokens for T is provided, should to_tokens take self by value rather than by reference so cloning isn't always necessary? (fn to_tokens(self, tokens: &mut TokenStream))

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@tgross35 tgross35 added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros WG-macros Working group: Macros labels Sep 28, 2024
@tgross35
Copy link
Contributor Author

I haven't had a chance to work on this yet and probably won't for a little while, so somebody else is welcome to pick this up. Should be pretty easy so I'll label it as such.

@tgross35 tgross35 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels Sep 28, 2024
@programmerjake
Copy link
Member

programmerjake commented Sep 29, 2024

I think a lot of those impls need ?Sized (I'm assuming you meant for the implementer to infer that, but just in case it isn't clear):

// Indirect types
impl<T: ToTokens + ?Sized> ToTokens for &T { /* ... */ }
impl<T: ToTokens + ?Sized> ToTokens for &mut { /* ... */ }
impl<T: ToTokens + ?Sized> ToTokens for Box<T> { /* ... */ }
impl<T: ToTokens + ?Sized> ToTokens for Rc<T> { /* ... */ }
impl<T: ToTokens> ToTokens for Option<T> { /* ... */ }
impl<T: ToTokens + ?Sized> ToTokens for Cow<T> { /* ... */ }

@tgross35
Copy link
Contributor Author

tgross35 commented Sep 29, 2024

Thanks, I updated the top post. I hadn't thought about that :)

@SpriteOvO
Copy link
Contributor

@rustbot claim

BTW, the link in the section "Public API" should be quote::ToTokens instead of proc_macro::ToTokens.

@SpriteOvO
Copy link
Contributor

SpriteOvO commented Oct 9, 2024

I have opened a PR #131441 for the initial implementation.

Considering impl<T: ToTokens> ToTokens for T is provided, should to_tokens take self by value rather than by reference so cloning isn't always necessary? (fn to_tokens(self, tokens: &mut TokenStream))

I have the same concerns about the unnecessary clone, and commented in #131441 (comment). In the current quote! macro, we have ownership of T. I think taking it by value would be better.

If this change will be applied, a more reasonable definition of the trait might be

trait IntoTokenStream {
    fn into_token_stream(self) -> TokenStream;

    fn extend_token_stream(self, tokens: &mut TokenStream) {
        tokens.extend_one(self.into_token_stream());
    }
}

Just a quick thought.

@tgross35
Copy link
Contributor Author

I think taking by value probably makes sense too, and then just cloning for the &T impls. But I think it would be best to (1) finish #131441 as-is, (2) update pm::quote to use it and add tests to make sure it does what we want, (3) after we have that, try a PR that changes &self to self and see if anything unexpected happens.

@programmerjake
Copy link
Member

programmerjake commented Oct 13, 2024

unless the macro auto-clones as necessary, ToTokens taking self by value would make it very difficult to use for repeated expansions like:

let ty: TokenStream = ...;
let fields: Vec<Ident> = ...;
quote::quote! {
    struct S {
        #(#fields: #ty,)* // calls ty.to_tokens() once for every field
    }
}

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 28, 2024
…it, r=dtolnay

Add a new trait `proc_macro::ToTokens`

Tracking issue rust-lang#130977

This PR adds a new trait `ToTokens`, implemented for types that can be interpolated inside a `quote!` invocation.

```rust
impl ToTokens for TokenTree
impl ToTokens for TokenStream
impl ToTokens for Literal
impl ToTokens for Ident
impl ToTokens for Punct
impl ToTokens for Group
impl<T: ToTokens + ?Sized> ToTokens for &T
impl<T: ToTokens + ?Sized> ToTokens for &mut T
impl<T: ToTokens + ?Sized> ToTokens for Box<T>
impl<T: ToTokens + ?Sized> ToTokens for Rc<T>
impl<T: ToTokens + ToOwned + ?Sized> ToTokens for Cow<'_, T>
impl<T: ToTokens> ToTokens for Option<T>
impl ToTokens for u{8,16,32,64,128}
impl ToTokens for i{8,16,32,64,128}
impl ToTokens for f{32,64}
impl ToTokens for {u,i}size
impl ToTokens for bool
impl ToTokens for char
impl ToTokens for str
impl ToTokens for String
impl ToTokens for CStr
impl ToTokens for CString
```

~This PR also implements the migration mentioned in the tracking issue, replacing `Extend<Token{Tree,Stream}>` with `Extend<T: ToTokens>`, and replacing `FromIterator<Token{Tree,Stream}>` with `FromIterator<T: ToTokens>`.~
**UPDATE**: Reverted.

```diff
-impl FromIterator<TokenTree> for TokenStream
-impl FromIterator<TokenStream> for TokenStream
+impl<T: ToTokens> FromIterator<T> for TokenStream

-impl Extend<TokenTree> for TokenStream
-impl Extend<TokenStream> for TokenStream
+impl<T: ToTokens> Extend<T> for TokenStream
```

I'm going to leave some comments in the review where I'm unsure and concerned.

r? `@dtolnay`
CC `@tgross35`
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 28, 2024
…it, r=dtolnay

Add a new trait `proc_macro::ToTokens`

Tracking issue rust-lang#130977

This PR adds a new trait `ToTokens`, implemented for types that can be interpolated inside a `quote!` invocation.

```rust
impl ToTokens for TokenTree
impl ToTokens for TokenStream
impl ToTokens for Literal
impl ToTokens for Ident
impl ToTokens for Punct
impl ToTokens for Group
impl<T: ToTokens + ?Sized> ToTokens for &T
impl<T: ToTokens + ?Sized> ToTokens for &mut T
impl<T: ToTokens + ?Sized> ToTokens for Box<T>
impl<T: ToTokens + ?Sized> ToTokens for Rc<T>
impl<T: ToTokens + ToOwned + ?Sized> ToTokens for Cow<'_, T>
impl<T: ToTokens> ToTokens for Option<T>
impl ToTokens for u{8,16,32,64,128}
impl ToTokens for i{8,16,32,64,128}
impl ToTokens for f{32,64}
impl ToTokens for {u,i}size
impl ToTokens for bool
impl ToTokens for char
impl ToTokens for str
impl ToTokens for String
impl ToTokens for CStr
impl ToTokens for CString
```

~This PR also implements the migration mentioned in the tracking issue, replacing `Extend<Token{Tree,Stream}>` with `Extend<T: ToTokens>`, and replacing `FromIterator<Token{Tree,Stream}>` with `FromIterator<T: ToTokens>`.~
**UPDATE**: Reverted.

```diff
-impl FromIterator<TokenTree> for TokenStream
-impl FromIterator<TokenStream> for TokenStream
+impl<T: ToTokens> FromIterator<T> for TokenStream

-impl Extend<TokenTree> for TokenStream
-impl Extend<TokenStream> for TokenStream
+impl<T: ToTokens> Extend<T> for TokenStream
```

I'm going to leave some comments in the review where I'm unsure and concerned.

r? ``@dtolnay``
CC ``@tgross35``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 29, 2024
…it, r=dtolnay

Add a new trait `proc_macro::ToTokens`

Tracking issue rust-lang#130977

This PR adds a new trait `ToTokens`, implemented for types that can be interpolated inside a `quote!` invocation.

```rust
impl ToTokens for TokenTree
impl ToTokens for TokenStream
impl ToTokens for Literal
impl ToTokens for Ident
impl ToTokens for Punct
impl ToTokens for Group
impl<T: ToTokens + ?Sized> ToTokens for &T
impl<T: ToTokens + ?Sized> ToTokens for &mut T
impl<T: ToTokens + ?Sized> ToTokens for Box<T>
impl<T: ToTokens + ?Sized> ToTokens for Rc<T>
impl<T: ToTokens + ToOwned + ?Sized> ToTokens for Cow<'_, T>
impl<T: ToTokens> ToTokens for Option<T>
impl ToTokens for u{8,16,32,64,128}
impl ToTokens for i{8,16,32,64,128}
impl ToTokens for f{32,64}
impl ToTokens for {u,i}size
impl ToTokens for bool
impl ToTokens for char
impl ToTokens for str
impl ToTokens for String
impl ToTokens for CStr
impl ToTokens for CString
```

~This PR also implements the migration mentioned in the tracking issue, replacing `Extend<Token{Tree,Stream}>` with `Extend<T: ToTokens>`, and replacing `FromIterator<Token{Tree,Stream}>` with `FromIterator<T: ToTokens>`.~
**UPDATE**: Reverted.

```diff
-impl FromIterator<TokenTree> for TokenStream
-impl FromIterator<TokenStream> for TokenStream
+impl<T: ToTokens> FromIterator<T> for TokenStream

-impl Extend<TokenTree> for TokenStream
-impl Extend<TokenStream> for TokenStream
+impl<T: ToTokens> Extend<T> for TokenStream
```

I'm going to leave some comments in the review where I'm unsure and concerned.

r? ```@dtolnay```
CC ```@tgross35```
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 29, 2024
…it, r=dtolnay

Add a new trait `proc_macro::ToTokens`

Tracking issue rust-lang#130977

This PR adds a new trait `ToTokens`, implemented for types that can be interpolated inside a `quote!` invocation.

```rust
impl ToTokens for TokenTree
impl ToTokens for TokenStream
impl ToTokens for Literal
impl ToTokens for Ident
impl ToTokens for Punct
impl ToTokens for Group
impl<T: ToTokens + ?Sized> ToTokens for &T
impl<T: ToTokens + ?Sized> ToTokens for &mut T
impl<T: ToTokens + ?Sized> ToTokens for Box<T>
impl<T: ToTokens + ?Sized> ToTokens for Rc<T>
impl<T: ToTokens + ToOwned + ?Sized> ToTokens for Cow<'_, T>
impl<T: ToTokens> ToTokens for Option<T>
impl ToTokens for u{8,16,32,64,128}
impl ToTokens for i{8,16,32,64,128}
impl ToTokens for f{32,64}
impl ToTokens for {u,i}size
impl ToTokens for bool
impl ToTokens for char
impl ToTokens for str
impl ToTokens for String
impl ToTokens for CStr
impl ToTokens for CString
```

~This PR also implements the migration mentioned in the tracking issue, replacing `Extend<Token{Tree,Stream}>` with `Extend<T: ToTokens>`, and replacing `FromIterator<Token{Tree,Stream}>` with `FromIterator<T: ToTokens>`.~
**UPDATE**: Reverted.

```diff
-impl FromIterator<TokenTree> for TokenStream
-impl FromIterator<TokenStream> for TokenStream
+impl<T: ToTokens> FromIterator<T> for TokenStream

-impl Extend<TokenTree> for TokenStream
-impl Extend<TokenStream> for TokenStream
+impl<T: ToTokens> Extend<T> for TokenStream
```

I'm going to leave some comments in the review where I'm unsure and concerned.

r? ````@dtolnay````
CC ````@tgross35````
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 29, 2024
Rollup merge of rust-lang#131441 - SpriteOvO:proc-macro-to-tokens-trait, r=dtolnay

Add a new trait `proc_macro::ToTokens`

Tracking issue rust-lang#130977

This PR adds a new trait `ToTokens`, implemented for types that can be interpolated inside a `quote!` invocation.

```rust
impl ToTokens for TokenTree
impl ToTokens for TokenStream
impl ToTokens for Literal
impl ToTokens for Ident
impl ToTokens for Punct
impl ToTokens for Group
impl<T: ToTokens + ?Sized> ToTokens for &T
impl<T: ToTokens + ?Sized> ToTokens for &mut T
impl<T: ToTokens + ?Sized> ToTokens for Box<T>
impl<T: ToTokens + ?Sized> ToTokens for Rc<T>
impl<T: ToTokens + ToOwned + ?Sized> ToTokens for Cow<'_, T>
impl<T: ToTokens> ToTokens for Option<T>
impl ToTokens for u{8,16,32,64,128}
impl ToTokens for i{8,16,32,64,128}
impl ToTokens for f{32,64}
impl ToTokens for {u,i}size
impl ToTokens for bool
impl ToTokens for char
impl ToTokens for str
impl ToTokens for String
impl ToTokens for CStr
impl ToTokens for CString
```

~This PR also implements the migration mentioned in the tracking issue, replacing `Extend<Token{Tree,Stream}>` with `Extend<T: ToTokens>`, and replacing `FromIterator<Token{Tree,Stream}>` with `FromIterator<T: ToTokens>`.~
**UPDATE**: Reverted.

```diff
-impl FromIterator<TokenTree> for TokenStream
-impl FromIterator<TokenStream> for TokenStream
+impl<T: ToTokens> FromIterator<T> for TokenStream

-impl Extend<TokenTree> for TokenStream
-impl Extend<TokenStream> for TokenStream
+impl<T: ToTokens> Extend<T> for TokenStream
```

I'm going to leave some comments in the review where I'm unsure and concerned.

r? ``@dtolnay``
CC ``@tgross35``
@tgross35
Copy link
Contributor Author

@SpriteOvO are you still interested in working on this? The expansion of proc_macro::quote! at https://github.com/rust-lang/rust/blob/13b77c687c0f5ad06a579af6d0787fe562747501/library/proc_macro/src/quote.rs still needs to be updated (no worries if not, I'll just unassign you).

@SpriteOvO
Copy link
Contributor

@tgross35 Oh, I missed that step.. I'm still willing to work on it, just a bit busy recently, probably this week or next.

@tgross35 tgross35 removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels Dec 20, 2024
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, ..) A-proc-macros Area: Procedural macros C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. WG-macros Working group: Macros
Projects
None yet
Development

No branches or pull requests

3 participants