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

Rustdoc and rustc accept different cfg syntax #84437

Closed
jyn514 opened this issue Apr 22, 2021 · 2 comments · Fixed by #84442
Closed

Rustdoc and rustc accept different cfg syntax #84437

jyn514 opened this issue Apr 22, 2021 · 2 comments · Fixed by #84442
Assignees
Labels
C-bug Category: This is a bug. F-doc_cfg `#![feature(doc_cfg)]` requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 22, 2021

I tried this code:

#![feature(doc_cfg)]
#[doc(cfg(x, y))]
fn bar() {}

I expected to see this happen: An error, to match the behavior on #[cfg(x, y)]:

error: multiple `cfg` predicates are specified
 --> cfg2.rs:1:10
  |
1 | #[cfg(x, y)]
  |          ^

Instead, this happened: Rustdoc silently accepts the code.

Relevant code:

crate fn parse(cfg: &MetaItem) -> Result<Cfg, InvalidCfgError> {

I think the relevant rustc code is
fn in_cfg(&self, attrs: &[Attribute]) -> bool {

Meta

rustdoc --version: rustdoc 1.53.0-nightly (392ba2b 2021-04-17)

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. F-doc_cfg `#![feature(doc_cfg)]` requires-nightly This issue requires a nightly compiler in some way. labels Apr 22, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 22, 2021

Fixing this properly will be hard. Rustc doesn't store the cfg predicates, it evaluates them immediately and returns whether they're active in the current crate. I guess an ok solution for now is to throw lots of tests at it? I'd be worried we're missing things, though.

@petrochenkov do you have suggestions for unifying the parsing somehow? For reference, this is what rustdoc currently uses: https://doc.rust-lang.org/nightly/nightly-rustc/rustdoc/clean/cfg/enum.Cfg.html

enum Cfg {
    True,
    False,
    Cfg(Symbol, Option<Symbol>),
    Not(Box<Cfg>),
    Any(Vec<Cfg>),
    All(Vec<Cfg>),
}

It's ok for the enum to be slightly different, but it can't just be a boolean true/false because rustdoc needs to display it.

@jyn514
Copy link
Member Author

jyn514 commented Apr 22, 2021

Hmm, I wonder if it makes more sense to just validate the cfg once with in_cfg, ignoring the result, and then have rustdoc parse it again.

@jyn514 jyn514 self-assigned this Apr 22, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? `@petrochenkov`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? ``@petrochenkov``
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? ```@petrochenkov```
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? ````@petrochenkov````
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 5, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? `````@petrochenkov`````
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 6, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? ``````@petrochenkov``````
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 7, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? ```````@petrochenkov```````
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 7, 2021
Unify rustc and rustdoc parsing of `cfg()`

This extracts a new `parse_cfg` function that's used between both.

- Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)]
  #[doc(cfg(y))]`. Previously it would be completely ignored.
- Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)]
  #[doc(cfg(x))]`. Previously, the cfg would be ignored.
- Pass the cfg predicate through to rustc_expand to be validated

Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters.

Fixes rust-lang#84437.

r? ````````@petrochenkov````````
@bors bors closed this as completed in 0c8c21d May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-doc_cfg `#![feature(doc_cfg)]` requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants