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

Remove the unused-#[doc(hidden)] logic from the unused_attributes lint #98336

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

fmease
Copy link
Member

@fmease fmease commented Jun 21, 2022

Fixes #96890.

It was found out that #[doc(hidden)] on trait impl items does indeed have an effect on the generated documentation (see the linked issue). In my opinion and the one of others, rustdoc's output is actually a bit flawed in that regard but that should be tracked in a new issue I suppose (I will open an issue for that in the near future).

The check was introduced in #96008 which is marked to be part of version 1.62 (current beta). As far as I understand, this means that this PR needs to be backported to beta to fix #96890 on time. Correct me if I am wrong.

CC @dtolnay (in case you would like to agree or disagree with my decision to fully remove this check)

@rustbot label A-lint T-compiler T-rustdoc

r? @rust-lang/compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 21, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 21, 2022
@rustbot rustbot added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jun 21, 2022
@fmease fmease changed the title Entirely remove unused #[doc(hidden)] logic from the unused_attributes lint Remove the unused #[doc(hidden)] logic from the unused_attributes lint Jun 21, 2022
@fmease fmease changed the title Remove the unused #[doc(hidden)] logic from the unused_attributes lint Remove the unused-#[doc(hidden)] logic from the unused_attributes lint Jun 21, 2022
@fmease fmease force-pushed the remove-faulty-doc-hidden-lint branch from c0d3eb9 to 67508f3 Compare June 21, 2022 12:06
@fmease
Copy link
Member Author

fmease commented Jun 21, 2022

Or does this need to be reviewed by T-lang?

@compiler-errors
Copy link
Member

compiler-errors commented Jun 21, 2022

cc @GuillaumeGomez @lcnr who approved the original PR that added this lint.
cc @rust-lang/lang, specifically @scottmcm who unofficially signed off on this.

nominating for backport consideration so this gets into 1.62

@compiler-errors compiler-errors added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 21, 2022
@compiler-errors
Copy link
Member

r=me, code changes look fine but i would like someone with more context about this lint getting added to sign off as well

@GuillaumeGomez
Copy link
Member

Isn't it possible to fix the issues rather than entirely removing the check? It provides useful information after all...

@compiler-errors
Copy link
Member

i would prefer to remove the check from beta and fix the lint later, so it can properly bake on nightly and beta.

@GuillaumeGomez
Copy link
Member

For that we can check whether we're in nightly and only run it in this case, no?

@compiler-errors
Copy link
Member

I don't think we typically gate logic like that based on nightly/beta/etc. Seems like a hack, and since this revert PR is very simple, it does not seem warranted.

@compiler-errors
Copy link
Member

imo, @fmease can reintroduce this logic in a couple weeks in nightly 1.64, so that it can have plenty of time to surface issues.

@GuillaumeGomez
Copy link
Member

Fair enough. Let's approve this revert for the time being even though it makes me sad. Let's hope the upgraded version will soon follow. :)

@bors r+

@bors
Copy link
Contributor

bors commented Jun 21, 2022

📌 Commit 67508f3 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 Jun 21, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#97867 (lub: don't bail out due to empty binders)
 - rust-lang#98099 (interpret: convert_tag_add_extra: allow tagger to raise errors)
 - rust-lang#98199 (Move some tests to more reasonable directories)
 - rust-lang#98334 (Add a full regression test for rust-lang#73727)
 - rust-lang#98336 (Remove the unused-`#[doc(hidden)]` logic from the `unused_attributes` lint)
 - rust-lang#98344 (This comment is out dated and misleading, the arm is about TAITs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b887da1 into rust-lang:master Jun 22, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 22, 2022
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 23, 2022
ehuss pushed a commit to ehuss/rust that referenced this pull request Jun 24, 2022
…t, r=GuillaumeGomez

Remove the unused-`#[doc(hidden)]` logic from the `unused_attributes` lint

Fixes rust-lang#96890.

It was found out that `#[doc(hidden)]` on trait impl items does indeed have an effect on the generated documentation (see the linked issue). In my opinion and the one of [others](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Validy.20checks.20for.20.60.23.5Bdoc.28hidden.29.5D.60/near/281846219), rustdoc's output is actually a bit flawed in that regard but that should be tracked in a new issue I suppose (I will open an issue for that in the near future).

The check was introduced in rust-lang#96008 which is marked to be part of version `1.62` (current `beta`). As far as I understand, this means that **this PR needs to be backported** to `beta` to fix rust-lang#96890 on time. Correct me if I am wrong.

CC `@dtolnay` (in case you would like to agree or disagree with my decision to fully remove this check)

`@rustbot` label A-lint T-compiler T-rustdoc

r? `@rust-lang/compiler`
@ehuss ehuss mentioned this pull request Jun 24, 2022
@ehuss ehuss modified the milestones: 1.63.0, 1.62.0 Jun 24, 2022
@ehuss ehuss removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 24, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2022
[beta] Beta backports

* Remove the unused-#[doc(hidden)] logic from the unused_attributes lint rust-lang#98336
* debuginfo: Fix NatVis for Rc and Arc with unsized pointees. rust-lang#98137
* Revert "remove num_cpus dependency" in rustc and update cargo rust-lang#97911
* Update LLVM submodule rust-lang#97690
* Revert rust-lang#96682. rust-lang#97636
* don't do Sized and other return type checks on RPIT's real type rust-lang#97431
* Temporarily disable submodule archive downloads. rust-lang#98423
@fmease fmease deleted the remove-faulty-doc-hidden-lint branch July 23, 2022 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.

False positive unused_attributes lint on doc(hidden) attribute on associated type
8 participants