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

Add some docs to rustdoc::clean::inline and def_id functions #76784

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Add some docs to rustdoc::clean::inline and def_id functions #76784

merged 1 commit into from
Oct 7, 2020

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Sep 16, 2020

Split from #76571 .

@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 Sep 16, 2020
@tesuji
Copy link
Contributor Author

tesuji commented Sep 16, 2020

r? @jyn514

@rust-highfive rust-highfive assigned jyn514 and unassigned ollie27 Sep 16, 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 Sep 16, 2020
jyn514
jyn514 reviewed Sep 16, 2020
View reviewed changes
compiler/rustc_span/src/def_id.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/inline.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/inline.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/inline.rs Outdated
@@ -290,10 +290,12 @@ pub fn build_ty(cx: &DocContext<'_>, did: DefId) -> Option<clean::Type> {
}
}

// Passed `DefId` has to be a Trait item/path/reexport.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure whether it could also be a primitive, because of lang items. Maybe @GuillaumeGomez would know?

Suggested change
// Passed `DefId` has to be a Trait item/path/reexport.
// Passed `DefId` has to be an ADT (struct/union/enum)

Copy link
Member

Choose a reason for hiding this comment

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

This looks good to merge once you address this comment.

I just realized this can't be a primitive because it assumes you have a DefId. So no need to worry about that bit.

Copy link
Member

Choose a reason for hiding this comment

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

This can be a primitive because of lang_items. Ugh rustdoc is a mess. https://doc.rust-lang.org/nightly/nightly-rustc/rustc_typeck/coherence/inherent_impls/struct.InherentCollect.html#method.check_primitive_impl

Suggested change
// Passed `DefId` has to be a Trait item/path/reexport.
// Passed `DefId` has to be an ADT (struct/union/enum) or a `lang_item` for a primitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

where's the call site passing lang_items of primitive to build_impls ?

@jyn514 jyn514 mentioned this pull request Sep 16, 2020
Closed
jyn514
jyn514 reviewed Sep 17, 2020
View reviewed changes
src/librustdoc/clean/inline.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/inline.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 Sep 17, 2020
@bors
Copy link
Contributor

bors commented Sep 22, 2020

☔ The latest upstream changes (presumably #77039) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@tesuji
Copy link
Contributor Author

tesuji commented Oct 5, 2020

Rebased. Not sure why is this waiting on me (author). It needs more input/info from reviewer.

@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot 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 Oct 5, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 5, 2020

I don't know this code well enough to review it.

r? @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Oct 6, 2020

📌 Commit 31084cf09499c43d309cd17062e97b0508b8f64c 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 Oct 6, 2020
@tesuji
Copy link
Contributor Author

tesuji commented Oct 6, 2020

Squashed to make commit history clean! Sorry for this inconvenient.

@tesuji
Add some docs to rustdoc::clean::inline and def_id functions
63e7234 
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
@GuillaumeGomez
Copy link
Member

No problem, r=me once CI pass.

@jyn514
Copy link
Member

jyn514 commented Oct 6, 2020

@bors r=GuillaumeGomez rollup

@bors
Copy link
Contributor

bors commented Oct 6, 2020

📌 Commit 63e7234 has been approved by GuillaumeGomez

@jyn514
Copy link
Member

jyn514 commented Oct 6, 2020

Thanks for working on this :) sorry I couldn't be much help.

@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Oct 6, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 6, 2020
@bors
Auto merge of rust-lang#77630 - Dylan-DPC:rollup-kfwl55z, r=Dylan-DPC
59dafb8 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#76784 (Add some docs to rustdoc::clean::inline and def_id functions)
 - rust-lang#76911 (fix VecDeque::iter_mut aliasing issues)
 - rust-lang#77400 (Fix suggestions for x.py setup)
 - rust-lang#77515 (Update to chalk 0.31)
 - rust-lang#77568 (inliner: use caller param_env)
 - rust-lang#77571 (Use matches! for core::char methods)
 - rust-lang#77582 (Move `EarlyOtherwiseBranch` to mir-opt-level 2)
 - rust-lang#77590 (Update RLS and Rustfmt)
 - rust-lang#77605 (Fix rustc_def_path to show the full path and not the trimmed one)
 - rust-lang#77614 (Let backends access span information)
 - rust-lang#77624 (Add c as a shorthand check alternative for new options rust-lang#77603)

Failed merges:

r? `@ghost`
@bors bors merged commit d26ca98 into rust-lang:master Oct 7, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 7, 2020
@tesuji tesuji deleted the rd_doc branch October 7, 2020 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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.49.0
Development

Successfully merging this pull request may close these issues.
7 participants
@tesuji @rust-highfive @bors @jyn514 @GuillaumeGomez @ollie27 @rustbot