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

Document the proc_macro feature in the Unstable Book #41476

Merged
merged 1 commit into from
May 17, 2017

Conversation

abonander
Copy link
Contributor

@abonander abonander commented Apr 23, 2017

Discusses the proc_macro feature flag and the features it enables:

  • Implicit enable of extern_use_macros feature and how to import proc macros
  • Error handling in proc macros (using panic messages)
  • Function-like proc macros using #[proc_macro] and a usage example for creating and invoking
  • Attribute-like proc macros using #[proc_macro_attribute] and a usage example for creating and invoking

Rendered

}
```

`my_macro_user/main.rs`:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do I need to show adding the dependency on my_macro_crate to my_macro_user/Cargo.toml?

Copy link
Member

Choose a reason for hiding this comment

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

it might be nice

Copy link
Contributor Author

@abonander abonander Apr 24, 2017

Choose a reason for hiding this comment

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

Maybe at the top where I show adding the [lib] proc-macro = true section, so it doesn't get repetitive? Or do we want to make the dependency relationship clearer in this specific example (and presumably in its sibling below as well)?

@abonander
Copy link
Contributor Author

I realized I used "which" a lot and now it's bothering me.

@abonander
Copy link
Contributor Author

I would like to have doctesting work for the code examples but I don't think doctests can test code snippets as separate crates.

Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

this is a great start 👍


#### Error Reporting

Currently the only support for error reporting from procedural macros is through panics. Any panics in a procedural
Copy link
Member

Choose a reason for hiding this comment

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

Usually, it's better to write things in a "timeless" way, if that makes any sense. Basically, if we miss updating these docs when different error reporting stuff is added, does this still make sense? With this phrasing, it doesn't, but if you remove the first sentence, it does. We might be able to find a way to re-phrase the first sentence too...

Copy link
Contributor Author

@abonander abonander Apr 24, 2017

Choose a reason for hiding this comment

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

If that's the convention you want to enforce, I won't argue it.

But in my mind, saying "currently" tends to encourage the reader to be a little more forgiving with outdated information (context helps, being the "unstable" book and all). If that first bit was dropped so that it read "The only support for error reporting [...]" and then that fact changed later without the book updating, it would seem... disingenuous? Something like that.

I would also note that using "currently" tells the reader to expect this aspect to change/improve in the future, though I guess they've probably already acknowledged that by this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh, after some consideration, dropping the whole first sentence seems okay.

Copy link
Member

Choose a reason for hiding this comment

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

(context helps, being the "unstable" book and all)

I think this might be it, that is, it's true that for now, this stuff is not too stable, but we also don't want to do too much massaging once things actually need to go in stable places. It's possible I'm thinking too much of the future here.

I would also note that using "currently" tells the reader to expect this aspect to change/improve in the future,

Right, so the problem with this has been that in the future, changes get made, and then docs don't change, and then they're left with the incorrect impression that changes never happened. It's just tough.

Regardless, all of this is fairly minor and I'm happy to land it either way, to be honest. As you said, it is the unstable book 😄

Currently the only support for error reporting from procedural macros is through panics. Any panics in a procedural
macro implementation will be caught by the compiler and turned into an error message pointing to the problematic
invocation. Thus, it is important to make your panic messages as informative as possible: use `Option::expect()`
instead of `Option::unwrap()` and `Result::expect()` instead of `Result::unwrap()`, and inform the user of the error
Copy link
Member

Choose a reason for hiding this comment

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

no ()s on function names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a note, I like ()s because it distinguishes functions from public fields in this kind of context.

Copy link
Member

Choose a reason for hiding this comment

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

I also prefer it, unfortunately, the RFC process yielded different results 😄

* A `Display` impl which writes out the contained tokens. Of course, the blanket `ToString` impl applies
to this type as well.

### Function-like / Bang-style Procedural Macros
Copy link
Member

Choose a reason for hiding this comment

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

we do need to sort out these names at some point 😄

Copy link
Contributor Author

@abonander abonander Apr 24, 2017

Choose a reason for hiding this comment

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

I'm personally leaning towards "function-like" just because it sounds more formal. "bang" or "bang-style" just does not sound like it belongs in a technical document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, is mentioning both all that bad? It might help the reader if they later come across discussions where the two terms are used interchangeably.

Copy link
Member

Choose a reason for hiding this comment

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

nudging people toward proper use is a good idea, IMHO. there's no reason to have two names.

}
```

