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

Cleanup more of rustdoc #79372

Merged
merged 4 commits into from
Nov 27, 2020
Merged

Cleanup more of rustdoc #79372

merged 4 commits into from
Nov 27, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Nov 24, 2020

  • Use Item::from_def_id for StructField
  • Use from_def_id_and_parts for primitives and keywords
  • Take String instead of Symbol in from_def_id - this avoids having to intern then immediately stringify the existing string.
  • Remove unused get_stability and get_deprecation
  • Remove unused attrs field from primitives
  • Remove unused attrs field from keywords

This will probably conflict with #79335 and I would prefer for that PR to land first - I'm anxious for #77467 to land :)

Makes #76998 easier to add.

r? @GuillaumeGomez

@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 24, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 24, 2020
@bors

This comment has been minimized.

@@ -124,7 +124,7 @@ crate fn try_inline(
let attrs = merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone);

cx.renderinfo.borrow_mut().inlined.insert(did);
let what_rustc_thinks = clean::Item::from_def_id_and_parts(did, Some(name), kind, cx);
let what_rustc_thinks = clean::Item::from_def_id_and_parts(did, Some(name.clean(cx)), kind, cx);
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't look deep but I wonder why do we need to have this .clean(cx) so many times unlike before?

Copy link
Member Author

Choose a reason for hiding this comment

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

Does the explanation in the commit message help? bbded0b

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, now I understand. I didn't know it is related to interning.

- Take `String` instead of `Symbol` - this avoids having to intern then
  immediately stringify the existing string.
- Remove unused `get_stability` and `get_deprecation`
- Remove unused `attrs` field from `primitives`
@jyn514
Copy link
Member Author

jyn514 commented Nov 26, 2020

This is ready for review.

@GuillaumeGomez
Copy link
Member

Awesome as usual, thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 26, 2020

📌 Commit 09a3bc1 has been approved by GuillaumeGomez

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

bors commented Nov 27, 2020

⌛ Testing commit 09a3bc1 with merge 77f09bb854d585c24f0e2bc98e35d7db33a6a293...

@bors
Copy link
Contributor

bors commented Nov 27, 2020

💥 Test timed out

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

jyn514 commented Nov 27, 2020

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

bors commented Nov 27, 2020

⌛ Testing commit 09a3bc1 with merge 6a88957...

@bors
Copy link
Contributor

bors commented Nov 27, 2020

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 6a88957 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 27, 2020
@bors bors merged commit 6a88957 into rust-lang:master Nov 27, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 27, 2020
@jyn514 jyn514 deleted the more-cleanup branch November 27, 2020 22:36
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2020
Remove doctree::Macro and distinguish between `macro_rules!` and `pub macro`

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

Fixes rust-lang#76761
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

6 participants