rfcs
rfcs copied to clipboard
rust-lang
RFC for doc_cfg, doc_cfg_auto, doc_cfg_hide and doc_cfg_show features
I'm in favour of this but I would recommend clarifying the #[cfg(any(doc, ...))] trick in the guide-level explanation a bit further, since folks reading it might be misled into thinking that rustdoc will just ignore conditional compilation entirely to report things, which isn't true; it just shows the conditions when they're met.
Updated the syntax for auto_cfg to instead accept true or false.
Removed restriction on doc(cfg()) which was marked as not being able to be used as a crate-level attribute as it currently works using it as such, so doesn't seem there is a good enough reason to change this behaviour.
I have re-read the reference part again, but I still don't understand why there are so many attributes.
- Why is
#![doc(auto_cfg = true)]necessary if it is the default? - Why is
#![doc(auto_cfg = false)]necessary if it is equivalent to#![doc(cfg_hide(any()))]?- If it is not equivalent, then why it needs to do something different? It would be better if they were equivalent and
auto_cfgdidn't need to exist.
- If it is not equivalent, then why it needs to do something different? It would be better if they were equivalent and
- Why is
#![doc(cfg_show(...))]necessary if it is equivalent to#![doc(cfg(...))]?- If it is not equivalent, then why it needs to do something different? It would be better if they were equivalent and
cfg_showdidn't need to exist.
- If it is not equivalent, then why it needs to do something different? It would be better if they were equivalent and
It looks like having just doc(cfg) and doc(cfg_hide) would be enough to both explicitly enable and disable whatever configs are necessary, with cfg acting as a default.
A couple more questions.
- Is
#[doc(cfg)]currently allowed on an item without#[cfg]? What are the semantics?- I'd expect this to be allowed making
cfg_showredundant as in the previous comment.
- I'd expect this to be allowed making
- Is it possible to show a cfg on an item, but hide it on all children of that item? (Without putting attributes on all those children individually.)
I have re-read the reference part again, but I still don't understand why there are so many attributes.
- Why is
#![doc(auto_cfg = true)]necessary if it is the default?
Because future possibilities include to make this attribute work not only at crate-level but also on any item. And based on this discussion, I think it'll part of this RFC.
- Why is
#![doc(auto_cfg = false)]necessary if it is equivalent to#![doc(cfg_hide(any()))]?
cfg_hide doesn't work like cfg: it only accepts "items". However, the RFC is very unclear about it, so I'll add it in the RFC.
- If it is not equivalent, then why it needs to do something different? It would be better if they were equivalent and
auto_cfgdidn't need to exist.
I have to admit I didn't think about cfg_show and cfg_hide to work this way at all. I'm not sure it's a good idea though. For example, how should we handle this case:
cfg_hide(any(windows, not(unix)))
How do we interpret the not(unix)? As a whole or should it invert the "hide unix"? So for simplicity, that's why not, any and all are not supported in cfg_hide and cfg_show and why we have cfg_auto.
- Why is
#![doc(cfg_show(...))]necessary if it is equivalent to#![doc(cfg(...))]?
It is not. As mentioned in the RFC:
It only applies to
#[doc(auto_cfg = true)], not to#[doc(cfg(...))].
- If it is not equivalent, then why it needs to do something different? It would be better if they were equivalent and
cfg_showdidn't need to exist.
Because they both fill different role. You might want to add your own "doc cfg" information to either override what auto_doc_cfg generates or add your own information.
A couple more questions.
- Is
#[doc(cfg)]currently allowed on an item without#[cfg]? What are the semantics?
The RFC says:
This attribute provides a standardized format to override
#[cfg()]attributes to document conditionally available items. ... This attribute has the same syntax as conditional compilation
So the semantics are covered I think. The RFC didn't make it clear it works with and without cfg on the item though, so I'll add a clarification.
I'd expect this to be allowed making
cfg_showredundant as in the previous comment.Is it possible to show a cfg on an item, but hide it on all children of that item? (Without putting attributes on all those children individually.)
I think I see why you mention it. I suppose you talk a case like:
#[cfg(windows)]
mod module {
#![doc(cfg_hide(windows))]
fn item() {}
}
In this case, since the doc(cfg_hide(windows)) is also an attribute of module, it should impact module. I think we should say that inner doc(cfg_hide) and doc(cfg_show) attributes don't impact the item if they are inner attributes.
So only remains the question:
#[cfg(windows)]
mod module {
#![doc(cfg_hide(windows))]
fn item() {}
}
How to allow cfg(windows) to be displayed on module while having it hidden on its children? I think what the RFC offer is good enough:
#[cfg(windows)]
#[doc(cfg(windows)] // Will be displayed all the time
#[doc(cfg_hide(windows))] // Hides `windows` from `doc(auto_cfg)` rendering on `module` and its children
pub mod module {
#[cfg(windows)] // Not displayed because of `cfg_hide(windows)`!
pub fn item() {}
}
So I'd be willing to not change anything in this regard.
Just wanted to make sure that my comment above (https://github.com/rust-lang/rfcs/pull/3631#pullrequestreview-2048876838) wasn't missed, as there has been no response or acknowledgement :slightly_smiling_face:
I don't understand your comments. Do you have an example of what you would want to do?
For an overly simplified case, see https://github.com/jhpratt/doc-cfg-demo
Running cargo +nightly doc --all-features on alpha will incorrectly say that alpha::S::foo is "Available on crate feature foo-method only." I hadn't checked this in a while, but previously it would have no annotation whatsoever. Ideally, the feature resolver would be utilized to show the correct annotation — it relies on the feature beta-foo-method.
This situation is quite common with façade crates and is the sole reason I have not migrated large amounts of code from time to time-core. Were I to do this, I could remove ~1000 LOC (off the top of my head) from time-macros that is currently duplicated.
Okay so if I understand correctly what you say, reexported items should only show cfgs from the reexport's point of view (so to speak)? That sounds reasonable. Once you confirm it, I'll update the RFC.
Effectively, yes. My point of view is "what does a consumer of the library care about". That would mean showing them what feature they need to enable such that the item is present, taking into account feature transitivity, etc.
I added an example but normally it was covered. For the current case, this is a bug that should be solved.
As long as we agree on intended behavior, and it seems we do, then I'm satisfied. As I'd said above, the behavior had changed since I originally checked; it previously had no annotation.
Feel free to use that example as a rustdoc test whenever that gets fixed, by the way. No credit necessary.
Team member @GuillaumeGomez has proposed to merge this. The next step is review by the rest of the tagged team members:
- [x] @GuillaumeGomez
- [x] @Manishearth
- [x] @Nemo157
- [ ] @aDotInTheVoid
- [x] @camelid
- [x] @fmease
- [ ] @jsha
- [x] @notriddle
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
Friendly ping @Manishearth, @Nemo157, @aDotInTheVoid, @camelid, @jsha, this is blocking progress on stabilizing #[doc(cfg(...))] etc.
I don't think my old concern that auto_cfg and cfg_show are confusing since they are new terms using cfg in different spots has been addressed.
I'm not going to block the RFC on that, but I don't want to approve it myself. If everyone else is fine with it that's fine, but I would like to see if the overall api surface can be made easier to remember.
I applied suggestions from @Manishearth about the hide/show attributes: since they're only applied on the auto_cfg feature cfgs, it makes more sense to embed them in the auto_cfg attribute (imo).
I also added an explanation about why cfged out items are not included by rustdoc.
@GuillaumeGomez nice! Should the syntax for auto_cfg then be doc(auto_cfg) and doc(auto_cfg = false) (with = true being accepted but not the primary documented syntax)?
You mean for doc(auto_cfg) to be equivalent to doc(auto_cfg = true), making the = true optional?
@GuillaumeGomez yes, so basically this means:
doc(auto_cfg)= auto ondoc(auto_cfg(show(..))= auto on, and show stuffdoc(auto_cfg = false)= turn it offdoc(auto_cfg = true)= auto on, but not the primary documented syntax
the idea is that this way we have consistency: if you mention auto_cfg it enables it, you don't have to explicitly put = true if you're using show and can use a single invocation.
Not a super big fan of the implicit but it also kinda makes sense. Does it also include doc(auto_cfg(hide(...)))? Following the logic "if auto_cfg without = is used, it's enabled", it'd mean that auto_cfg(hide) would also enable auto_cfg. I'm fine with it but just want to confirm with you before updating the RFC.
Yeah, hide doesn't make any sense without auto_cfg.
I mostly don't want to mix the = and the () syntax too much, and this way most normal users only use the () syntax
At the very least, doc(auto_cfg) should work. My whole thrust here is to make the syntax relatively uniform and memorable.
Added the new syntax (allowing to have no argument in auto_cfg) and also the new behaviour for auto_cfg(hide(...)) and auto_cfg(show(...)).
Thanks for the ideas!