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

Tracking issue for `#[doc(cfg(...))]` #43781

Open
kennytm opened this issue Aug 10, 2017 · 17 comments · May be fixed by #79263
Open

Tracking issue for `#[doc(cfg(...))]` #43781

kennytm opened this issue Aug 10, 2017 · 17 comments · May be fixed by #79263

Comments

@kennytm
Copy link
Member

@kennytm kennytm commented Aug 10, 2017

This is a tracking issue for the #[doc(cfg(...))] attribute introduced in #43348.

Steps:

(cc #1998)

fengalin added a commit to fengalin/gstreamer-rs that referenced this issue Mar 17, 2018
There are different implementations and signatures for `get_pollfd` depending
on whether the target plateform is unix or windows. When generating the doc,
we need both implementations to appear regardless of the target platform. This
commit is inspired by the way Rust `std` library deals with `process::Command`
OS dependent variants
(https://doc.rust-lang.org/std/process/struct.Command.html#impl-CommandExt).

Documentation can't be accurate though as we can't use the`std::os::windows`
on `unix` and vice versa. As a workaround a fake fd class matching the other
platform is declared.

This could be further enhanced once `#[doc(cfg(...))]` is stabilized
(rust-lang/rust#43781) by declaring `#[doc(cfg(unix))]`
or `#[doc(cfg(windows))]` instead of the hard coded comments `This is supported
on **Windows/Unix** only`. Unfortunately, these comments disappear when
generating will `--all-features` because they are not part of the documentation
in the gir file.
fengalin added a commit to fengalin/gstreamer-rs that referenced this issue Mar 17, 2018
There are different implementations and signatures for `get_pollfd` depending
on whether the target platform is unix or windows. When generating the doc,
we need both implementations to appear regardless of the target platform. This
commit is inspired by the way Rust `std` library deals with `process::Command`
OS dependent variants
(https://doc.rust-lang.org/std/process/struct.Command.html#impl-CommandExt).

Documentation can't be accurate though as we can't use the`std::os::windows`
on `unix` and vice versa. As a workaround a fake fd class matching the other
platform is declared.

This could be further enhanced once `#[doc(cfg(...))]` is stabilized
(rust-lang/rust#43781) by declaring `#[doc(cfg(unix))]`
or `#[doc(cfg(windows))]` instead of the hard coded comments `This is supported
on **Windows/Unix** only`. Unfortunately, these comments disappear when
generating will `--all-features` because they are not part of the documentation
in the gir file.
fengalin added a commit to fengalin/gstreamer-rs that referenced this issue Mar 18, 2018
There are different implementations and signatures for `get_pollfd` depending
on whether the target platform is unix or windows. When generating the doc,
we need both implementations to appear regardless of the target platform. This
commit is inspired by the way Rust `std` library deals with `process::Command`
OS dependent variants
(https://doc.rust-lang.org/std/process/struct.Command.html#impl-CommandExt).

Documentation can't be accurate though as we can't use the`std::os::windows`
on `unix` and vice versa. As a workaround a fake fd class matching the other
platform is declared.

This could be further enhanced once `#[doc(cfg(...))]` is stabilized
(rust-lang/rust#43781) by declaring `#[doc(cfg(unix))]`
or `#[doc(cfg(windows))]` instead of the hard coded comments `This is supported
on **Windows/Unix** only`. Unfortunately, these comments disappear when
generating will `--all-features` because they are not part of the documentation
in the gir file.
fengalin added a commit to fengalin/gstreamer-rs that referenced this issue Mar 19, 2018
There are different implementations and signatures for `get_pollfd` depending
on whether the target platform is unix or windows. When generating the doc,
we need both implementations to appear regardless of the target platform. This
commit is inspired by the way Rust `std` library deals with `process::Command`
OS dependent variants
(https://doc.rust-lang.org/std/process/struct.Command.html#impl-CommandExt).

Documentation can't be accurate though as we can't use the`std::os::windows`
on `unix` and vice versa. As a workaround a fake fd class matching the other
platform is declared.

This could be further enhanced once `#[doc(cfg(...))]` is stabilized
(rust-lang/rust#43781) by declaring `#[doc(cfg(unix))]`
or `#[doc(cfg(windows))]` instead of the hard coded comments `This is supported
on **Windows/Unix** only`. Unfortunately, these comments disappear when
generating will `--all-features` because they are not part of the documentation
in the gir file.
sdroege added a commit to sdroege/gstreamer-rs that referenced this issue Mar 19, 2018
There are different implementations and signatures for `get_pollfd` depending
on whether the target platform is unix or windows. When generating the doc,
we need both implementations to appear regardless of the target platform. This
commit is inspired by the way Rust `std` library deals with `process::Command`
OS dependent variants
(https://doc.rust-lang.org/std/process/struct.Command.html#impl-CommandExt).

Documentation can't be accurate though as we can't use the`std::os::windows`
on `unix` and vice versa. As a workaround a fake fd class matching the other
platform is declared.

This could be further enhanced once `#[doc(cfg(...))]` is stabilized
(rust-lang/rust#43781) by declaring `#[doc(cfg(unix))]`
or `#[doc(cfg(windows))]` instead of the hard coded comments `This is supported
on **Windows/Unix** only`. Unfortunately, these comments disappear when
generating will `--all-features` because they are not part of the documentation
in the gir file.
@TheDan64 TheDan64 mentioned this issue Mar 29, 2018
24 of 49 tasks complete
@QuietMisdreavus QuietMisdreavus added T-rustdoc and removed T-rustdoc labels May 15, 2018
charlie-ht added a commit to charlie-ht/gstreamer-rs that referenced this issue Oct 28, 2018
There are different implementations and signatures for `get_pollfd` depending
on whether the target platform is unix or windows. When generating the doc,
we need both implementations to appear regardless of the target platform. This
commit is inspired by the way Rust `std` library deals with `process::Command`
OS dependent variants
(https://doc.rust-lang.org/std/process/struct.Command.html#impl-CommandExt).

Documentation can't be accurate though as we can't use the`std::os::windows`
on `unix` and vice versa. As a workaround a fake fd class matching the other
platform is declared.

This could be further enhanced once `#[doc(cfg(...))]` is stabilized
(rust-lang/rust#43781) by declaring `#[doc(cfg(unix))]`
or `#[doc(cfg(windows))]` instead of the hard coded comments `This is supported
on **Windows/Unix** only`. Unfortunately, these comments disappear when
generating will `--all-features` because they are not part of the documentation
in the gir file.
@sfackler
Copy link
Member

@sfackler sfackler commented Jan 7, 2019

#[cfg(rustdoc)] is also gated on this issue but seems distinct (and less risky). Could we FCP that portion in particular?

asomers added a commit to asomers/futures-locks that referenced this issue Jan 30, 2019
asomers added a commit to asomers/futures-locks that referenced this issue Jan 30, 2019
asomers added a commit to asomers/futures-locks that referenced this issue Jan 30, 2019
@asomers
Copy link
Contributor

@asomers asomers commented Jan 30, 2019

I think that #[cfg(rustdoc)], when applied to structs, should automatically skip any non-public members. That would greatly reduce the amount of extra typing required by crates like Nix.

@Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Jul 4, 2019

I was just trying this out for documenting crate features, it "works" but if this is a potential usecase it would be nice to special case the rendering for it:


Screenshot 2019-07-04 at 12 35 06


Screenshot 2019-07-04 at 12 35 27

@Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Jul 4, 2019

There is also the issue that it repeats every feature on every item in a page:

Screenshot 2019-07-04 at 14 17 45

When I last attempted to do something about rendering features I found it much more useful to separately keep track of "all required features" to render at the top of the items page and "newly introduced features" to render on the sub-items on the page, so you don't get this distracting repetition on every item.

Centril added a commit to Centril/rust that referenced this issue Nov 23, 2019
…c, r=QuietMisdreavus

Stabilize cfg(doc)

cc rust-lang#43781.
@dtolnay
Copy link
Member

@dtolnay dtolnay commented Dec 25, 2019

We tried out this feature in Syn (dtolnay/syn#734) and decided against using it yet.


What I am happy with

I like how the message turns out at the top of the doc page of a single type or function.

We had previously displayed this information using an italicized note, which was less noticeable.


What I am not happy with

Our index page becomes extremely noisy. I wish there were a way to not show all of these in our case. It is enough to have this information on the type's individual page. Cfg combinations are not among the most important information to show on the index page.

Also inheriting the same note onto every public field seems unnecessary in our use case.

@Lokathor
Copy link
Contributor

@Lokathor Lokathor commented May 6, 2020

The links in the opening post have gone dead.

pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 28, 2020
…=GuillaumeGomez

Improve rendering of crate features via doc(cfg)

The current rendering of crate features with `doc(cfg(feature = ".."))` is verbose and unwieldy for users, `doc(cfg(target_feature = ".."))` is special-cased to make it render nicely, and a similar rendering can be applied to `doc(cfg(feature))` to make it easier for users to read.

I also added special casing of `all`/`any` cfgs consisting of just `feature`/`target-feature` to remove the repetitive "target/crate feature" prefix.

The downside of this current rendering is that there is no distinction between `feature` and `target_feature` in the shorthand display. IMO this is ok, or if anything `target_feature` should have a more verbose shorthand, because `doc(cfg(feature = ".."))` usage is going to vastly outstrip `doc(cfg(target_feature = ".."))` usage in non-stdlib crates when it eventually stabilizes (or even before that given the number of crates using `cfg_attr(docsrs)` like constructs).

## Previously

<img width="259" alt="Screenshot 2020-08-09 at 13 32 42" src="https://user-images.githubusercontent.com/81079/89731110-d090c000-da44-11ea-96fa-56adc6339123.png">
<img width="438" alt="image" src="https://user-images.githubusercontent.com/81079/89731116-d7b7ce00-da44-11ea-87c6-022d192d6eca.png">
<img width="765" alt="image" src="https://user-images.githubusercontent.com/81079/89731152-24030e00-da45-11ea-9552-1c270bff2729.png">
<img width="671" alt="image" src="https://user-images.githubusercontent.com/81079/89731158-28c7c200-da45-11ea-8acb-97d8a4ce00eb.png">

## Now

<img width="216" alt="image" src="https://user-images.githubusercontent.com/81079/89731123-e1d9cc80-da44-11ea-82a8-5900bd9448a5.png">
<img width="433" alt="image" src="https://user-images.githubusercontent.com/81079/89731127-e8684400-da44-11ea-9d18-572fd810f19f.png">
<img width="606" alt="image" src="https://user-images.githubusercontent.com/81079/89731162-2feed000-da45-11ea-98d2-8a88c364d903.png">
<img width="669" alt="image" src="https://user-images.githubusercontent.com/81079/89731991-ccb46c00-da4b-11ea-9416-cd20a3193826.png">

cc rust-lang#43781
@jhpratt
Copy link
Contributor

@jhpratt jhpratt commented Sep 24, 2020

What's the status of this? I've been using it on the time crate for as long as I can remember, and a number of other crates have been doing so as well.

@Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Oct 7, 2020

For searchability: this is feature doc_cfg.

@Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Oct 7, 2020

I changed the rendering of feature="foo" cfgs in #75330, which vastly simplifies the display on module index pages like shown in the syn screenshot above. I have also just opened #77672 to address the other point @dtolnay had, that there is no need to repeat the exact same cfg rendering over and over when it is already implied by context.

Other than those changes, there are bugs around trait implementation handling such as #68100, I want to try and create an exhaustive test covering trait implementations once #77672 is done and fix their handling.

After that, I feel like this would be ready for stabilization, it's had quite thorough usage on docs.rs already, so when rendering is all fixed we can rebuild the documentation for some large crates like tokio that use it and check whether there's other edgecase bugs remaining.

@ArniDagur
Copy link

@ArniDagur ArniDagur commented Oct 7, 2020

I'm wondering: Would it not make sense to generate doc_cfg hints automatically for any #[cfg(...)] items, without also needing to type #[doc(cfg(...))]?

So the following

#[cfg(all(target_feature = "avx", target_feature = "avx2"))]
#[doc(cfg(all(target_feature = "avx", target_feature = "avx2")))]
pub mod avx;

could just be

#[cfg(all(target_feature = "avx", target_feature = "avx2"))]
pub mod avx;

Otherwise, we're duplicating information unnecessarily, right?

@Lokathor
Copy link
Contributor

@Lokathor Lokathor commented Oct 7, 2020

...pretty much

@Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Oct 8, 2020

In the general case yes, but there are quite a few crates using things like internal cfg's generated by their build.rs for compiler version detection that would not want to show those cfg to their users.

@jyn514
Copy link
Member

@jyn514 jyn514 commented Oct 15, 2020

What about the opposite - having #[doc(cfg(x))] also imply #[cfg(x)]? I can't imagine a scenario where you wouldn't want to have that.

@jyn514
Copy link
Member

@jyn514 jyn514 commented Oct 15, 2020

Not sure how hard that would be to implement, though - maybe @petrochenkov would know?

JohnTitor added a commit to JohnTitor/rust that referenced this issue Oct 15, 2020
Simplify doc-cfg rendering based on the current context

For sub-items on a page don't show cfg that has already been rendered on
a parent item. At its simplest this means not showing anything that is
shown in the portability message at the top of the page, but also for
things like fields of an enum variant if that variant itself is
cfg-gated then don't repeat those cfg on each field of the variant.

This does not touch trait implementation rendering, as that is more
complex and there are existing issues around how it deals with doc-cfg
that need to be fixed first.

### Screenshots, left is current, right is new:

![image](https://user-images.githubusercontent.com/81079/95387261-c2e6a200-08f0-11eb-90d4-0a9734acd922.png)

![image](https://user-images.githubusercontent.com/81079/95387458-06411080-08f1-11eb-81f7-5dd7f37695dd.png)

![image](https://user-images.githubusercontent.com/81079/95387702-6637b700-08f1-11eb-82f4-46b6cd9b24f2.png)

![image](https://user-images.githubusercontent.com/81079/95387905-b9aa0500-08f1-11eb-8d95-8b618d31d419.png)

![image](https://user-images.githubusercontent.com/81079/95388300-5bc9ed00-08f2-11eb-9ac9-b92cbdb60b89.png)

cc rust-lang#43781
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 16, 2020
Simplify doc-cfg rendering based on the current context

For sub-items on a page don't show cfg that has already been rendered on
a parent item. At its simplest this means not showing anything that is
shown in the portability message at the top of the page, but also for
things like fields of an enum variant if that variant itself is
cfg-gated then don't repeat those cfg on each field of the variant.

This does not touch trait implementation rendering, as that is more
complex and there are existing issues around how it deals with doc-cfg
that need to be fixed first.

### Screenshots, left is current, right is new:

![image](https://user-images.githubusercontent.com/81079/95387261-c2e6a200-08f0-11eb-90d4-0a9734acd922.png)

![image](https://user-images.githubusercontent.com/81079/95387458-06411080-08f1-11eb-81f7-5dd7f37695dd.png)

![image](https://user-images.githubusercontent.com/81079/95387702-6637b700-08f1-11eb-82f4-46b6cd9b24f2.png)

![image](https://user-images.githubusercontent.com/81079/95387905-b9aa0500-08f1-11eb-8d95-8b618d31d419.png)

![image](https://user-images.githubusercontent.com/81079/95388300-5bc9ed00-08f2-11eb-9ac9-b92cbdb60b89.png)

cc rust-lang#43781
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 15, 2020
…GuillaumeGomez

Add tests and improve rendering of cfgs on traits

Shows the additional features required to get the trait implementation, suppressing any already shown on the current page. One interesting effect from this is if you have a cfg-ed type, implementing a cfg-ed trait (so the implementation depends on both cfgs), you will get the inverted pair of cfgs shown on each page:

![image](https://user-images.githubusercontent.com/81079/97904671-207bdc00-1d41-11eb-8144-707e8017d2b6.png)

![image](https://user-images.githubusercontent.com/81079/97904700-27a2ea00-1d41-11eb-8b9f-e925ba339044.png)

The hidden items on the trait implementation also now get the correct cfgs displayed on them.

Tests are blocked on rust-lang#78673.

fixes rust-lang#68100
cc rust-lang#43781
m-ou-se added a commit to m-ou-se/rust that referenced this issue Nov 16, 2020
…GuillaumeGomez

Add tests and improve rendering of cfgs on traits

Shows the additional features required to get the trait implementation, suppressing any already shown on the current page. One interesting effect from this is if you have a cfg-ed type, implementing a cfg-ed trait (so the implementation depends on both cfgs), you will get the inverted pair of cfgs shown on each page:

![image](https://user-images.githubusercontent.com/81079/97904671-207bdc00-1d41-11eb-8144-707e8017d2b6.png)

![image](https://user-images.githubusercontent.com/81079/97904700-27a2ea00-1d41-11eb-8b9f-e925ba339044.png)

The hidden items on the trait implementation also now get the correct cfgs displayed on them.

Tests are blocked on rust-lang#78673.

fixes rust-lang#68100
cc rust-lang#43781
@est31
Copy link
Contributor

@est31 est31 commented Nov 20, 2020

I'm wondering: Would it not make sense to generate doc_cfg hints automatically for any #[cfg(...)] items, without also needing to type #[doc(cfg(...))]?

Issue with that is if you want to implement the same function on different platforms in different ways. It shouldn't show up as gated on the current platform, but as not gated at all... or if it's only implemented on a set of platforms, then as that list.

So if there is such an implication, it should be opt in, eg via an empty #[doc(cfg)] tag. Maybe one can have a feature to make it opt out for a region of code, or an entire crate, but that requires manual review of the code, so can't be the default.

having #[doc(cfg(x))] also imply #[cfg(x)]?

IMO it's strange if #[doc(...)] influences normal code.

@dtolnay
Copy link
Member

@dtolnay dtolnay commented Nov 21, 2020

This is mostly looking good for syn. Filed one bug regarding the rendering of doc cfg on impls of empty traits: #79279.

@jhpratt
Copy link
Contributor

@jhpratt jhpratt commented Nov 23, 2020

One situation where it would be nice to have doc_cfg but doesn't currently seem to be feasible is on derive-generated implementations. Even if the provider of the derive wanted to, they'd have to manually accept an equivalent attribute to doc_cfg, only to pass it on.

Not something worth holding up stabilization over, but just something I noticed when adding in more attributes in some of my code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.