rust-lang / libs-team

The home of the library team
Apache License 2.0
116 stars 18 forks source link

proc_macro: Implementation of Extend (parameterized over Group, Ident, Punct, and Literal) for TokenStream #242

Closed JohnScience closed 1 year ago

JohnScience commented 1 year ago

Proposal

Problem statement

At the moment of writing, proc_macro::TokenStream implements parametrizations of core::iter::Extend over proc_macro::TokenStream and proc_macro::TokenTree, i.e. Extend<TokenStream> and Extend<TokenTree>. However, it is not implemented for parametrizations of Extend over

  1. proc_macro::Group
  2. proc_macro::Ident
  3. proc_macro::Punct
  4. proc_macro::Literal

which all can can be stored in respective variants of TokenTree.

I believe that this is a problem because it either

  1. causes unnecessary verbosity/boilerplate when one of Group, Ident, Punct, or Literal, is added to a TokenStream, as will be demonstrated further; or
  2. forces the developers to implement a local twin of Extend that they would implement for TokenStream, either manually or using a non-standard library.

Such inconveniences could push developers use quote, which results in higher compile times, even in situations where proc_macro with the proposed changes would suffice.

Also, it probably results in higher compile times in projects which extensively use procedural macros because these macros have to generate more Rust syntax.

Motivating examples or use cases

Before:

use proc_macro::{TokenStream, TokenTree, Ident, Span};
let mut ts = TokenStream::new();
ts.extend(iter::once(TokenTree::Ident(Ident::new("new_ident", Span::call_site()))));

After:

use proc_macro::{TokenStream, TokenTree, Ident, Span};
let mut ts = TokenStream::new();
ts.extend(iter::once(Ident::new("new_ident", Span::call_site())));

Once (or when) Extend::extend_one stabilizes, the difference will be even more noticeable.

Before:

use proc_macro::{TokenStream, TokenTree, Ident, Span};
let mut ts = TokenStream::new();
ts.extend_one(TokenTree::Ident(Ident::new("new_ident", Span::call_site())));

After:

use proc_macro::{TokenStream, TokenTree, Ident, Span};
let mut ts = TokenStream::new();
ts.extend_one(Ident::new("new_ident", Span::call_site()));

Solution sketch

impl Extend<Group> for TokenStream {
    fn extend<I: IntoIterator<Item = Group>>(&mut self, groups: I) {
        self.extend(groups.into_iter().map(TokenTree::Group))
    }
}

// And so on for the 3 remaining items, or we can use a macro_rules! for that.

Alternatives

Leave it as it is and delegate the implementation to a non-standard library via an extension trait that would be a twin of Extend. I dislike this approach because all these types belong to standard libraries and it would result in unnecessary complexity.

Links and related work

dtolnay commented 1 year ago

Seconded. Please send PR.

eggyal commented 1 year ago

TokenTree already implements From<Group> etc, so it's possible today to do stream.extend(iter::once(TokenTree::from(...))) which this proposal would simplify to stream.extend(iter::once(...)).

JohnScience commented 1 year ago

Development Progress

It's my first contribution to the standard library so I might make some silly mistakes as I make it happen. In order to make the development of the PR more transparent, I'll document what I'm doing along the way.

Steps taken

Found Standard library developers Guide.

Took a mental note of the Standard library developers Guide > 2. Building and debugging libraries,

Most of the instructions from the rustc-dev-guide also apply to the standard library since it is built with the same build system, so it is recommended to read it first.

From Rust Compiler Development Guide > Getting Started I learned about mentored issues.

Q: What are the benefits? It looks like I could use some help.

Followed Rust Compiler Development Guide > Building and debugging rustc > 1. How to build and run the compiler and shallow-cloned the repo, as suggested.

Tried ./x.py setup on Windows. Got the following error:

'.' is not recognized as an internal or external command, operable program or batch file.

Q: should the doc be updated to make it aware of Windows?

Tried python x.py setup and it worked.

Installed x, binary-wrapper around x.py, as suggested. However, x test library/proc_macro opened x.py in vscode, instead of running it.

Q: I guess I should report this bug later?

Forked rust-lang/rust, thereby getting JohnScience/rust.

Shallow-cloned and setup the fork.

Started adding a new feature gate by following Rust Compiler Development Guide > Contributing to Rust > 15. Feature Gates:

Q: I do need to add another feature gate, right?

...
tidy check
Expected a gate test for the feature 'ts_extend_w_tt_item'.
Hint: create a failing test file named 'feature-gate-ts_extend_w_tt_item.rs'
      in the 'ui' test suite, with its failures due to
      missing usage of `#![feature(ts_extend_w_tt_item)]`.
Hint: If you already have such a test and don't want to rename it,
      you can also add a // gate-test-ts_extend_w_tt_item line to the test file.
tidy error: Found 1 features without a gate test.
some tidy checks failed
Build completed unsuccessfully in 0:08:43
error: failed to push some refs to 'https://github.com/JohnScience/rust'