`my_macro_user/main.rs`:
Copy link
Member

Choose a reason for hiding this comment

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

it might be nice

**Note**: invocations of this kind of macro require a wrapping `[]`, `{}` or `()` like regular macros, but these do not
appear in the input, only the tokens between them. The tokens between the braces do not need to be valid Rust syntax.

`my_macro_crate/lib.rs`:
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should steal the formatting from the book:

Check out the *src/main.rs* file:

<span class="filename">Filename: src/main.rs</span>

```rust
fn main() {
    println!("Hello, world!");
}
```

https://github.com/raw/rust-lang/book/master/second-edition/src/ch02-00-guessing-game-tutorial.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At what point does it stop being "stealing" and becomes "following convention"? 😄

@carols10cents carols10cents added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 24, 2017
@abonander
Copy link
Contributor Author

@steveklabnik Requested revisions made. Now I'm mainly just waiting for an executive decision from either @nrc or @jseyfried on the preferred terminology as mentioned above.

@abonander
Copy link
Contributor Author

@steveklabnik Weird error in the test failure, it's trying to test a slice of an ignored code block...?

@Mark-Simulacrum
Copy link
Member

I suspect that the placing of the filename directly above the code block is causing the parser (hoedown? pulldown? I don't recall what we're at right now) to misinterpret the code block somehow. Perhaps a blank line would be a good idea.

@abonander
Copy link
Contributor Author

Worth a shot but the other code blocks work fine so it's weird. It's also starting to interpret the code block halfway through it as far as I can tell.

@steveklabnik
Copy link
Member

/cc @GuillaumeGomez for the weird errors

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Apr 25, 2017

We're back to hoedown so it should be the old (and long running) doc test system. Ping me if you can't figure it out once tests finished.

@abonander
Copy link
Contributor Author

Huh, looks like these latest test failures are truly unrelated.

@abonander
Copy link
Contributor Author

@steveklabnik Can you get bors to retry the build?

@GuillaumeGomez
Copy link
Member

@bors: retry

@alexcrichton
Copy link
Member

@steveklabnik it looks like some updates have happened since you last took a look I think, mind taking another look?

@alexcrichton alexcrichton added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2017
@abonander
Copy link
Contributor Author

Still waiting on answers to the questions in the OP as well. @jseyfried @nrc

@steveklabnik
Copy link
Member

This looks great to me; we still have the naming question to resolve though. Unsure exactly how best to do that...

@Mark-Simulacrum
Copy link
Member

@jseyfried @nrc Is there a chance you could take a look and provide some thoughts on the naming? Specifically, what do we call function-like / bang-macros?

note: Also pinged @nrc on IRC; jseyfried wasn't present.

@nrc
Copy link
Member

nrc commented May 15, 2017

Hey, sorry I missed the earlier pings - I was on parental leave and then had to rather forcefully clear my inbox.

I definitely prefer 'function-like' to 'bang' macros - I think it is highly likely that readers will not associate ! with "bang" and I think that ! is really an implementation detail of how macros are called, rather than something essential about them.


* Attribute procedural macros which can be applied to any item which built-in attributes can
Copy link
Member

Choose a reason for hiding this comment

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

I call these 'attribute-like' since they only look like attributes and are implemented differently - also for symmetry with 'function-like'

the syntax is valid but not in the context in which the macro is invoked, an error is thrown outside the macro.

* A `Display` impl which writes out the contained tokens. Of course, the blanket `ToString` impl applies
to this type as well.
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't describe these as the main APIs - they are kind of a stop-gap measure for custom derives. Macro authors should prefer using quote! to create tokens. (And of course more APIs will be arriving in the future).


### Function-like / Bang-style Procedural Macros

These are procedural macros that are invoked like `macro_rules!` macros. They are declared as public functions in crates
Copy link
Member

Choose a reason for hiding this comment

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

terminology nit - I prefer 'declarative macros' to 'macro_rules! macros' since the former is a descriptive name and the latter is a detail of how they are (currently) declared.

@abonander
Copy link
Contributor Author

@nrc nits fixed

@steveklabnik
Copy link
Member

r? @nrc

