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

Re-exported proc-macros are documented incorrectly #68690

Closed
SergioBenitez opened this issue Jan 31, 2020 · 4 comments · Fixed by #68814
Closed

Re-exported proc-macros are documented incorrectly #68690

SergioBenitez opened this issue Jan 31, 2020 · 4 comments · Fixed by #68814
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@SergioBenitez
Copy link
Contributor

SergioBenitez commented Jan 31, 2020

When multiple proc-macros are re-exported, their documentation is permuted in such a way that the docs for one macro become the docs for another. For a specific case, consider Rocket's proc-macros, defined in earnest in the rocket_codegen crate. The documentation for the rocket_codegen crate, which is rendered correctly and as expected, looks like:

|-----------------|----------------------------------------------------------|
| async_test      |                                                          |
| catchers        | Generates a Vec of Catchers from a set of catcher paths. |
| routes          | Generates a Vec of Routes from a set of route paths.     |
| uri             | Type safe generation of route URIs.                      |
| catch           | Attribute to generate a Catcher and associated metadata. |
| delete          | Attribute to generate a Route and associated metadata.   |
| get             | Attribute to generate a Route and associated metadata.   |
| head            | Attribute to generate a Route and associated metadata.   |
| options         | Attribute to generate a Route and associated metadata.   |
| patch           | Attribute to generate a Route and associated metadata.   |
| post            | Attribute to generate a Route and associated metadata.   |
| put             | Attribute to generate a Route and associated metadata.   |
| route           | Attribute to generate a Route and associated metadata.   |
| FromForm        | Derive for the FromForm trait.                           |
| FromFormValue   | Derive for the FromFormValue trait.                      |
| Responder       | Derive for the Responder trait.                          |
| UriDisplayPath  | Derive for the UriDisplay<Path> trait.                   |
| UriDisplayQuery | Derive for the UriDisplay<Query> trait.                  |
|-----------------|----------------------------------------------------------|

When these proc-macros are re-exported from rocket via pub use rocket_codegen::*, their documentation in rocket is rendered as:

|---------------------|----------------------------------------------------------|
| async_test          | Type safe generation of route URIs.                      |
| catchers            | Attribute to generate a Route and associated metadata.   |
| rocket_internal_uri | Attribute to generate a Route and associated metadata.   |
| routes              | Attribute to generate a Route and associated metadata.   |
| uri                 | Attribute to generate a Route and associated metadata.   |
| catch               | Generates a Vec of Catchers from a set of catcher paths. |
| delete              | Derive for the Responder trait.                          |
| get                 |                                                          |
| head                | Derive for the UriDisplay<Query> trait.                  |
| options             | Generates a Vec of Routes from a set of route paths.     |
| patch               | Derive for the UriDisplay<Path> trait.                   |
| post                | Derive for the FromForm trait.                           |
| put                 | Derive for the FromFormValue trait.                      |
| route               | Attribute to generate a Catcher and associated metadata. |
| FromForm            | Attribute to generate a Route and associated metadata.   |
| Responder           | Attribute to generate a Route and associated metadata.   |
| UriDisplayPath      | Attribute to generate a Route and associated metadata.   |
| UriDisplayQuery     | Attribute to generate a Route and associated metadata.   |
|---------------------|----------------------------------------------------------|

Clicking through an item leads to the documentation alluded to by the snippet which is similarly incorrect.

Note that rocket_internal_uri is #[doc(hidden)] in rocket_codegen; it erroneously appears in rocket. Also note that FromFormValue is missing in rocket. The expected rendering, of course, is one identical to the rendering for rocket_codegen.

@petrochenkov
Copy link
Contributor

I suspect fixing this will help with the standard library documentation as well - #63268.

@Aaron1011
Copy link
Member

@rustbot claim

@jonas-schievink jonas-schievink added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 31, 2020
@Aaron1011
Copy link
Member

It looks like these proc-macros are themselves being generated by a macro_rules! macro:

https://github.com/SergioBenitez/Rocket/blob/d0bfd8a3bb9a9b1b2e3848356164c7bf312bf1b5/core/codegen/src/lib.rs#L322-L325

This is the kind of thing that I was previously concerned about: #64528 (comment). We're currently relying on having the proc-macro-harness generation order match the HIR iteration order, so that we can properly match up proc-macros to their associated data during deserialization.

Given that this is the second time this kind of ordering mismatch has occured (the first time was #64251), I think it would be a mistake to try to patch it up again.

I think the best solution is to revive my commit 9b4cf01 (from PR #64528), which explicitly tracks the correspondence between the proc-macro-harness items and the serialized metadata.

cc @alexcrichton @petrochenkov (you were both reviewers on PR #64528)

@petrochenkov
Copy link
Contributor

petrochenkov commented Feb 4, 2020

I'll look at the proc macro serialization order issue this weekend.
(Proc macro harness runs after macro expansion, so it's not clear to me how proc macros being macro-expanded would affect anything.)

bors added a commit that referenced this issue Feb 16, 2020
…nkov

Record proc macro harness order for use during metadata deserialization

Fixes #68690

When we generate the proc macro harness, we now explicitly recorder the
order in which we generate entries. We then use this ordering data to
deserialize the correct proc-macro-data from the crate metadata.
@bors bors closed this as completed in 51a16e5 Feb 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants