Skip to content

Conversation

@phil-opp
Copy link
Member

@phil-opp phil-opp commented Mar 4, 2023

This flag includes #[cfg] bounds in the generated documentation.

For example, the docs for the x86_64::structures::paging::mapper module now look like this:

image

We see that OffsetPageTable is now annotated with 64-bit because the module has #![cfg(target_pointer_width = "64")]. Similarly, the RecursivePageTable struct is now annotated with instructions because it is #[cfg(feature = "instructions")].

@phil-opp phil-opp requested review from Freax13 and josephlr March 7, 2023 08:40
@Freax13
Copy link
Member

Freax13 commented Mar 7, 2023

What's the relationship between doc_cfg and doc_auto_cfg? The nightly features enables the doc_cfg feature, should it also enable doc_auto_cfg?

Overall I really like this change.


The failing CI check is unrelated to this pr, though we should take a look at it nonetheless.

@phil-opp
Copy link
Member Author

phil-opp commented Mar 7, 2023

What's the relationship between doc_cfg and doc_auto_cfg?

See https://doc.rust-lang.org/rustdoc/unstable-features.html#doccfg-recording-what-platforms-or-features-are-required-for-code-to-be-present. The doc_cfg feature allows the same thing, but through manual annotations. Looks like we used in the past for annotating nightly-only macros that require inline assembly: 17ca2b0. Now that inline assembly is stable, we don't use that feature anymore, so I think we can remove it.

The nightly features enables the doc_cfg feature, should it also enable doc_auto_cfg?

It does make the generated docs nicer, but it increases the chance of nightly breakage. Given that cargo doc automatically resolves cfg flags when building the documentation of dependencies, the value for users of this crate would be limited. So I think it's better to keep it disabled by default.

@phil-opp
Copy link
Member Author

phil-opp commented Mar 7, 2023

I opened #408 to remove the doc_cfg feature.

Copy link
Member

@Freax13 Freax13 left a comment

Choose a reason for hiding this comment

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

What's the relationship between doc_cfg and doc_auto_cfg?

See https://doc.rust-lang.org/rustdoc/unstable-features.html#doccfg-recording-what-platforms-or-features-are-required-for-code-to-be-present. The doc_cfg feature allows the same thing, but through manual annotations. Looks like we used in the past for annotating nightly-only macros that require inline assembly: 17ca2b0. Now that inline assembly is stable, we don't use that feature anymore, so I think we can remove it.

The nightly features enables the doc_cfg feature, should it also enable doc_auto_cfg?

It does make the generated docs nicer, but it increases the chance of nightly breakage. Given that cargo doc automatically resolves cfg flags when building the documentation of dependencies, the value for users of this crate would be limited. So I think it's better to keep it disabled by default.

Thanks for the explanations!

@phil-opp phil-opp enabled auto-merge March 7, 2023 12:07
@phil-opp phil-opp merged commit 3168216 into master Mar 7, 2023
@phil-opp phil-opp deleted the doc_auto_cfg branch March 7, 2023 12:13
@Freax13 Freax13 mentioned this pull request Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants