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

rustdoc: give proc-macros their own pages #54577

Merged
merged 8 commits into from
Sep 29, 2018

Conversation

QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Sep 25, 2018

related to #49553 but i don't think it'll fix it

Currently, rustdoc doesn't expose proc-macros all that well. In the source crate, only their definition function is exposed, but when re-exported, they're treated as a macro! This is an awkward situation in all accounts. This PR checks functions to see whether they have any of #[proc_macro], #[proc_macro_attribute], or #[proc_macro_derive], and exposes them as macros instead. In addition, attributes and derives are exposed differently than other macros, getting their own item-type, CSS class, and module heading.

image

Function-like proc-macros are lumped in with macro_rules! macros, but they get a different declaration block (i'm open to tweaking this, it's just what i thought of given how function-proc-macros operate):

image

Proc-macro attributes and derives get their own pages, with a representative declaration block. Derive macros also show off their helper attributes:

image

image

There's one wrinkle which this PR doesn't address, which is why i didn't mark this as fixing the linked issue. Currently, proc-macros don't expose their attributes or source span across crates, so while rustdoc knows they exist, that's about all the information it gets. This leads to an "inlined" macro that has absolutely no docs on it, and no [src] link to show you where it was declared.

The way i got around it was to keep proc-macro re-export disabled, since we do get enough information across crates to properly link to the source page:

image

Until we can get a proc-macro's docs (and ideally also its source span) across crates, i believe this is the best way forward.

