-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC for doc_cfg, doc_cfg_auto, doc_cfg_hide and doc_cfg_show features #3631
base: master
Are you sure you want to change the base?
Changes from 1 commit
e6b88c1
e5d1eeb
3978a1c
cae5f57
08f47ce
686c7e9
d2ee5df
aad1fbd
4746afc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,283 @@ | ||
Rustdoc: stabilization of the `doc(cfg*)` attributes | ||
|
||
- Features Name: `doc_cfg` | ||
- Start Date: 2022-12-07 | ||
- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) | ||
- Rust Issue: [rust-lang/rust#43781](https://github.com/rust-lang/rust/issues/43781) | ||
|
||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
This RFC aims at providing rustdoc users the possibility to add visual markers to the rendered documentation to know under which conditions an item is available (currently possible through the following unstable features: `doc_cfg`, `doc_auto_cfg` and `doc_cfg_hide`). | ||
|
||
It does not aim to allow having a same item with different `cfg`s to appear more than once in the generated documentation. | ||
|
||
It does not aim to document items which are *inactive* under the current configuration (i.e., “`cfg`'ed out”). | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
The goal of this RFC is to stabilize the possibility to add visual markers to the rendered documentation to know under which conditions an item is available. | ||
|
||
Providing this information to users will solve a common issue: “Why can I see this item in the documentation and yet can't use it in my code?”. | ||
The end goal being to provide this information automatically so that the documentation maintenance cost won't increase. | ||
|
||
|
||
# Guide-level explanation | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mentioned this as a regular comment, but I'll move it to a review so it gets noticed: I think that the guide-level explanation should clarify the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was not ignored. You can see the RFC mentioning this here. If it's not clear enough, how would you word it? |
||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
This RFC proposes to add the following attributes: | ||
|
||
* `#[doc(cfg(...))]` | ||
|
||
This attribute is used to document the operating systems, feature flags, and build profiles where an item is available. For example, `#[doc(cfg(unix))` will add a tag that says "this is supported on **unix** only" to the item. | ||
|
||
The syntax of this attribute is the same as the syntax of the [`#[cfg(unix)]` attribute][cfg attribute] used for conditional compilation. | ||
|
||
* `#[doc(auto_cfg)]`/`#[doc(no_auto_cfg)]` | ||
|
||
When this is turned on, `#[cfg]` attributes are shown in documentation just like `#[doc(cfg)]` attributes are. | ||
|
||
* `#[doc(cfg_hide(...))]` / `#[doc(cfg_show(...))]` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: so i've never been happy with the sheer number of different doc options this feature introduces: the names are kind of all over the place and hard to predict. I think Perhaps There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally wouldn't like using That said, perhaps there would be a case for having Still has some issues, but, if we were to change the existing attributes, I'd prefer something like that over just combining the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @clarfonthey point taken on subtle syntax, I didn't really want to propose that anyway which is why I proposed it coming out of To me the main distinction between If everything trees out of a single attribute it's easier to remember IMO, and not confusing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And yes I'm happy with experimentation with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not exactly convinced that My main point here is that having "fewer attributes" isn't really a principle we should be guided by, since what matters most is the user writing the code, not the compiler parsing it. To a user, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what you suggested @Nemo157. Do you mean that if no argument is provided to So to summarize the discussion, it seems that we tend to for #[doc(cfg_show(test))]
#[doc(cfg_hide(test))] As for enabling/disabling the feature, what do you think about: #[doc(cfg_auto(enable)]
#[doc(cfg_auto(disable)] ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding of the issue, after completely misunderstanding for a while (apologies, @Manishearth) is that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't have a concrete syntax proposal, I just wanted to seed the idea in case someone else has thoughts on how it could be accomplished. Being able to reduce this feature to introducing a total of two attributes instead of five I think is a helpful simplification. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @clarfonthey yes, thanks. my two main complaints are that the split of attributes starting and ending with cfg is confusing (and the potential for them to do either makes it harder to remember even if we pick consistent names¹), and the second one is the one you describe: it's not clear that show/hide is about auto stuff. ¹ basically, even if we picked cfg_auto, cfg_show, cfg_hide, I am somewhat (but less) worried people will forget which way things are. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So to summarize the current attributes this RFC proposes:
It's 4 new attributes. If I understood what @Manishearth suggested, they'd like to instead go with:
Did I miss something else you suggested @Manishearth ? |
||
|
||
These attributes suppress or un-suppress the `auto_cfg` behavior for a particular configuration predicate. | ||
|
||
For example, `#[doc(cfg_hide(windows))]` shall be used in newer versions of the [`windows` crate] to prevent the "this is supported on **windows** only" tag from being shown on every single item. | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
[cfg attribute]: https://doc.rust-lang.org/reference/conditional-compilation.html | ||
[`windows` crate]: https://docs.rs/windows/latest/windows/ | ||
|
||
All of these attributes can be added to a module or to the crate root, and they will be inherited by the child items unless another attribute overrides it (except that `doc(cfg)` cannot be added to the crate root). This is why "opposite" attributes like `cfg_hide` and `cfg_show` are provided: they allow a child item to override its parent. | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
# Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
## The attributes | ||
|
||
### `#[doc(cfg(...))]` | ||
|
||
This attribute provides a standardized format to document conditionally available items. Example: | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```rust | ||
// the "real" cfg condition | ||
#[cfg(feature = "futures-io")] | ||
// the `doc(cfg())` so it's displayed to the readers | ||
#[doc(cfg(feature = "futures-io"))] | ||
pub mod futures {} | ||
``` | ||
|
||
It will display in the documentation for this module: | ||
|
||
![This is supported on feature="futures-io" only.](https://user-images.githubusercontent.com/81079/89731116-d7b7ce00-da44-11ea-87c6-022d192d6eca.png) | ||
|
||
This attribute has the same syntax as conditional compilation, but it only causes documentation to be added. This means `#[doc(cfg(false))` will not cause your docs to be hidden, even though `#[cfg(false)]` does do that. | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This attribute works on modules and on items but cannot be used at the crate root level. | ||
|
||
### `#[doc(auto_cfg)]`/`#[doc(no_auto_cfg)]` | ||
|
||
By default, `#[doc(auto_cfg)]` is enabled at the crate-level. When it's enabled, Rustdoc will automatically display `cfg(...)` compatibility information as-if the same `#[doc(cfg(...))]` had been specified. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to change this default in existing code, or maybe this should be edition tied? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can see, it doesn't bring downside to have more information displayed by default, so I think it shouldn't be tied to an edition. |
||
|
||
So if we take back the previous example: | ||
|
||
```rust | ||
#[cfg(feature = "futures-io")] | ||
pub mod futures {} | ||
``` | ||
|
||
There's no need to "duplicate" the `cfg` into a `doc(cfg())` to make Rustdoc display it. | ||
|
||
In some situations, the detailed conditional compilation rules used to implement the feature might not serve as good documentation (for example, the list of supported platforms might be very long, and it might be better to document them in one place). To turn it off, add the `#[doc(no_auto_cfg)]` attribute. | ||
|
||
Both `#[doc(auto_cfg)]` and `#[doc(no_auto_cfg)]` attributes impact all there descendants. You can then enable/disable them by using the opposite attribute on a given item. They can be used as follows: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation only allows these attributes at the crate-level, has there been a push to support this arbitrary nesting from somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Well, actually the attributes don't exist, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, it's a change brought by this RFC: users will be able to use them on items directly if they want to have better control where they want it to be enabled or not. Just like lints, I think it's better to give better control over this. For example if someone wants to completely override doc cfg attributes on a given item, they will be able to disable it for this item completely and then add what they want using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The use case where you would want to disable Similarly wanting to Making these attributes work within the module tree brings in extra complexity by requiring the inverses to exist. If they're restricted to applying only at the crate root we only need two new attributes: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point. I'll limit For Anyway, if no one seems to be interested into having There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just thought about a use case where having #![doc(auto_cfg = true)]
#![doc(cfg_hide(/* list of all supported OSes */)] Or instead disable So with this in mind, I think we should lift the crate-level only restriction on |
||
|
||
```rust | ||
// As an inner attribute, all this module's descendants will have this feature | ||
// enabled. | ||
#![doc(auto_cfg)] | ||
|
||
// As an outer attribute. So in this case, `foo` and all its | ||
// descendants won't have the `auto_cfg` feature enabled. | ||
#[doc(no_auto_cfg)] | ||
pub mod foo { | ||
// We re-enable the feature on `Bar` and on all its descendants. | ||
#[doc(auto_cfg)] | ||
pub struct Bar { | ||
pub f: u32, | ||
} | ||
} | ||
``` | ||
|
||
As mentioned, both attributes can be used on modules, items and crate root level. | ||
|
||
### `#[doc(cfg_hide(...))]` | ||
|
||
This attribute is used to prevent some `cfg` to be generated in the visual markers. So in the previous example: | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```rust | ||
#[cfg(any(doc, feature = "futures-io"))] | ||
pub mod futures {} | ||
``` | ||
|
||
It currently displays both `doc` and `feature = "futures-io"` into the documentation, which is not great. To prevent the `doc` cfg to ever be displayed, you can use this attribute at the crate root level: | ||
|
||
```rust | ||
#![doc(cfg_hide(doc))] | ||
``` | ||
|
||
Or directly on a given item/module as it covers any of the item's descendants: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again the current implementation only supports being set at the crate-level, are there usecases that want to be able to limit it to certain sub-trees? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The one case I'm thinking about is to reduce complexity of rendered doc cfg as described in this comment. |
||
|
||
```rust | ||
#[doc(cfg_hide(doc))] | ||
#[cfg(any(doc, feature = "futures-io"))] | ||
pub mod futures { | ||
// `futures` and all its descendants won't display "doc" in their cfgs. | ||
} | ||
``` | ||
|
||
Then, the `doc` cfg will never be displayed into the documentation. | ||
|
||
Rustdoc currently hides `doc` and `doctest` attributes by default and reserves the right to change the list of "hidden by default" attributes. | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The attribute accepts only a list of identifiers or key/value items. So you can write: | ||
|
||
```rust | ||
#[doc(cfg_hide(doc, doctest, feature = "something"))] | ||
#[doc(cfg_hide())] | ||
``` | ||
|
||
But you cannot write: | ||
|
||
```rust | ||
#[doc(cfg_hide(not(doc)))] | ||
``` | ||
|
||
### `#[doc(cfg_show(...))]` | ||
|
||
This attribute does the opposite of `#[doc(cfg_hide(...))]`: if you used `#[doc(cfg_hide(...))]` and want to revert its effect on an item and its descendants, you can use `#[doc(cfg_show(...))]`: | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```rust | ||
#[doc(cfg_hide(doc))] | ||
#[cfg(any(doc, feature = "futures-io"))] | ||
pub mod futures { | ||
// `futures` and all its descendants won't display "doc" in their cfgs. | ||
#[doc(cfg_show(doc))] | ||
pub mod child { | ||
// `child` and all its descendants will display "doc" in their cfgs. | ||
} | ||
} | ||
``` | ||
|
||
The attribute accepts only a list of identifiers or key/value items. So you can write: | ||
|
||
```rust | ||
#[doc(cfg_show(doc, doctest, feature = "something"))] | ||
#[doc(cfg_show())] | ||
``` | ||
|
||
But you cannot write: | ||
|
||
```rust | ||
#[doc(cfg_show(not(doc)))] | ||
``` | ||
|
||
## Inheritance | ||
|
||
Rustdoc merges `cfg` attributes from parent modules to its children. For example, in this case, the module `non_unix` will describe the entire compatibility matrix for the module, and not just its directly attached information: | ||
|
||
```rust | ||
#[doc(cfg(any(windows, unix)))] | ||
pub mod desktop { | ||
#[doc(cfg(not(unix)))] | ||
pub mod non_unix { | ||
// | ||
} | ||
} | ||
``` | ||
|
||
> ![Available on (Windows or Unix) and non-Unix only.](https://hackmd.io/_uploads/SJrmwYeF2.png) | ||
|
||
[Future versions of rustdoc][boolean simplification] may simplify this display down to "available on **Windows** only." | ||
|
||
### Re-exports and inlining | ||
|
||
`cfg` attributes of a re-export are never merged the re-exported item(s). If `#[doc(inline)]` attribute is used on a re-export, the `cfg` of the re-exported item will be merged with the re-export's. | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```rust | ||
#[doc(cfg(any(windows, unix)))] | ||
pub mod desktop { | ||
#[doc(cfg(not(unix)))] | ||
pub mod non_unix { | ||
// | ||
} | ||
} | ||
|
||
#[doc(cfg(target_os = "freebsd"))] | ||
pub use desktop::non_unix as non_unix_desktop; | ||
#[doc(cfg(target_os = "macos"))] | ||
#[doc(inline)] | ||
pub use desktop::non_unix as inlined_non_unix_desktop; | ||
``` | ||
|
||
In this example, `non_unix_desktop` will only display `cfg(target_os = "freeebsd")` and not display any `cfg` from `desktop::non_unix`. | ||
|
||
On the contrary, `inlined_non_unix_desktop` will have cfgs from both the re-export and the re-exported item. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
A potential drawback is that it adds more attributes, making documentation more complex. | ||
|
||
|
||
# Rationale and alternatives | ||
[rationale-and-alternatives]: #rationale-and-alternatives | ||
|
||
## Why not merging cfg and doc(cfg) attributes by default? | ||
|
||
It was debated and implemented in [rust-lang/rust#113091](https://github.com/rust-lang/rust/pull/113091). | ||
|
||
When re-exporting items with different cfgs there are two things that can happen: | ||
|
||
1. The re-export uses a subset of cfgs, this subset is sufficient so that the item will appear exactly with the subset | ||
2. The re-export uses a non-subset of cfgs like in this code: | ||
```rust | ||
#![feature(doc_auto_cfg)] | ||
|
||
#[cfg(target_os = "linux")] | ||
mod impl_ { | ||
pub fn foo() { /* impl for linux */ } | ||
} | ||
|
||
#[cfg(target_os = "macos")] | ||
mod impl_ { | ||
pub fn foo() { /* impl for darwin */ } | ||
} | ||
|
||
pub use impl_::foo; | ||
``` | ||
If the non-subset cfgs are active (e.g. compiling this example on windows), then this will be a compile error as the item doesn't exist to re-export. If the subset cfgs are active it behaves like described in 1. | ||
|
||
|
||
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
|
||
# Future possibilities | ||
[future possibilities]: #future-possibilities | ||
|
||
## Boolean simplification | ||
[boolean simplification]: #boolean-simplification | ||
|
||
> ![Available on (Windows or Unix) and non-Unix only.](https://hackmd.io/_uploads/SJrmwYeF2.png) | ||
|
||
Of course, the above example is equivalent to "available on **Windows** only." | ||
|
||
Making this actually work all the time is equivalent to a [boolean satisfiability] check, coliquially called a "SAT problem," and can take exponential time. | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
[boolean satisfiability]: https://en.wikipedia.org/wiki/Boolean_satisfiability_problem | ||
|
||
We probably don't want to make promises one way or the other about whether rustdoc does this, but for compatibility's sake, Rustdoc does promise that `#[doc(cfg(false))` will not hide the documentation. This means simplification can be added, and it won't cause docs to mysteriously vanish. | ||
|
||
This is tracked in issue [rust-lang/rust#104991](https://github.com/rust-lang/rust/issues/104991). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...I'm slightly confused by this preamble, honestly, because I would figure the most important time to know when something is available is when it is, in fact, not available... when it has been "cfg'd out" but can be enabled by other means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, this sentence doesn't tell the full story and should be improved (however, it's technically correct). I'm pretty sure the RFC does elaborate on it further down in the document (Ctrl+F
doc
(Match Case, Match Full Words)).It implicitly refers to rust-lang/rust#1998.
As you can probably guess, rustdoc doesn't and won't magically recompile the to-be-documented crate under all possible cfgs and combinations thereof to gather the information necessary to display the "portability info box".
So for the following crate,
function
wouldn't show up in the generated docs unless you actually passed--cfg special
torustdoc
:Therefore, the common and offical workaround is the use of the semi-special cfg
doc
:This gets more hairy if there are "multiple" functions called
function
(where the cfg specs differ for obvious reasons), one needs to choose which of the many functions to annotate withdoc
.Of course, this doesn't scale to more complicated cases. E.g., if the parameter/return types differ by cfg spec. So, yeah. The general problem isn't solved. That's what I meant with the sentence you highlighted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, I've certainly written documentation that needs that invocation (or similar) to work properly. I guess I'm just feeling a bit perplexed that there is seemingly quite a lot of new concepts in this RFC and that's left unsolved. I would expect an RFC that has the sense of adding a Lot Of Stuff to also have an answer to that critical missing piece, because it's a right pain in the ass! ...or I would expect it to be smaller and more tightly scoped.
I realize that's a Pure Vibes thing, but I do think it can be a bit hard to evaluate a solution... and thus people would come to consensus slower... if it feels incorrectly scoped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean, rustc now tracks enough information that it doesn’t have to
rust-lang/rust#109005
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, rustc only stores the name of cfg'ed out items but rustdoc would need to have access to so much more (struct fields, function signatures, ...). See
StrippedCfgItem
.Edit: And doing that would drastically increase size of the crate metadata / rustc's memory use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There actually wasn't an explanation about this in the RFC so I added one in the
Unresolved questions
section.