Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues by GuillaumeGomez · Pull Request #88619 · rust-lang/rust

GuillaumeGomez · 2021-09-03T15:23:31Z

Fixes #88304.

I tested it based on #88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc @jyn514

rust-highfive · 2021-09-03T15:23:34Z

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

jyn514 · 2021-09-03T15:50:52Z

I don't think this is the right approach. We should find why the link-to-definition pass is broken and fix it, not change the standard library to avoid rustdoc bugs.

GuillaumeGomez · 2021-09-03T16:17:17Z

Well, it's not specific to the jump to definition feature. The link itself is correct after all... The problem is that we have a different rendering with "cfg(doc)". We literally can't fix it.

jyn514 · 2021-09-03T16:19:40Z

Got it, that makes sense. This seems like a reasonable change then, to make the information available to rustdoc when documenting it as a dependency.

GuillaumeGomez · 2021-09-03T16:21:04Z

I'll check what's wrong with the changes I made.

GuillaumeGomez · 2021-09-03T19:15:18Z

So: in the first commit I used cfg-if to have a clean state to work on. But because of rust-lang/rust-analyzer#6038, I removed it in the second commit and reverted back to code without macro.

GuillaumeGomez · 2021-09-04T11:21:17Z

Seems like it's ready for review! :)

jyn514

@GuillaumeGomez I was planning to let @joshtriplett review, I don't think we should be changing fundamental things about libstd without asking T-libs.

jyn514 · 2021-09-03T20:35:22Z

library/std/src/os/mod.rs

This comment is out of date.

GuillaumeGomez · 2021-09-04T16:38:39Z

@GuillaumeGomez I was planning to let @joshtriplett review, I don't think we should be changing fundamental things about libstd without asking T-libs.

Which is why I didn't set you as reviewer. ;)

But reviews from you remain very welcome!

GuillaumeGomez · 2021-09-09T14:54:42Z

r? @Amanieu

library/std/src/os/mod.rs

GuillaumeGomez · 2021-09-10T11:49:01Z

Updated!

GuillaumeGomez · 2021-09-13T09:49:06Z

@matthiaskrgr @camsteffen: Any idea what's going on with the clippy lint by any chance?

camsteffen · 2021-09-13T14:33:56Z

That lint uses the absolute path of Permissions here and I'm guessing the path is changed. You can change the path in Clippy. The proper fix is to use a diagnostic item.

GuillaumeGomez · 2021-09-13T14:37:42Z

Oh I see, thanks!

GuillaumeGomez · 2021-09-13T21:06:16Z

@bors: r=Amanieu

bors · 2021-09-13T21:06:17Z

📌 Commit 9a3b1cf has been approved by Amanieu

…ports, r=Amanieu Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues Fixes rust-lang#88304. I tested it based on rust-lang#88292. Not sure if it's the best approach, but at least it makes thing a bit simpler. cc ``@jyn514``

…ports, r=Amanieu Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues Fixes rust-lang#88304. I tested it based on rust-lang#88292. Not sure if it's the best approach, but at least it makes thing a bit simpler. cc ```@jyn514```

…ports, r=Amanieu Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues Fixes rust-lang#88304. I tested it based on rust-lang#88292. Not sure if it's the best approach, but at least it makes thing a bit simpler. cc ````@jyn514````

bors · 2021-09-15T09:30:03Z

⌛ Testing commit 9a3b1cf with merge e846f9c...

bors · 2021-09-15T12:31:51Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing e846f9c to master...

rust-timer · 2021-09-15T16:12:46Z

Finished benchmarking commit (e846f9c): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 3, 2021

rust-highfive assigned joshtriplett Sep 3, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 3, 2021

This comment has been minimized.

Sign in to view

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 3, 2021

jyn514 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 Sep 3, 2021

GuillaumeGomez force-pushed the simplify-std-os-reexports branch from 5e090fd to 8ab4b68 Compare September 3, 2021 19:05

jyn514 changed the title ~~Simplify std::os module reexports to fix rustdoc linking issues~~ Remove cfg(doc) from std::os module reexports to fix rustdoc linking issues Sep 3, 2021

jyn514 reviewed Sep 4, 2021

View reviewed changes

library/std/src/os/mod.rs Outdated

Comment on lines 15 to 17

Copy link

Member

jyn514 Sep 3, 2021

Choose a reason for hiding this comment

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

This comment is out of date.

GuillaumeGomez force-pushed the simplify-std-os-reexports branch 2 times, most recently from 0570239 to 85809ea Compare September 7, 2021 13:42

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the simplify-std-os-reexports branch from 0c1e11a to ceb85de Compare September 7, 2021 13:56

rust-highfive assigned Amanieu and unassigned joshtriplett Sep 9, 2021

Amanieu reviewed Sep 10, 2021

View reviewed changes

library/std/src/os/mod.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the simplify-std-os-reexports branch from ceb85de to 88d5aa6 Compare September 10, 2021 11:48

GuillaumeGomez added 4 commits September 13, 2021 21:00

Simplify std::os module reexports to fix rustdoc linking issues

8d879aa

Remove usage of cfg_if in std/src/os/mod.rs

1c4873c

Move fortanix module position in std::os reexports for alpha sort

7965a9f

Update permissions path for clippy lint

9a3b1cf

GuillaumeGomez force-pushed the simplify-std-os-reexports branch from b98ba6f to 9a3b1cf Compare September 13, 2021 19:28

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 Sep 13, 2021

Manishearth mentioned this pull request Sep 14, 2021

Rollup of 9 pull requests #88940

Closed

Manishearth mentioned this pull request Sep 14, 2021

Rollup of 8 pull requests #88943

Closed

Manishearth mentioned this pull request Sep 14, 2021

Rollup of 8 pull requests #88948

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 15, 2021

bors merged commit e846f9c into rust-lang:master Sep 15, 2021

rustbot added this to the 1.57.0 milestone Sep 15, 2021

GuillaumeGomez deleted the simplify-std-os-reexports branch September 15, 2021 12:35

GuillaumeGomez mentioned this pull request Sep 15, 2021

Enable --generate-link-to-definition for rustc's docs #88292

Merged

bors mentioned this pull request Sep 15, 2021

Add SOLID targets #86191

Merged

4 tasks

Uh oh!

Conversation

GuillaumeGomez commented Sep 3, 2021

Uh oh!

rust-highfive commented Sep 3, 2021

Uh oh!

This comment has been minimized.

jyn514 commented Sep 3, 2021

Uh oh!

GuillaumeGomez commented Sep 3, 2021

Uh oh!

jyn514 commented Sep 3, 2021

Uh oh!

GuillaumeGomez commented Sep 3, 2021

Uh oh!

GuillaumeGomez commented Sep 3, 2021

Uh oh!

GuillaumeGomez commented Sep 4, 2021

Uh oh!

jyn514 left a comment

Choose a reason for hiding this comment

Uh oh!

jyn514 Sep 3, 2021

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Sep 4, 2021

Uh oh!

This comment has been minimized.

GuillaumeGomez commented Sep 9, 2021

Uh oh!

Uh oh!

GuillaumeGomez commented Sep 10, 2021

Uh oh!

GuillaumeGomez commented Sep 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camsteffen commented Sep 13, 2021

Uh oh!

GuillaumeGomez commented Sep 13, 2021

Uh oh!

GuillaumeGomez commented Sep 13, 2021

Uh oh!

bors commented Sep 13, 2021

Uh oh!

bors commented Sep 15, 2021

Uh oh!

bors commented Sep 15, 2021

Uh oh!

rust-timer commented Sep 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

GuillaumeGomez commented Sep 13, 2021 •

edited

Loading