Q: should the chapter on feature gates in Rust Compiler Development Guide be updated to add this info?


To resolve the error and pass the git hook used for the development of rustc, you need to create a test file for the feature gate you added. The error message suggests creating a failing test file named 'feature-gate-ts_extend_w_tt_item.rs' in the 'ui' test suite, which reflects the feature 'ts_extend_w_tt_item'. Here's how you can write the test file:

  1. Navigate to the 'ui' test suite directory of your Rust fork. The path would typically be <your-rust-fork>/src/test/ui/.

  2. Create a new file named 'feature-gate-ts_extend_w_tt_item.rs' in that directory.

  3. Open the 'feature-gate-ts_extend_w_tt_item.rs' file in a text editor and add the following content:

// gate-test-ts_extend_w_tt_item
// compile-flags: --error-format human --edition 2018
// check-pass
// aux-build:feature-gate-ts_extend_w_tt_item.rs

extern crate feature_gate_ts_extend_w_tt_item;

fn main() {
    // Add test code here that uses the feature gated by ts_extend_w_tt_item.
    // Make sure the code compiles and runs correctly when the feature is enabled.
}

Let's break down the contents of the file:

Save the file, commit your changes, and push them to your fork of rust-lang/rust. The git hook should now accept the push, as you have created the necessary test file for the feature gate.


Q: I haven't found the list of the comments in the Rust Compiler Development Guide. Is it a thing that is yet to be documented?

Q: Can't git hook either propose changes or apply them automatically?

tidy error: following path contains more than 870 entries, you should move the test to some relevant subdirectory (current: 871): C:\Users\USER\Documents\github\rust\tests\ui

and after asking on official Discord server in #contribute channel figured out that the point of that lint is to prevent the mess from growing. I haven't figured out whether it goes to proc-macro or feature-gates, so I chose proc-macro.

Implementation status

Status: Postponed until the beginning of August

Possible statuses

Active - doing the work. Coffee break - enjoying some rest but will come back soon (probably). Lengthy break - sleep or walk in a park.

m-ou-se commented 1 year ago

Started adding a new feature gate by following Rust Compiler Development Guide > Contributing to Rust > 15. Feature Gates:

That documentation is for compiler features. For library features, you don't need to touch any files in compiler/, nor do you need to add any feature gate tests. For library features, just an #[unstable] attribute is enough. (We should document this better, sorry.)

This specific feature however are trait implementations (for already stable types and trait), which unfortunately cannot be unstable. So your PR would need to add these implementations as stable right away (and requires team signoff through an FCP).

jyn514 commented 1 year ago

It's my first contribution to the standard library so I might make some silly mistakes as I make it happen. In order to make the development of the PR more transparent, I'll document what I'm doing along the way.

@JohnScience this is extremely helpful, thank you ❤️ i don't have time right now but i plan to improve several of the issues you mentioned this weekend

m-ou-se commented 1 year ago

@jyn514 and I just agreed to spend some time working together to make sure the rustc and std dev guides cover these things better and refer to each other in the proper places. Stay tuned. :)

Thanks for documenting your experience, @JohnScience.

jyn514 commented 1 year ago

Q: should the doc be updated to make it aware of Windows?

I've opened https://github.com/rust-lang/rustc-dev-guide/pull/1701/files?short_path=cdf7c1e#diff-cdf7c1e6d2d1ec81510204b0c3250cb2c90880843f005bb942bafb84996a90b0 to improve the docs here.

and after asking on official Discord server in #contribute channel figured out that the point of that lint is to prevent the mess from growing. I haven't figured out whether it goes to proc-macro or feature-gates, so I chose proc-macro.

I made this error from tidy more helpful in https://github.com/rust-lang/rust/pull/113008/files#diff-31008ceb5198220e3ec8834cac868728b9db572697eb5a14e48b402edc9c2246.

Q: I guess I should report this bug later?

I cannot replicate this bug. It looks like you're using cmd.exe, but when I run x test library/proc_macro in cmd it does the right thing automatically. Are you sure that you've added ~/.cargo/bin to PATH?

Tried ./x.py setup on Windows. Got the following error:

We do not really support cmd.exe; it's horribly limited in what it can do. Powershell should work a lot better. I tried to document that in the rustc-dev-guide PR.

Q: I haven't found the list of the comments in the Rust Compiler Development Guide. Is it a thing that is yet to be documented?

This is documented at https://rustc-dev-guide.rust-lang.org/tests/headers.html#header-commands. I'm not sure how to make it easier to find.

  • Started playing around with rg and huniq to find more comments but the terminal crashed. Due to that, I decided to create a test using only limited knowledge.

    • Faced problem related to LF vs CRLF. Fixed it in Visual Studio Code.

Q: Can't git hook either propose changes or apply them automatically?

Can you talk more about the error you ran into here? do you mean this?

tidy check
tidy error: C:\Users\vboxuser\rust\tests\ui\feature-gates\foo.rs:1: CR character

it would be helpful if you could open an issue on rust-lang/rust asking for this to be --blessable :)