@rust-highfive rust-highfive assigned nrc and unassigned steveklabnik May 15, 2017
@nrc
Copy link
Member

nrc commented May 16, 2017

@bors: r+

@abonander abonander changed the title [WIP] Document the proc_macro feature in the Unstable Book Document the proc_macro feature in the Unstable Book May 16, 2017
@abonander
Copy link
Contributor Author

@nrc squashed and PR post cleaned up.

@Mark-Simulacrum
Copy link
Member

@bors r=nrc

@bors
Copy link
Contributor

bors commented May 16, 2017

📌 Commit e4208d5 has been approved by nrc

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 16, 2017
@bors
Copy link
Contributor

bors commented May 16, 2017

⌛ Testing commit e4208d5 with merge 88d44aa...

@bors
Copy link
Contributor

bors commented May 16, 2017

💔 Test failed - status-travis

@abonander
Copy link
Contributor Author

Can I get an @bors retry

@abonander
Copy link
Contributor Author

Ugh, it's hitting a wall with the broken-link-checker, on a relative link to the chapter for use_extern_macros. Is there any guidelines here?

@steveklabnik
Copy link
Member

Hm, I am not sure why it fails in that case, looks right to me. @frewsxcv any ideas?

@alexcrichton
Copy link
Member

It looks like CI is still failing?

[00:43:59] Linkcheck (x86_64-unknown-linux-gnu)
[00:43:59] unstable-book/print.html:1457: broken link - unstable-book/use-extern-macros.html
[00:43:59] unstable-book/language-features/proc-macro.html:87: broken link - unstable-book/use-extern-macros.html
[00:44:04] thread 'main' panicked at 'found some broken links', /checkout/src/tools/linkchecker/main.rs:49
[00:44:04] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:44:04] 
[00:44:04] 
[00:44:04] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/linkchecker" "/checkout/obj/build/x86_64-unknown-linux-gnu/doc"
[00:44:04] expected success, got: exit code: 101
[00:44:04] 
[00:44:04] 
[00:44:04] Build completed unsuccessfully in 0:08:50
[00:44:04] make: *** [check] Error 1
[00:44:04] Makefile:54: recipe for target 'check' failed

@frewsxcv
Copy link
Member

@abonander can you try rebasing this off master? there's a chance this could be fixed with #41992, not entirely sure though what's going on with the linkchecker

@arielb1 arielb1 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 16, 2017
@abonander
Copy link
Contributor Author

Rebased, let's see.

@abonander
Copy link
Contributor Author

I think the link is supposed to be to language-features/use-extern-macros.html, which doesn't really make sense because proc-macro.html is in the same directory, so just linking to the file should be sufficient. Instead, it's assuming a base url of <docs root>/unstable-book which is why the broken link is unstable-book/use-extern-macros.html when it should be unstable-book/language-features/use-extern-macros.html.

@frewsxcv
Copy link
Member

which doesn't really make sense because proc-macro.html is in the same directory, so just linking to the file should be sufficient

So it turns out that for subdirectories (like library-features/), mdbook will automatically include a <base href="../"> tag which is presumably why the library-features/ prefix is needed. Considering this is causing confusion, I think mdbook should stop using <base> so this is more obvious.

Anyways, tests seem to be passing now so:

@bors r=nrc

@bors
Copy link
Contributor

bors commented May 17, 2017

📌 Commit e616d12 has been approved by nrc

@bors
Copy link
Contributor

bors commented May 17, 2017

⌛ Testing commit e616d12 with merge 42e3732...

bors added a commit that referenced this pull request May 17, 2017
Document the `proc_macro` feature in the Unstable Book

Discusses the `proc_macro` feature flag and the features it enables:

* Implicit enable of `extern_use_macros` feature and how to import proc macros
* Error handling in proc macros (using panic messages)
* Function-like proc macros using `#[proc_macro]` and a usage example for creating and invoking
* Attribute-like proc macros using `#[proc_macro_attribute]` and a usage example for creating and invoking

[Rendered](https://github.com/abonander/rust/blob/book_proc_macro/src/doc/unstable-book/src/language-features/proc-macro.md)
@bors
Copy link
Contributor

bors commented May 17, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nrc
Pushing 42e3732 to master...

@bors bors merged commit e616d12 into rust-lang:master May 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants