Skip to content

Apply doc(cfg) from parent items while collecting trait impls - #79349

Merged
bors merged 2 commits into
rust-lang:masterfrom
Nemo157:issue-79201
Dec 12, 2020
Merged

bors merged 2 commits into
rust-lang:masterfrom
Nemo157:issue-79201

Conversation

@Nemo157

@Nemo157 Nemo157 commented Nov 23, 2020

Copy link
Copy Markdown
Contributor

Because trait impls bypass the standard clean hierarchy they do not participate in the propagate_doc_cfg pass, so instead we need to pre-collect all possible doc(cfg) attributes that will apply to them when cleaning.

fixes #79201

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @GuillaumeGomez

(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 23, 2020
Comment thread src/librustdoc/passes/collect_trait_impls.rs Outdated
@GuillaumeGomez

Copy link
Copy Markdown
Member

Looks good to me, thanks! Maybe @jyn514 will want to take a look at the compiletest change? ;)

@Nemo157

Nemo157 commented Nov 23, 2020

Copy link
Copy Markdown
Contributor Author

Just pushed the test I had written but forgot to commit before 🤦

@jyn514 jyn514 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM but this will conflict with #79312 - do you mind rebasing over that?

@Nemo157

Nemo157 commented Nov 23, 2020

Copy link
Copy Markdown
Contributor Author

Ah, the debug derives. I can just drop that commit, it was just something I used while trying to track down where this needed to go and thought would be useful to leave in for others.

Because trait impls bypass the standard `clean` hierarchy they do not
participate in the `propagate_doc_cfg` pass, so instead we need to
pre-collect all possible `doc(cfg)` attributes that will apply to them
when cleaning.
@jyn514

jyn514 commented Nov 23, 2020

Copy link
Copy Markdown
Member

@Nemo157 no, the whole thing will conflict - doctree::Impl is being deleted and the code for impls works off of hir::ItemKind now.

@Nemo157

Nemo157 commented Nov 23, 2020

Copy link
Copy Markdown
Contributor Author

I cherry-picked this change onto that branch, it applied without any conflicts 🤷

debug!("build_impl: impl {:?} for {:?}", trait_.def_id(), for_.def_id());

ret.push(clean::Item::from_def_id_and_parts(
let mut item = clean::Item::from_def_id_and_parts(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like to distinguish when things are being overriden by rustdoc (as opposed to what most tools will see).

Suggested change
let mut item = clean::Item::from_def_id_and_parts(
let mut what_rustc_thinks = clean::Item::from_def_id_and_parts(

I guess it doesn't make as much sense here because you used mutability instead of record update syntax ... up to you whether you want to change it.

@jyn514

jyn514 commented Nov 23, 2020

Copy link
Copy Markdown
Member

r=me unless you want to fix the nit.

@bors delegate=Nemo157

@bors

bors commented Nov 23, 2020

Copy link
Copy Markdown
Collaborator

✌️ @Nemo157 can now approve this pull request

@crlf0710 crlf0710 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 Dec 11, 2020
@crlf0710

Copy link
Copy Markdown
Member

@Nemo157 Triage: what's the current status of this?

@jyn514

jyn514 commented Dec 11, 2020

Copy link
Copy Markdown
Member

I think we should just land this.

@bors r+

@bors

bors commented Dec 11, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit d93f1d6 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 Dec 11, 2020
@bors

bors commented Dec 11, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit d93f1d6 with merge 9eb3a7c...

@bors

bors commented Dec 12, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 9eb3a7c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 12, 2020
@bors
bors merged commit 9eb3a7c into rust-lang:master Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doc(cfg) doesn't inherit down onto trait impls in modules

7 participants