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.

Sign up for GitHub

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

Jump to bottom

Remove doctree::Macro and distinguish between macro_rules! and pub macro #79455

Merged
merged 7 commits into from
Nov 29, 2020
Merged

Remove doctree::Macro and distinguish between macro_rules! and pub macro #79455

merged 7 commits into from
Nov 29, 2020

Conversation

CraftSpider
Copy link
Contributor

@CraftSpider CraftSpider commented Nov 26, 2020 •
edited
Loading

This is a part of #78082, removing doctree::Macro. Uses the changes in #79372

Fixes #76761

@rust-highfive
Copy link
Collaborator

r? @ollie27

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 27, 2020

r? @jyn514

@rust-highfive rust-highfive assigned jyn514 and unassigned ollie27 Nov 27, 2020
@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 27, 2020
jyn514
jyn514 reviewed Nov 27, 2020
View reviewed changes
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2020
jyn514
jyn514 reviewed Nov 27, 2020
View reviewed changes
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
jyn514
jyn514 reviewed Nov 27, 2020
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please add some tests for the new behavior around pub macro. They'd go in src/test/rustdoc, there are instructions in https://rustc-dev-guide.rust-lang.org/rustdoc-internals.html#dotting-is-and-crossing-ts.

@jyn514 jyn514 changed the title Remove doctree::Macro Remove doctree::Macro and distinguish between macro_rules! and pub macro Nov 27, 2020
jyn514
jyn514 reviewed Nov 27, 2020
View reviewed changes
src/test/rustdoc/macros_2.rs Outdated Show resolved Hide resolved
src/test/rustdoc/macros_2.rs Outdated Show resolved Hide resolved
GuillaumeGomez
GuillaumeGomez approved these changes Nov 27, 2020
View reviewed changes
Copy link
Member

@GuillaumeGomez GuillaumeGomez left a comment

Choose a reason for hiding this comment

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

Nice!

@jyn514 jyn514 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 Nov 27, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 27, 2020

This looks pretty good to me. Once #79372 is merged (hopefully today), can you rebase over those changes and squash the "Appease tidy" commits at the same time? I want to look at it once more after that but I don't expect it to need many changes.

@jyn514 jyn514 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. 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-review Status: Awaiting review from the assignee but also interested parties. S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Nov 27, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 27, 2020

#79372 was merged.

@CraftSpider
Copy link
Contributor Author

Rebased and squashed, as requested

@CraftSpider
Update src/test/rustdoc/decl_macro_priv.rs
a61c09a 
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
@jyn514
Copy link
Member

jyn514 commented Nov 28, 2020

@bors r+

🎉 🎉

@bors
Copy link
Contributor

bors commented Nov 28, 2020

📌 Commit a61c09a has been approved by jyn514

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 28, 2020
@bors
Copy link
Contributor

bors commented Nov 28, 2020

⌛ Testing commit a61c09a with merge 514bb94c0c1a94f8dda90b0bf65068d57f43232c...

bjorn3
bjorn3 reviewed Nov 28, 2020
View reviewed changes
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
@jyn514
Copy link
Member

jyn514 commented Nov 28, 2020

@bors r-

@CraftSpider do you mind adding a test for multiple matches in a decl macro (and fixing it if it breaks)? There should be some examples of multiple matches in src/test/rustdoc/macro.rs.

@bors bors 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 Nov 28, 2020
@bors
Copy link
Contributor

bors commented Nov 28, 2020

☀️ Try build successful - checks-actions
Build commit: 514bb94c0c1a94f8dda90b0bf65068d57f43232c (514bb94c0c1a94f8dda90b0bf65068d57f43232c)

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Nov 29, 2020

📌 Commit d23b57c has been approved by jyn514

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 29, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

Thanks for sticking with this!

@CraftSpider
Copy link
Contributor Author

CraftSpider commented Nov 29, 2020 •
edited
Loading

No problem! I'd rather spend more time to make sure everything is right, than just merge something half-functional. Thank you for your continued assistance.

@bors
Copy link
Contributor

bors commented Nov 29, 2020

⌛ Testing commit d23b57c with merge 3cbb56f...

@bors
Copy link
Contributor

bors commented Nov 29, 2020

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 3cbb56f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 29, 2020
@bors bors merged commit 3cbb56f into rust-lang:master Nov 29, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 29, 2020
@bors bors mentioned this pull request Nov 29, 2020
Closed
@RalfJung
Copy link
Member

Nice, thanks for the fix! Will this make it easier to fix #74355?

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

@RalfJung in the sense that the code is easier to work with, yes, but it may actually make it harder in the short term because the approach in #77862 no longer works; current_mod no longer exists and I don't know a way to move macros around in the AST. I continue to think #77862 is solving a problem in rustdoc that should be fixed in rustc directly.

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

Actually, that might be a little strong - I think you could still use the same approach to chose a different module to attach the MacroDef to. So the same hack would work, but it would still be a hack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 approved these changes

@bjorn3 bjorn3 bjorn3 left review comments

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@jyn514 jyn514

Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. 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
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.

Rustdoc displays macros as macro_rules!s
9 participants
@CraftSpider @rust-highfive @jyn514 @bors @RalfJung @GuillaumeGomez @bjorn3 @ollie27 @rustbot