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

add doc(cfg(...)) to feature-flagged impls #74

Closed
wants to merge 1 commit into from
Closed

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Nov 22, 2021

Currently, feature-flagged public APIs will not be documented as
requiring feature flags. This branch adds #[doc(cfg(...))] to all
feature-flagged APIs.

For the most part, this is done using the feature! macro lifted from
Tokio:
https://github.com/tokio-rs/tokio/blob/8943e8aeef0b33f371d6dc69f62b38da390b5d5f/tokio/src/macros/cfg.rs#L3-L14

In a couple of places, though, the feature-flagged thing is in a
position other than item position, so in that case, it was necessary to
just paste the attribute...

Signed-off-by: Eliza Weisman [email protected]

@hawkw hawkw requested review from carllerche and taiki-e November 22, 2021 20:14
Currently, feature-flagged public APIs will not be documented as
requiring feature flags. This branch adds `#[doc(cfg(...))]` to all
feature-flagged APIs.

For the most part, this is done using the `feature!` macro lifted from
Tokio:
https://github.com/tokio-rs/tokio/blob/8943e8aeef0b33f371d6dc69f62b38da390b5d5f/tokio/src/macros/cfg.rs#L3-L14

In a couple of places, though, the feature-flagged thing is in a
position other than item position, so in that case, it was necessary to
just paste the attribute...

Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw changed the base branch from eliza/cow-ify to master November 23, 2021 18:49
@davidbarsky
Copy link
Member

@hawkw I don't think that this is necessary anymore as per this tweet. @taiki-e's PR (tokio-rs/tokio#4268) removed these feature flags.

@hawkw
Copy link
Member Author

hawkw commented Nov 23, 2021

@hawkw I don't think that this is necessary anymore as per this tweet. @taiki-e's PR (tokio-rs/tokio#4268) removed these feature flags.

huh, i just built the docs on nightly yesterday and I thought I didn't see the feature flag docs, so I had thought that wasn't ready yet. let me double-check!

@hawkw
Copy link
Member Author

hawkw commented Nov 23, 2021

@davidbarsky hmm, i'm still not seeing them without the attribute, on today's nightly:
image

image

it looks like it worked for tokio, though. i wonder if the issue is that in valuable currently, the only things that are feature flagged are trait impls for foreign types; maybe RustDoc doesn't generate the cfg docs for those implicitly?

@hawkw
Copy link
Member Author

hawkw commented Nov 23, 2021

@davidbarsky OH i'm a dumbass, it turns out --- we do still need to enable the doc_cfg feature, we just don't need to add the attributes everywhere. lmao.

@hawkw
Copy link
Member Author

hawkw commented Nov 23, 2021

huh. I still can't get it to work without the attributes, even with #![feature(doc_cfg)] added unconditionally. I'm on last night's nightly build:

:; cargo +nightly rustdoc --  --version --verbose
 Documenting valuable v0.1.0 (/home/eliza/Code/valuable/valuable)
rustdoc 1.58.0-nightly (936f2600b 2021-11-22)
binary: rustdoc
commit-hash: 936f2600b6c903b04387f74ed5cbce88bb06d243
commit-date: 2021-11-22
host: x86_64-unknown-linux-gnu
release: 1.58.0-nightly
LLVM version: 13.0.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s

@davidbarsky
Copy link
Member

huh. I still can't get it to work without the attributes, even with #![feature(doc_cfg)] added unconditionally. I'm on last night's nightly build:

that's effin' weird. anyways, ignore me! please don't let me nerdsnipe you on this shit

@carllerche
Copy link
Member

Where we taking this? Moving forward or holding back?

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

This way does not seem to work in some places.
As an alternative, I filed #80 that uses the way used in the tweet @davidbarsky mentioned.

Comment on lines +124 to 132
feature! {
#![feature = "alloc"]

deref! {
alloc::boxed::Box<T>,
alloc::rc::Rc<T>,
#[cfg(not(valuable_no_atomic_cas))]
alloc::sync::Arc<T>,
}
Copy link
Member

@taiki-e taiki-e Dec 29, 2021

Choose a reason for hiding this comment

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

feature! macro doesn't seem to work on nested macro.

doc

@taiki-e taiki-e closed this in #80 Dec 29, 2021
@taiki-e taiki-e deleted the eliza/doc-cfg branch August 19, 2023 00:54
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.

4 participants