RFC for doc_cfg, doc_cfg_auto, doc_cfg_hide and doc_cfg_show features

[GuillaumeGomez]

tracking issue

cc @rust-lang/rustdoc

Rendered

[Nemo157]

Do we want to change this default in existing code, or maybe this should be edition tied?

[GuillaumeGomez]

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.

[Nemo157]

The current implementation only allows these attributes at the crate-level, has there been a push to support this arbitrary nesting from somewhere?

[Nemo157]

(Well, actually the attributes don't exist, feature(doc_auto_cfg) implies default-active doc(auto_cfg), but IIRC previous discussions about how to expose a stable toggle had only considered adding in the attribute toggling it at the crate-level).

[GuillaumeGomez]

The current implementation only allows these attributes at the crate-level, has there been a push to support this arbitrary nesting from somewhere?

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 doc(cfg()). For this I'm thinking mostly about if you have "parent" features with a lot of children and for some reasons you have a complex cfg to handle it and want to simplify its display when rendered in documentation.

[Nemo157]

#[doc(cfg(...))] already overrides #[cfg(...)] on a single item, you don't need to disable the auto-cfg for that.

The use case where you would want to disable doc(auto_cfg) on a sub-tree seems to be if you have a lot of cfg on sub-items of that tree that you don't want rendered at all (but these cfg aren't ones you want to globally #[doc(cfg_hide(...))]), and so by disabling it you get to avoid putting #[doc(cfg())] on each sub-item individually. But that seems like an unlikely situation to me.

Similarly wanting to #[cfg_hide(...)] some cfg within just a sub-tree seems very unlikely,

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: #[doc(no_auto_cfg)] (assuming the default is active) and #[doc(cfg_hide(...))].

[GuillaumeGomez]

Fair point. I'll limit doc(auto_cfg) as a crate-level attribute.

For cfg_hide I'm less convinced as you might want to hide some cfgs only for some modules/items (this is very hypothetical though). As for the complexity, it seems pretty low in a visitor: we add new entries before iterating to the children and remove them once we leave the current item (but that's just an implementation detail, not sure it's something that we should take into account here).

Anyway, if no one seems to be interested into having cfg_show/cfg_hide to be used on a specific module/item, then I will turn them into crate-level attribute (well, cfg_show would be useless in this case so it'll be simply removed).

[GuillaumeGomez]

I just thought about a use case where having doc(auto_cfg) not only as a crate-level attribute would be a nice improvement! In sysinfo, I disabled the doc_auto_cfg because every item is available, whatever the OS (even unsupported ones). Except that I recently splitted the components to enable them through features. So I can either do:

#![doc(auto_cfg = true)]
#![doc(cfg_hide(/* list of all supported OSes */)]

Or instead disable auto_cfg at the crate level and enable it on the components module directly without needing to worry about hiding cfgs since the OS information handling is one level upper.

So with this in mind, I think we should lift the crate-level only restriction on auto_cfg. What do you think?

[Nemo157]

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?

[GuillaumeGomez]

The one case I'm thinking about is to reduce complexity of rendered doc cfg as described in this comment.

[clarfonthey]

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.

[jhpratt]

Was any thought given to what I mentioned in the draft RFC? Correctly annotating re-exports from a different crate is something that would be very useful for façade crates and similar, and is the sole blocker for refactoring time to be split along more sensible lines. That would reduce code duplication a significant amount.

[Manishearth]

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 #[doc(cfg(..))] is fine, but it would be nice if the rest all derived from each other.

Perhaps #[doc(auto_cfg(disable))], #[doc(auto_cfg(show(...), hide(...))] etc? Ideally we only introduced one new doc attribute but I'm not sure if that's easy since cfg can accept anything. Potentially could use the distinction between cfg(..) and cfg = ... here. Thoughts?

[clarfonthey]

I personally wouldn't like using cfg = ... since it would just make the distinction more confusing. I'm not really sure there's a massive benefit to having doc(auto_cfg(show(...))) versus doc(cfg_show(...)) since, while it is literally combining two attributes into one, it just feels like unnecessary extra typing to me.

That said, perhaps there would be a case for having doc(auto_cfg(...)) for doc(cfg_show(...)) and doc(auto_cfg(not(...))) for doc(cfg_hide(...)), since it would fit in with existing cfg and be a bit more discoverable. Perhaps doc(auto_cfg(all)) and doc(auto_cfg(none)) could be allowed as aliases for enabling/disabling all options.

Still has some issues, but, if we were to change the existing attributes, I'd prefer something like that over just combining the show and hide into a single auto_cfg attribute.

[Manishearth]

@clarfonthey point taken on subtle syntax, I didn't really want to propose that anyway which is why I proposed it coming out of auto_cfg

To me the main distinction between cfg_show and auto_cfg(show) is the latter is easier to remember: there are many potential names for cfg_show and I won't remember which one it is. Especially since auto_cfg has cfg as the second word. It's inconsistent and confusing.

If everything trees out of a single attribute it's easier to remember IMO, and not confusing.

[Manishearth]

And yes I'm happy with experimentation with auto_cfg, I didn't want to propose one particular solution there just want it to be something straightforward, intuitive, and easy to remember.

[clarfonthey]

I'm not exactly convinced that auto_cfg(show(...)) is easier to remember compared to cfg_show(...), and in both cases, IDEs can offer suggestions after typing doc(cfg to let you know what's available.

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, auto_cfg(show(...)) and auto_cfg(hide(...)) are still two attributes; they just require extra parentheses compared to cfg_show(...) and cfg_hide(...).

[Nemo157]

Another option could be #[doc(auto_cfg_hide(...))] then.

Though, I'm now wondering whether there's a way to change the semantics slightly so that cfg_hide can take over the role of no_auto_cfg too. Instead of disabling auto_cfg completely just tell it to hide everything so it doesn't actually do anything. (Combined with my discussion above of restricting this to the crate-root that means there would only be two new attributes required in this RFC: #[doc(cfg(...))] anywhere and #![doc(cfg_hide(...))] at the root).

[GuillaumeGomez]

I don't understand what you suggested @Nemo157. Do you mean that if no argument is provided to cfg_hide, it means that we hide everything?

So to summarize the discussion, it seems that we tend to for show and hide:

#[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)]

?

[clarfonthey]

My understanding of the issue, after completely misunderstanding for a while (apologies, @Manishearth) is that the cfg_show and cfg_hide attributes make no reference to auto_cfg and so it's not clear that they control automatically showing and hiding cfg attributes. Manish' proposal of auto_cfg(show(...)) and auto_cfg(hide(...)) is one way of accomplishing this, but auto_cfg_show and auto_cfg_hide attributes could also accomplish this.

[Nemo157]

I don't understand what you suggested @Nemo157. Do you mean that if no argument is provided to cfg_hide, it means that we hide everything?

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.

[Manishearth]

@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.

[GuillaumeGomez]

Applied first round of review comments. Thanks everyone!

[clarfonthey]

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 cfg(any(doc, ...)) trick for making auto_cfg work properly, since it's not immediately clear otherwise. Basically, conditional compilation is still applied before rustdoc runs, meaning you have to ensure that rustdoc can see code to document it. Then, because auto_cfg by default ignores cfg(doc), only the other cfgs are documented.

[GuillaumeGomez]

It was not ignored. You can see the RFC mentioning this here. If it's not clear enough, how would you word it?

[GuillaumeGomez]

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.