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

Rustdoc: don't hide anonymous reexport #108936

Merged
merged 2 commits into from
Mar 10, 2023
Merged

Rustdoc: don't hide anonymous reexport #108936

merged 2 commits into from
Mar 10, 2023

Conversation

GuillaumeGomez
Copy link
Member

Fixes #108931.

From #108931, it appears that having anonymous re-exports for traits is actually used in some places, so instead of hiding them automatically, we should prevent them to be ever inlined.

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 9, 2023
@sdroege
Copy link
Contributor

sdroege commented Mar 9, 2023

Maybe this should only consider traits. Re-exporting anything but a trait as _ has no effect AFAIU.

Or maybe re-exporting anything but a trait as _ should actually be a compiler warning (or error)?

@GuillaumeGomez
Copy link
Member Author

I approve both suggestions (only for traits and the compiler warning).

@sdroege
Copy link
Contributor

sdroege commented Mar 9, 2023

The reference says https://doc.rust-lang.org/reference/items/use-declarations.html#underscore-imports

Items can be imported without binding to a name by using an underscore with the form use path as _. This is particularly useful to import a trait so that its methods may be used without importing the trait's symbol, for example if the trait's symbol may conflict with another symbol. Another example is to link an external crate without importing its name.

So crates might also have an effect, but which? Together with #[macro_use] maybe?

This should be in a separate PR in any case, and in either case should probably also get some documentation updates so the behaviour of it for non-trait items is actually clear.

@notriddle
Copy link
Contributor

Maybe this should only consider traits. Re-exporting anything but a trait as _ has no effect AFAIU.

I would prefer not to do anything more complicated here than we have to:

  • If the author doesn't like the result, they can #[doc(hidden)] it.
  • If the re-export truly has no effect, rustc should warn on it.

@GuillaumeGomez
Copy link
Member Author

I'm fine with this approach too. If so, the current PR is ready then.

@GuillaumeGomez GuillaumeGomez changed the title Rustdoc anonymous reexport Rustdoc: don't hide anonymous reexport Mar 9, 2023
@notriddle
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 9, 2023

📌 Commit 76480e834611536d8b5797740fb6c25f586b0b0a has been approved by notriddle

It is now in the queue for this repository.

@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 Mar 9, 2023
@GuillaumeGomez GuillaumeGomez force-pushed the rustdoc-anonymous-reexport branch from 76480e8 to 9b788da Compare March 9, 2023 17:08
@notriddle
Copy link
Contributor

Huh, yeah, thanks for catching that.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 9, 2023

📌 Commit 9b788da has been approved by notriddle

It is now in the queue for this repository.

@matthiaskrgr matthiaskrgr mentioned this pull request Mar 10, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2023
@bors
Auto merge of rust-lang#108977 - matthiaskrgr:rollup-1bnl1hu, r=matth…
35a0961 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#108879 (Unconstrained terms should account for infer vars being equated)
 - rust-lang#108936 (Rustdoc: don't hide anonymous reexport)
 - rust-lang#108940 (Add myself to compiler reviewers list)
 - rust-lang#108945 (Make some report and emit errors take DefIds instead of BodyIds)
 - rust-lang#108946 (Document the resulting values produced when using `From<bool>` on floats)
 - rust-lang#108956 (Make ptr::from_ref and ptr::from_mut in rust-lang#106116 const.)
 - rust-lang#108960 (Remove `body_def_id` from `Inherited`)
 - rust-lang#108963 (only call git on git checkouts during bootstrap)
 - rust-lang#108964 (Fix the docs for pointer method with_metadata_of)

Failed merges:

 - rust-lang#108950 (Directly construct Inherited in typeck.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6ef07c2 into rust-lang:master Mar 10, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 10, 2023
@GuillaumeGomez GuillaumeGomez deleted the rustdoc-anonymous-reexport branch March 10, 2023 19:15
@GuillaumeGomez GuillaumeGomez mentioned this pull request Mar 10, 2023
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 19, 2023
@Dylan-DPC
Rollup merge of rust-lang#109003 - GuillaumeGomez:useless-anonymous-r…
462e7e7 
…eexport-lint, r=cjgillot

Add `useless_anonymous_reexport` lint

This is a follow-up of rust-lang#108936. We once again show all anonymous re-exports in rustdoc, however we also wanted to add a lint to let users know that it very likely doesn't have the effect they think it has.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aDotInTheVoid aDotInTheVoid aDotInTheVoid left review comments

Assignees

@notriddle notriddle

Labels
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.70.0
Development

Successfully merging this pull request may close these issues.

rustdoc doesn't show traits re-exported as _
6 participants
@GuillaumeGomez @sdroege @notriddle @bors @aDotInTheVoid @rustbot