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

Implement function-like procedural macros ( #[proc_macro]) #40129

Merged
merged 1 commit into from
Mar 2, 2017

Conversation

abonander
Copy link
Contributor

@abonander abonander commented Feb 27, 2017

Adds the #[proc_macro] attribute, which expects bare functions of the kind fn(TokenStream) -> TokenStream, which can be invoked like my_macro!().

cc rust-lang/rfcs#1913, #38356

r? @jseyfried
cc @nrc

@abonander
Copy link
Contributor Author

Unanswered question: did we decide whether or not to include the outer delimiters from the invocation in the input? i.e. rewrite!("Hello, world!") would see "( \"Hello, world!\" )" I forget.

function_name: Ident,
span: Span,
}

struct CollectProcMacros<'a> {
derives: Vec<ProcMacroDerive>,
attr_macros: Vec<AttrProcMacro>,
attr_macros: Vec<ProcMacroDef>,
bang_macros: Vec<ProcMacroDef>,
Copy link
Contributor Author

@abonander abonander Feb 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on the fence about reusing the typedef. I didn't see much point to having a unique type with the same signature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good as is.

pub fn rewrite(input: TokenStream) -> TokenStream {
let input = input.to_string();

assert_eq!(input, r#""Hello, world!""#);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might fail the test because the wrapping delimiters might get included. I forget what the executive decision was on whether or not to include those.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, works as-is.

@abonander abonander changed the title [WIP?] Implement function-like procedural macros ( #[proc_macro]) Implement function-like procedural macros ( #[proc_macro]) Feb 27, 2017
@abonander
Copy link
Contributor Author

abonander commented Feb 27, 2017

<jseyfried> DroidLogician: We decided not to include the wrapping () / [] / {} for now

Looks like I didn't have to do any extra work to support that as-is, very nice.

@@ -232,7 +257,8 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
let mut found_attr: Option<&'a ast::Attribute> = None;

for attr in &item.attrs {
if attr.check_name("proc_macro_derive") || attr.check_name("proc_macro_attribute") {
if attr.check_name("proc_macro_derive") || attr.check_name("proc_macro_attribute") ||
attr.check_name("proc_macro") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting suggestion welcome here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be indented three spaces (to align with first condition) or eight spaces (to distinguish from body).

Copy link
Contributor Author

@abonander abonander Feb 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, I don't really like how either of those look so it's your call. I would almost prefer a .check_names() method which can take a slice or iterator or something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I'd write:

if PROC_MACRO_KINDS.iter().any(|kind| attr.check_name(kind)) { ... }

where

const PROC_MACRO_KINDS: [&'static str, 3] =
    ["proc_macro", "proc_macro_attribute", "proc_macro_derive"];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I might use the static-in-const feature to make that a little shorter too.

@@ -125,6 +125,10 @@ pub mod __internal {
fn register_attr_proc_macro(&mut self,
name: &str,
expand: fn(TokenStream, TokenStream) -> TokenStream);

fn register_bang_proc_macro(&mut self,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bikeshed: "fnlike" instead of "bang"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer Bang inside the compiler to avoid overloading "function". Also, we might want to use "proc macro function" for the underlying function item annotated with #[proc_macro*].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we might want to use "proc macro function" for the underlying function item annotated with #[proc_macro*].

Clarify?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For #[proc_macro_derive(A)] fn f(..) { .. }, we would say A is a [derive] proc macro and f is the [derive] proc macro function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. That doesn't affect anything in this PR, though, right? That's purely wording for documentation and stuff?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's just an argument for "bang proc macro" instead of "function / fnlike procedural macro".

@abonander abonander force-pushed the proc_macro_bang branch 3 times, most recently from c7035cb to e7f258e Compare February 28, 2017 01:40
("proc_macro", Normal, Gated(Stability::Unstable,
"proc_macro",
"function-like proc macros are currently unstable",
cfg_fn!(proc_macro))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: These should be indented to the same column as Stability::Unstable,.

}
if let Some(s) = e.downcast_ref::<&'static str>() {
err.help(&format!("message: {}", s));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to factor this out to avoid duplication across macro kinds (not needed for this PR though).

@@ -27,21 +27,25 @@ use syntax_pos::{Span, DUMMY_SP};

use deriving;

const PROC_MACRO_KINDS: [&'static str; 3] = ["proc_macro_derive", "proc_macro_attribute",
"proc_macro"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would break after the = or give each macro kind its own line.

function_name: Ident,
span: Span,
}

struct CollectProcMacros<'a> {
derives: Vec<ProcMacroDerive>,
attr_macros: Vec<AttrProcMacro>,
attr_macros: Vec<ProcMacroDef>,
bang_macros: Vec<ProcMacroDef>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good as is.

@jseyfried
Copy link
Contributor

@abonander Looks great!

What happens when we #[macro_use] a bang procedural macro? I think this should behave the same as #[macro_use]ing an attribute procedural macro (i.e. error when the #[macro_use] import is used and suggest a use import instead).

cc @keeperofdakeys

@abonander
Copy link
Contributor Author

I'll have to test that, I think we're only checking that in the attribute proc macro codepath.

@keeperofdakeys
Copy link
Contributor

keeperofdakeys commented Feb 28, 2017

Only supporting use seems like the best thing.

@abonander Nice. Maybe you'd like to add a misspelled call for a proc macro to this test, so we can test suggestions? They should be working, but it would be good to make sure.

@abonander
Copy link
Contributor Author

Yeah #[macro_use] is currently importing bang proc macros without a problem so I'll fix that real quick. Don't give me any hints, I want to see if I can figure it out myself since I've already been digging around in resolve.


// aux-build:bang-macro.rs

#![allow(warnings)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't. I copy-pasted this from another file to get the copyright and stuff and I must have forgotten to remove that.

@keeperofdakeys
Copy link
Contributor

@abonander Awesome, thanks.

@abonander
Copy link
Contributor Author

@jseyfried Last nits fixed and rebased.

fn collect_bang_proc_macro(&mut self, item: &'a ast::Item, attr: &'a ast::Attribute) {
if let Some(_) = attr.meta_item_list() {
self.handler.span_err(attr.span, "`#[proc_macro]` attribute
cannot contain any meta items");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"meta item" is compiler jargon -- I'd say "does not accept arguments" (also applies to proc_macro_attribute).

fn main() {
bang_proc_macro!(println!("Hello, world!"));
//~^ ERROR: procedural macros cannot be imported with `#[macro_use]`
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: trailing newline

@jseyfried
Copy link
Contributor

r=me modulo comments
@bors delegate=abonander

@bors
Copy link
Contributor

bors commented Mar 1, 2017

✌️ @abonander can now approve this pull request

@abonander
Copy link
Contributor Author

@bors r=jseyfried

@bors
Copy link
Contributor

bors commented Mar 1, 2017

📌 Commit 2fcbb48 has been approved by jseyfried

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 2, 2017
Implement function-like procedural macros ( `#[proc_macro]`)

Adds the `#[proc_macro]` attribute, which expects bare functions of the kind `fn(TokenStream) -> TokenStream`, which can be invoked like `my_macro!()`.

cc rust-lang/rfcs#1913, rust-lang#38356

r? @jseyfried
cc @nrc
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 2, 2017
Implement function-like procedural macros ( `#[proc_macro]`)

Adds the `#[proc_macro]` attribute, which expects bare functions of the kind `fn(TokenStream) -> TokenStream`, which can be invoked like `my_macro!()`.

cc rust-lang/rfcs#1913, rust-lang#38356

r? @jseyfried
cc @nrc
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 2, 2017
Implement function-like procedural macros ( `#[proc_macro]`)

Adds the `#[proc_macro]` attribute, which expects bare functions of the kind `fn(TokenStream) -> TokenStream`, which can be invoked like `my_macro!()`.

cc rust-lang/rfcs#1913, rust-lang#38356

r? @jseyfried
cc @nrc
bors added a commit that referenced this pull request Mar 2, 2017
Rollup of 7 pull requests

- Successful merges: #39832, #40104, #40110, #40117, #40129, #40139, #40166
- Failed merges:
@bors bors merged commit 2fcbb48 into rust-lang:master Mar 2, 2017
chpio added a commit to vtree-rs/vtree that referenced this pull request Mar 9, 2017
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.

4 participants