Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upproc_macro: Reorganize public API #49597
Conversation
alexcrichton
requested a review
from
nrc
Apr 2, 2018
rust-highfive
assigned
nikomatsakis
Apr 2, 2018
This comment has been minimized.
This comment has been minimized.
|
cc @dtolnay, @nrc, @jseyfried |
This comment has been minimized.
This comment has been minimized.
|
r? @nrc |
rust-highfive
assigned
nrc
and unassigned
nikomatsakis
Apr 2, 2018
This comment has been minimized.
This comment has been minimized.
Are there concrete plans on changes there? Has this been documented anywhere? |
This comment has been minimized.
This comment has been minimized.
|
@sgrif AFAIK there are no concrete plans yet, so no documentation either |
dtolnay
approved these changes
Apr 2, 2018
This comment has been minimized.
This comment has been minimized.
|
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton LMK if there's any discussion. Diesel is using pretty much all the methods on |
alexcrichton
force-pushed the
alexcrichton:proc-macro-v2
branch
2 times, most recently
from
e211ef8
to
2b24ea5
Apr 2, 2018
This comment has been minimized.
This comment has been minimized.
|
@sgrif will do! |
alexcrichton
referenced this pull request
Apr 2, 2018
Closed
Incorrect and inconsistent jointness of tokens in desugared doc comment #49596
alexcrichton
force-pushed the
alexcrichton:proc-macro-v2
branch
from
2b24ea5
to
553c04d
Apr 2, 2018
pietroalbini
added
the
S-waiting-on-review
label
Apr 2, 2018
alexcrichton
referenced this pull request
Apr 2, 2018
Closed
Inconsistent tokens between the same struct defined inside vs outside of function #49604
This comment has been minimized.
This comment has been minimized.
|
Ok, maybe it's time to ask before it's stabilized. I always found it actively confusing. |
This comment has been minimized.
This comment has been minimized.
|
Maybe |
This comment has been minimized.
This comment has been minimized.
|
@petrochenkov I think |
This comment has been minimized.
This comment has been minimized.
Sigh, macros 1.1 strike again.
As I see it: enum TokenTree {
Leaf(LeafAuxData /* leaf kind, span, ... */),
Subtree(Vec<TokenTree>, SubTreeAuxData /* delimiter kind, span, ... */),
}In this sense The current library, however introduces one more layer of abstraction than strictly necessary - type TokenStream = impl Iterator<TokenTree> + Aux /* printing, parsing, ...*/;From this perspective the name "stream" actually starts making sense. I'm not 100% sure this additional layer is necessary, but it probably is, at least it's certainly better to put something like I also can guess where this scheme comes from historically - |
petrochenkov
approved these changes
Apr 3, 2018
This comment has been minimized.
This comment has been minimized.
|
The only thing I'm not entirely happy about is naming for |
This comment has been minimized.
This comment has been minimized.
|
Ok thanks for the review! I talked about this pretty extensively with @nrc at the recent all-hands so I'm going to go ahead and... @bors: r=petrochenkov @dtolnay do you have thoughts on @petrochenkov's thoughts? I'd personally agree that |
This comment has been minimized.
This comment has been minimized.
|
|
bors
removed
the
S-waiting-on-review
label
Apr 3, 2018
alexcrichton
referenced this pull request
Apr 4, 2018
Closed
Support inner doc comments in proc_macro #49656
alexcrichton
force-pushed the
alexcrichton:proc-macro-v2
branch
from
21e8cd2
to
a57b1fb
Apr 4, 2018
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
alexcrichton
referenced this pull request
Apr 4, 2018
Closed
Wrong tokenization of doc comments in proc_macro #49655
petrochenkov
self-assigned this
Apr 4, 2018
This comment has been minimized.
This comment has been minimized.
|
lgtm, r+ |
nrc
approved these changes
Apr 5, 2018
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Apr 5, 2018
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Apr 5, 2018
bors
added a commit
that referenced
this pull request
Apr 5, 2018
alexcrichton
merged commit a57b1fb
into
rust-lang:master
Apr 5, 2018
1 check passed
alexcrichton
deleted the
alexcrichton:proc-macro-v2
branch
Apr 5, 2018
SimonSapin
referenced this pull request
Apr 6, 2018
Closed
Regression: `FromIterator<TokenStream>` is not implemented for `TokenStream` #49725
This comment has been minimized.
This comment has been minimized.
|
I think this has regressed a stable impl: #49725 |
alexcrichton commentedApr 2, 2018
•
edited
This commit is a reorganization of the
proc_macrocrate's public user-facingAPI. This is the result of a number of discussions at the recent Rust All-Hands
where we're hoping to get the
proc_macrocrate into ship shape forstabilization of a subset of its functionality in the Rust 2018 release.
The reorganization here is motivated by experiences from the
proc-macro2,quote, andsyncrates on crates.io (and other crates which depend on them).The main focus is future flexibility along with making a few more operations
consistent and/or fixing bugs. A summary of the changes made from today's
proc_macroAPI is:The
TokenNodeenum has been removed and the public fields ofTokenTreehave also been removed. Instead the
TokenTreetype is now a public enum(what
TokenNodewas) and each variant is an opaque struct which internallycontains
Spaninformation. This makes the various tokens a bit moreconsistent, require fewer wrappers, and otherwise provides good
future-compatibility as opaque structs are easy to modify later on.
Literalinteger constructors have been expanded to be unambiguous as to whatthey're doing and also allow for more future flexibility. Previously
constructors like
Literal::floatandLiteral::integerwere used to createunsuffixed literals and the concrete methods like
Literal::i32would createa suffixed token. This wasn't immediately clear to all users (the
suffixed/unsuffixed aspect) and having one constructor for unsuffixed
literals required us to pick a largest type which may not always be true. To
fix these issues all constructors are now of the form
Literal::i32_unsuffixedorLiteral::i32_suffixed(for all integral types).This should allow future compatibility as well as being immediately clear
what's suffixed and what isn't.
Each variant of
TokenTreeinternally contains aSpanwhich can also beconfigured via
set_span. For exampleLiteralandTermnow bothinternally contain a
Spanrather than having it stored in an auxiliarylocation.
Constructors of all tokens are called
newnow (akaTerm::internis gone)and most do not take spans. Manufactured tokens typically don't have a fresh
span to go with them and the span is purely used for error-reporting
except the span for
Term, which currently affects hygiene. The defaultspans for all these constructed tokens is
Span::call_site()for now.The
Termtype's constructor explicitly requires passing in aSpantoprovide future-proofing against possible hygiene changes. It's intended that a
first pass of stabilization will likely only stabilize
Span::call_site()which is an explicit opt-in for "I would like no hygiene here please". The
intention here is to make this explicit in procedural macros to be
forwards-compatible with a hygiene-specifying solution.
Some of the conversions for
TokenStreamhave been simplified a little.The
TokenTreeIteriterator was renamed totoken_stream::IntoIter.Overall the hope is that this is the "final pass" at the API of
TokenStreamand most of
TokenTreebefore stabilization. Explicitly left out here is anychanges to
Span's API which will likely need to be re-evaluated beforestabilization.
All changes in this PR have already been reflected to the
proc-macro2,quote, andsyncrates. New versions of all these crates have also beenpublished to crates.io.
Once this lands in nightly I plan on making an internals post again summarizing
the changes made here and also calling on all macro authors to give the APIs a
spin and see how they work. Hopefully pending no major issues we can then have
an FCP to stabilize later this cycle!
Closes #49596