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

Add Documentation for Custom Attributes and Error Reporting in Procedural Macros #39845

Merged
merged 4 commits into from Feb 25, 2017

Conversation

Projects
None yet
7 participants
@JDemler
Copy link
Contributor

commented Feb 15, 2017

This fixes #39821 .

I'm not sure if the process of how to access custom attributes should be documented as well.
But I feel, that this should rather be documented in syn

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Feb 15, 2017

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@steveklabnik
Copy link
Member

left a comment

I have some small formatting comments, but looks good to me!

@@ -209,5 +209,73 @@ Ok so now, let's compile `hello-world`. Executing `cargo run` now yields:
Hello, World! My name is FrenchToast
Hello, World! My name is Waffles
```
## Custom Attributes
In some cases it might make sense to allow users some kind of configuration.

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik Feb 15, 2017

Member

could you put a newline above this line, please?

## Raising Errors
Let's assume that we do not want to accept `Enums` as input to our custom derive method.

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik Feb 15, 2017

Member

can you add a newline above this line?

```
Multiple attributes can be specified that way.

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik Feb 15, 2017

Member

could you delete one of these newlines?

This comment has been minimized.

Copy link
@JDemler

JDemler Feb 16, 2017

Author Contributor

Do you mean the newline before the heading or after the code block?

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik Feb 16, 2017

Member

there should only be one newline, like this:

Multiple attributes can be specified that way.

## Raising Errors

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik Feb 16, 2017

Member

(that is, before the heading)

This comment has been minimized.

Copy link
@JDemler

JDemler Feb 16, 2017

Author Contributor

So the fixed whitespace issues-commit fixed this issue, correct?

@xen0n
Copy link
Contributor

left a comment

Some minor nits, but the snippets are LGTM.

Let's assume that we do not want to accept `Enums` as input to our custom derive method.
This condition can be easily checked with the help of `syn`.
But how to we tell the user, that we do not accept `Enums`.

This comment has been minimized.

Copy link
@xen0n

xen0n Feb 17, 2017

Contributor
  • how to -> how do
  • trailing . -> ?
This condition can be easily checked with the help of `syn`.
But how to we tell the user, that we do not accept `Enums`.
The idiomatic was to report errors in procedural macros is to panic:

This comment has been minimized.

Copy link
@xen0n

xen0n Feb 17, 2017

Contributor

was -> way

error: The attribute `HelloWorldName` is currently unknown to the compiler and may have meaning added to it in the future (see issue #29642)
```
The compiler needs to know that we handle this attribute and to not respond with an error.

This comment has been minimized.

Copy link
@xen0n

xen0n Feb 17, 2017

Contributor

we handle -> we're handling

@@ -210,4 +210,74 @@ Hello, World! My name is FrenchToast
Hello, World! My name is Waffles
```

We've done it!

This comment has been minimized.

Copy link
@xen0n

xen0n Feb 17, 2017

Contributor

Removing this for the next section seems to make the previous section truncated, IMO removal is not needed.

## Custom Attributes

In some cases it might make sense to allow users some kind of configuration.
For our example the user might want to overwrite the name that is printed in the `hello_world()` method.

This comment has been minimized.

Copy link
@xen0n

xen0n Feb 17, 2017

Contributor

For example, ... is enough for introducing the example.

```
The compiler needs to know that we handle this attribute and to not respond with an error.
This is done in the `hello-world-derive`-crate by adding `attributes` to the `proc_macro_derive` attribute:

This comment has been minimized.

Copy link
@xen0n

xen0n Feb 17, 2017

Contributor

The hyphen before crate should be replaced by a space.

## Raising Errors
Let's assume that we do not want to accept `Enums` as input to our custom derive method.

This comment has been minimized.

Copy link
@xen0n

xen0n Feb 17, 2017

Contributor

The use of Enums is over-emphasis, IMO just enums or enumerations without the inline code block is enough.

@JDemler

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2017

Thanks for the nits. They should be resolved now.

@steveklabnik

This comment has been minimized.

Copy link
Member

commented Feb 24, 2017

@bors: r+ rollup

thank you!

@bors

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2017

📌 Commit 198208b has been approved by steveklabnik

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 24, 2017

Rollup merge of rust-lang#39845 - JDemler:master, r=steveklabnik
Add Documentation for Custom Attributes and Error Reporting in Procedural Macros

This fixes rust-lang#39821 .

I'm not sure if the process of how to access custom attributes should be documented as well.
But I feel, that this should rather be documented in `syn`

bors added a commit that referenced this pull request Feb 24, 2017

Auto merge of #40072 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 11 pull requests

- Successful merges: #39777, #39815, #39845, #39886, #39940, #40010, #40030, #40048, #40050, #40052, #40071
- Failed merges:

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 24, 2017

Rollup merge of rust-lang#39845 - JDemler:master, r=steveklabnik
Add Documentation for Custom Attributes and Error Reporting in Procedural Macros

This fixes rust-lang#39821 .

I'm not sure if the process of how to access custom attributes should be documented as well.
But I feel, that this should rather be documented in `syn`

bors added a commit that referenced this pull request Feb 24, 2017

Auto merge of #40080 - frewsxcv:rollup, r=frewsxcv
Rollup of 17 pull requests

- Successful merges: #39777, #39815, #39845, #39886, #39892, #39903, #39905, #39914, #39927, #39940, #40010, #40030, #40048, #40050, #40052, #40060, #40071
- Failed merges:

bors added a commit that referenced this pull request Feb 24, 2017

Auto merge of #40072 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 11 pull requests

- Successful merges: #39777, #39815, #39845, #39886, #39940, #40010, #40030, #40048, #40050, #40052, #40071
- Failed merges:

bors added a commit that referenced this pull request Feb 24, 2017

Auto merge of #40080 - frewsxcv:rollup, r=frewsxcv
Rollup of 17 pull requests

- Successful merges: #39777, #39815, #39845, #39886, #39892, #39903, #39905, #39914, #39927, #39940, #40010, #40030, #40048, #40050, #40052, #40060, #40071
- Failed merges:

bors added a commit that referenced this pull request Feb 24, 2017

Auto merge of #40080 - frewsxcv:rollup, r=frewsxcv
Rollup of 17 pull requests

- Successful merges: #39777, #39815, #39845, #39886, #39892, #39903, #39905, #39914, #39927, #39940, #40010, #40030, #40048, #40050, #40052, #40060, #40071
- Failed merges:

bors added a commit that referenced this pull request Feb 25, 2017

Auto merge of #40072 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 11 pull requests

- Successful merges: #39777, #39815, #39845, #39886, #39940, #40010, #40030, #40048, #40050, #40052, #40071
- Failed merges:

@bors bors merged commit 198208b into rust-lang:master Feb 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gilescope

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2018

Hoping feedback is always welcome: A paragraph pointing to syn with a little example of how to access HelloWorldName = "the best Pancakes" would be really really appreciated on that page. Its not clear from the TokenStream input type - I looked here https://doc.rust-lang.org/proc_macro/struct.TokenStream.html and find no mention of attributes.

@trsh

This comment has been minimized.

Copy link

commented Jul 24, 2018

It's still not written how to read HelloWorldName in macro fn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.