Proc-macros don't emit their attributes or source spans across crates.
This means that rustdoc can't actually see the docs of a proc-macro if
it wasn't defined in the active crate, and attempting to inline it
creates an empty page with no docs or source link. In lieu of attempting
to fix that immediately, this commit forces proc-macro re-exports to
never inline, which at least creates usable links to complete
documentation.
{
if m.kind == MacroKind::Bang {
write!(w, "<pre class='rust macro'>")?;
write!(w, "{}!() {{ /* proc-macro */ }}", it.name.as_ref().unwrap())?;
Copy link
Member

Choose a reason for hiding this comment

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

No unwrap please (expect!).

@GuillaumeGomez
Copy link
Member

That's quite the nice add, thanks a lot!

However, the UI looks a bit... empty? At least it tells readers that it exists but not much more...

@Centril
Copy link
Contributor

Centril commented Sep 25, 2018

Function-like proc-macros are lumped in with macro_rules! macros

This is great; don't change it :)
I think it makes perfect sense for all bang macros to be lumped together.

@QuietMisdreavus
Copy link
Member Author

However, the UI looks a bit... empty? At least it tells readers that it exists but not much more...

@GuillaumeGomez Yeah, there's not much to say generically about proc-macros in general, other than their name and form factor. (I mean, it's technically possible for a macro author to create some kind of BASIC dialect in one of these proc-macros...) It's up to the author to fill in how it's supposed to be used. I can change up the attribute and derive pages to give them a "declaration" block, though. That should help guide those pages some. (It's possible to gather a custom derive's helper attribute, so i should definitely figure out how to display that one...)

I think it makes perfect sense for all bang macros to be lumped together.

@Centril Yeah, at some point i realized there wasn't enough reason to separate macro_rules! macros and bang-proc-macros, so they're put under the same heading.

@XAMPPRocky XAMPPRocky added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 26, 2018
@QuietMisdreavus
Copy link
Member Author

I've pushed up a couple new commits that add more information to attribute and derive pages. New images were added to the PR description to show off how they look now.

@GuillaumeGomez
Copy link
Member

Seems good to me. A bit weird but as long as it's working... Please also add the proc-macros to the "all items" page (look for AllTypes).

@QuietMisdreavus
Copy link
Member Author

Good catch! I've added sections to the "All items" page for attributes and derives, since function proc-macros already appear in the existing "Macros" section. I also found another issue where the link to the "all items" page was only being printed if the --crate-version argument was sent, so i went ahead and fixed that here.

@GuillaumeGomez
Copy link
Member

Thanks for the fix. Please also add a test to check if they are correctly listed in the all items page (just add it to the all items test). Once done and CI green, r=me.

@QuietMisdreavus
Copy link
Member Author

I added it to the proc-macro links test, so that i wouldn't have to mess with the crate type of the all-items test.

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Sep 27, 2018

📌 Commit d37f369 has been approved by GuillaumeGomez

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 27, 2018
@bors
Copy link
Contributor

bors commented Sep 28, 2018

⌛ Testing commit d37f369 with merge b26a7513c941147d087f6cae47f42649e7d49848...

@bors
Copy link
Contributor

bors commented Sep 28, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 28, 2018
@rust-highfive
Copy link
Collaborator

The job armhf-gnu of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:system_info

Network availability confirmed.
Setting APT mirror in /etc/apt/sources.list: http://us-central1.gce.archive.ubuntu.com/ubuntu/
travis_fold:start:update_heroku
Updating Heroku
$ heroku version
heroku/7.16.0 linux-x64 node-v10.10.0
travis_fold:end:update_heroku
Installing APT Packages
travis_time:start:0a06fdd8
$ travis_apt_get_update
travis_time:end:0a06fdd8:start=1538159523378727802,finish=1538159528333874680,duration=4955146878
---
travis_time:end:354e0a50:start=1538160937446623206,finish=1538160937451140669,duration=4517463
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0e80f7ce
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0dae1878
travis_time:start:0dae1878
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:2443d70d
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@alexcrichton
Copy link
Member

@bors: retry

@bors bors 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 Sep 28, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Sep 29, 2018
…uillaumeGomez

rustdoc: give proc-macros their own pages

related to rust-lang#49553 but i don't think it'll fix it

Currently, rustdoc doesn't expose proc-macros all that well. In the source crate, only their definition function is exposed, but when re-exported, they're treated as a macro! This is an awkward situation in all accounts. This PR checks functions to see whether they have any of `#[proc_macro]`, `#[proc_macro_attribute]`, or `#[proc_macro_derive]`, and exposes them as macros instead. In addition, attributes and derives are exposed differently than other macros, getting their own item-type, CSS class, and module heading.

![image](https://user-images.githubusercontent.com/5217170/46044803-6df8da00-c0e1-11e8-8c3b-25d2c3beb55c.png)

Function-like proc-macros are lumped in with `macro_rules!` macros, but they get a different declaration block (i'm open to tweaking this, it's just what i thought of given how function-proc-macros operate):

![image](https://user-images.githubusercontent.com/5217170/46044828-84069a80-c0e1-11e8-9cc4-127e5477c395.png)

Proc-macro attributes and derives get their own pages, with a representative declaration block. Derive macros also show off their helper attributes:

![image](https://user-images.githubusercontent.com/5217170/46094583-ef9f4500-c17f-11e8-8f71-fa0a7895c9f6.png)

![image](https://user-images.githubusercontent.com/5217170/46101529-cab3cd80-c191-11e8-857a-946897750da1.png)

There's one wrinkle which this PR doesn't address, which is why i didn't mark this as fixing the linked issue. Currently, proc-macros don't expose their attributes or source span across crates, so while rustdoc knows they exist, that's about all the information it gets. This leads to an "inlined" macro that has absolutely no docs on it, and no `[src]` link to show you where it was declared.

The way i got around it was to keep proc-macro re-export disabled, since we do get enough information across crates to properly link to the source page:

![image](https://user-images.githubusercontent.com/5217170/46045074-2cb4fa00-c0e2-11e8-81bc-33a8205fbd03.png)

Until we can get a proc-macro's docs (and ideally also its source span) across crates, i believe this is the best way forward.
bors added a commit that referenced this pull request Sep 29, 2018
Rollup of 8 pull requests

Successful merges:

 - #54564 (Add 1.29.1 release notes)
 - #54567 (Include path in stamp hash for debuginfo tests)
 - #54577 (rustdoc: give proc-macros their own pages)
 - #54590 (std: Don't let `rust_panic` get inlined)
 - #54598 (Remove useless lifetimes from `Pin` `impl`s.)
 - #54604 (Added help message for `self_in_typedefs` feature gate)
 - #54635 (Improve docs for std::io::Seek)
 - #54645 (Compute Android gdb version in compiletest)
@bors
Copy link
Contributor

bors commented Sep 29, 2018

⌛ Testing commit d37f369 with merge 9653f79...

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-enhancement Category: An issue proposing an enhancement or a PR with one. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants