-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Adds a ProcMacro
form of syntax extension
#36154
Conversation
|
||
struct TokResult<'a> { | ||
parser: Parser<'a>, | ||
span: Span, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the call site span of the macro invocation that expanded into the TokResult
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
☔ The latest upstream changes (presumably #35957) made this pull request unmergeable. Please resolve the merge conflicts. |
This commit adds syntax extension forms matching the types for procedural macros 2.0 (RFC rust-lang#1566), these still require the usual syntax extension boiler plate, but this is a first step towards proper implementation and should be useful for macros 1.1 stuff too. Supports both attribute-like and function-like macros.
56405da
to
973eae2
Compare
@jseyfried r? I think I addressed all your earlier comments |
let parser = self.cx.new_parser_from_tts(&tok_result.to_tts()); | ||
let result = Box::new(TokResult { parser: parser, span: attr.span }); | ||
|
||
let items: Vec<Annotatable> = match item { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This match
could be the trailing expression statement if each arm had type Expansion
.
match result.make_items() { | ||
Some(items) => items.into_iter() | ||
.map(|i| Annotatable::Item(i)) | ||
.collect(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. this could be Some(items) => Expansion::Item(items),
Some(t) => self.pos > t, | ||
None => false, | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: extra newline
pub fn from_span<'b>(span_diagnostic: &'b Handler, | ||
span: Span, | ||
codemap: &CodeMap) | ||
-> StringReader<'b> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this method is unused.
@@ -60,6 +62,14 @@ impl HasAttrs for Annotatable { | |||
} | |||
|
|||
impl Annotatable { | |||
pub fn span(&self) -> Span { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is unused.
@@ -111,6 +111,8 @@ impl<'a> Registry<'a> { | |||
} | |||
MultiDecorator(ext) => MultiDecorator(ext), | |||
MultiModifier(ext) => MultiModifier(ext), | |||
SyntaxExtension::ProcMacro(ext) => SyntaxExtension::ProcMacro(ext), | |||
SyntaxExtension::AttrProcMacro(ext) => SyntaxExtension::AttrProcMacro(ext), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These four lines could just be _ => extension,
(unless the explicitness is intentional, that is).
let new_items = changer.fold_impl_item(item); | ||
items.push_all(new_items); | ||
|
||
return Some(items); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this arm always returns, macros will only be able to expand into one impl item (same with trait items) -- not sure if this is intentional.
items.push_all(new_items); | ||
} | ||
Ok(None) => { | ||
return Some(items); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parse_item()
will return None
if the next token does not begin an item. For example, if a procedural macro returned +
in an item position, it would silently expand into nothing here.
ext::tt::macro_rules::ParserAnyMacro
solves this problem with ensure_complete_parse
.
In general, I think it would be a good idea to follow the semantics of ParserAnyMacro
more closely or even merge ParserAnyMacro
and TokResult
(moving the ChangeSpan
fold into expand.rs
), at least until we clean up the semantics of the parser.parse_*
methods used here.
|
||
fn make_expr(mut self: Box<Self>) -> Option<P<ast::Expr>> { | ||
match self.parser.parse_expr() { | ||
Ok(e) => Some(e), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like ensure_complete_parse
is also needed here so that valid[expression] } followed by arbitrary tokens
is not allowed (c.f. my comment about ParserAnyMacro
).
TokenStream::from_tts(marked_tts)); | ||
let parser = self.cx.new_parser_from_tts(&tok_result.to_tts()); | ||
let result = Box::new(TokResult { parser: parser, span: span }); | ||
kind.make_from(result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a good idea to change the spans here instead of in each method of TokResult
-- e.g. kind.make_from(result).fold_with(&mut ChangeSpan { span: span })
.
973eae2
to
d956d7e
Compare
@jseyfried changes made |
let result = Box::new(TokResult { parser: parser, span: attr.span }); | ||
|
||
match item { | ||
Annotatable::Item(_) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last simplification that I didn't notice earlier -- I believe this whole match can be replaced with:
kind.make_from(result).unwrap_or_else(|| {
let msg = format!("macro could not be expanded into {} position", kind.name());
self.cx.span_err(&msg);
kind.dummy(attr.span)
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah nice, I was hoping there was a way to get rid of this whole match
@nrc r=me with that last simplification. |
d956d7e
to
3863834
Compare
@bors: r=@jseyfried |
📌 Commit 3863834 has been approved by |
Adds a `ProcMacro` form of syntax extension This commit adds syntax extension forms matching the types for procedural macros 2.0 (RFC #1566), these still require the usual syntax extension boiler plate, but this is a first step towards proper implementation and should be useful for macros 1.1 stuff too. Supports both attribute-like and function-like macros. Note that RFC #1566 has not been accepted yet, but I think there is consensus that we want to head in vaguely that direction and so this PR will be useful in any case. It is also fairly easy to undo and does not break any existing programs. This is related to #35957 in that I hope it can be used in the implementation of macros 1.1, however, there is no direct overlap and is more of a complement than a competing proposal. There is still a fair bit of work to do before the two can be combined. r? @jseyfried cc @alexcrichton, @cgswords, @eddyb, @aturon
This commit adds syntax extension forms matching the types for procedural macros 2.0 (RFC #1566), these still require the usual syntax extension boiler plate, but this is a first step towards proper implementation and should be useful for macros 1.1 stuff too.
Supports both attribute-like and function-like macros.
Note that RFC #1566 has not been accepted yet, but I think there is consensus that we want to head in vaguely that direction and so this PR will be useful in any case. It is also fairly easy to undo and does not break any existing programs.
This is related to #35957 in that I hope it can be used in the implementation of macros 1.1, however, there is no direct overlap and is more of a complement than a competing proposal. There is still a fair bit of work to do before the two can be combined.
r? @jseyfried
cc @alexcrichton, @cgswords, @eddyb, @aturon