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

Rewrite tokenization with proc-macro2 tokens #146

Merged
merged 1 commit into from
May 26, 2017

Conversation

alexcrichton
Copy link
Collaborator

This ended up being a bit larger of a commit than I intended! I imagine that
this'll be one of the larger of the commits working towards #142. The purpose of
this commit is to use an updated version of the quote crate which doesn't work
with strings but rather works with tokens form the proc-macro2 crate. The
proc-macro2 crate itself is based on the proposed API for proc_macro itself,
and will continue to mirror it. The hope is that we'll flip an easy switch
eventually to use compiler tokens, whereas for now we'll stick to string parsing
at the lowest layer.

The largest change here is the addition of span information to the AST. Building
on the previous PRs to refactor the AST this makes it relatively easy from a
user perspective to digest and use the AST still, it's just a few extra fields
on the side. The fallout from this was then quite large throughout the
printing feature of the crate. The parsing, fold, and visit features
then followed suit to get updated as well.

This commit also changes the the semantics of the AST somewhat as well.
Previously it was inferred what tokens should be printed, for example if you
have a closure argument syn would automatically not print the colon in a: b
if the type listed was "infer this type". Now the colon is a separate field and
must be in sync with the type listed as the colon/type will be printed
unconditionally (emitting no output if both are None).

This ended up being a bit larger of a commit than I intended! I imagine that
this'll be one of the larger of the commits working towards dtolnay#142. The purpose of
this commit is to use an updated version of the `quote` crate which doesn't work
with strings but rather works with tokens form the `proc-macro2` crate. The
`proc-macro2` crate itself is based on the proposed API for `proc_macro` itself,
and will continue to mirror it. The hope is that we'll flip an easy switch
eventually to use compiler tokens, whereas for now we'll stick to string parsing
at the lowest layer.

The largest change here is the addition of span information to the AST. Building
on the previous PRs to refactor the AST this makes it relatively easy from a
user perspective to digest and use the AST still, it's just a few extra fields
on the side. The fallout from this was then quite large throughout the
`printing` feature of the crate. The `parsing`, `fold`, and `visit` features
then followed suit to get updated as well.

This commit also changes the the semantics of the AST somewhat as well.
Previously it was inferred what tokens should be printed, for example if you
have a closure argument `syn` would automatically not print the colon in `a: b`
if the type listed was "infer this type". Now the colon is a separate field and
must be in sync with the type listed as the colon/type will be printed
unconditionally (emitting no output if both are `None`).
@alexcrichton
Copy link
Collaborator Author

alexcrichton commented May 26, 2017

Alright so this is a bit of a monster PR! I'm hoping this is the final version of the AST before we close out #142. Next steps I can see are:

So far I'm pretty happy with how this turned out, it ended up being pretty easy to implement parsing, ToTokens, visiting, and folding after this change. I don't think there's redundant information (only what's needed), but an extra pair of eyes is always helpful!

@alexcrichton
Copy link
Collaborator Author

I'll also note that I ended up diverging somewhat from the proposed proc_macro API. I tried to stick as closely as possible to the current revision in rust-lang/rust#40939 as possible, but to get all the tests passing in this repo (especially the round-trip ones) I had to significantly bolster the Literal API to meet the various needs.

I imagine we'll solve all these issues before stabilizing proc_macro, however.

@alexcrichton
Copy link
Collaborator Author

Also note that this does not parse from tokens yet, this only updates tokenization. All parser code is "untouched" minus the updates necessary to get this new AST being generated.

@dtolnay
Copy link
Owner

dtolnay commented May 26, 2017

This is pretty clearly the future so I am comfortable merging this and following up with any necessary tweaks separately, rather than futzing with dependent PRs.

My only concern is I would like to have the real proc-macro API from rust-lang/rust#40939 in better shape before publishing either of proc-macro2 or syn. Experimenting with a bolstered API is fine but when publishing a syn release I would like to make sure that proc-macro2 has "stable" and "unstable" modules that are perfectly identical.

@dtolnay dtolnay merged commit d630f9d into dtolnay:master May 26, 2017
@alexcrichton alexcrichton deleted the new-tokens2 branch May 27, 2017 03:10
@alexcrichton
Copy link
Collaborator Author

Yeah sounds good to me. If it's ok I think we should hold off on publishing proc-macro2 and the next version of this crate until the change to rustc happens. I think that the current API is sufficient albeit very difficult to work around, but we could make do if need be.

My current thinking is that when the proc_macro API lands on nightly we'll freeze the stable module and the interface of the proc_macro2 crate to look exactly like that. As the proc_macro API changes (if it does) we'll update the unstable module to do a translation between the old and the new, maintaining API compatibility of the proc_macro2 crate itself. Then finally once we get closer to stabilization I figure we can release a new breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants