Skip to content

Commit

Permalink
Rollup merge of rust-lang#77672 - Nemo157:simplify-cfg, r=jyn514
Browse files Browse the repository at this point in the history
Simplify doc-cfg rendering based on the current context

For sub-items on a page don't show cfg that has already been rendered on
a parent item. At its simplest this means not showing anything that is
shown in the portability message at the top of the page, but also for
things like fields of an enum variant if that variant itself is
cfg-gated then don't repeat those cfg on each field of the variant.

This does not touch trait implementation rendering, as that is more
complex and there are existing issues around how it deals with doc-cfg
that need to be fixed first.

### Screenshots, left is current, right is new:

![image](https://user-images.githubusercontent.com/81079/95387261-c2e6a200-08f0-11eb-90d4-0a9734acd922.png)

![image](https://user-images.githubusercontent.com/81079/95387458-06411080-08f1-11eb-81f7-5dd7f37695dd.png)

![image](https://user-images.githubusercontent.com/81079/95387702-6637b700-08f1-11eb-82f4-46b6cd9b24f2.png)

![image](https://user-images.githubusercontent.com/81079/95387905-b9aa0500-08f1-11eb-8d95-8b618d31d419.png)

![image](https://user-images.githubusercontent.com/81079/95388300-5bc9ed00-08f2-11eb-9ac9-b92cbdb60b89.png)

cc rust-lang#43781
  • Loading branch information
JohnTitor committed Oct 15, 2020
2 parents 72dbc60 + 4409cb2 commit 3cef494
Show file tree
Hide file tree
Showing 6 changed files with 334 additions and 52 deletions.
31 changes: 31 additions & 0 deletions src/librustdoc/clean/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,37 @@ impl Cfg {
_ => false,
}
}

/// Attempt to simplify this cfg by assuming that `assume` is already known to be true, will
/// return `None` if simplification managed to completely eliminate any requirements from this
/// `Cfg`.
///
/// See `tests::test_simplify_with` for examples.
pub(crate) fn simplify_with(&self, assume: &Cfg) -> Option<Cfg> {
if self == assume {
return None;
}

if let Cfg::All(a) = self {
let mut sub_cfgs: Vec<Cfg> = if let Cfg::All(b) = assume {
a.iter().filter(|a| !b.contains(a)).cloned().collect()
} else {
a.iter().filter(|&a| a != assume).cloned().collect()
};
let len = sub_cfgs.len();
return match len {
0 => None,
1 => sub_cfgs.pop(),
_ => Some(Cfg::All(sub_cfgs)),
};
} else if let Cfg::All(b) = assume {
if b.contains(self) {
return None;
}
}

Some(self.clone())
}
}

impl ops::Not for Cfg {
Expand Down
36 changes: 36 additions & 0 deletions src/librustdoc/clean/cfg/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,39 @@ fn test_render_long_html() {
);
})
}

#[test]
fn test_simplify_with() {
// This is a tiny subset of things that could be simplified, but it likely covers 90% of
// real world usecases well.
with_default_session_globals(|| {
let foo = word_cfg("foo");
let bar = word_cfg("bar");
let baz = word_cfg("baz");
let quux = word_cfg("quux");

let foobar = Cfg::All(vec![foo.clone(), bar.clone()]);
let barbaz = Cfg::All(vec![bar.clone(), baz.clone()]);
let foobarbaz = Cfg::All(vec![foo.clone(), bar.clone(), baz.clone()]);
let bazquux = Cfg::All(vec![baz.clone(), quux.clone()]);

// Unrelated cfgs don't affect each other
assert_eq!(foo.simplify_with(&bar).as_ref(), Some(&foo));
assert_eq!(foobar.simplify_with(&bazquux).as_ref(), Some(&foobar));

// Identical cfgs are eliminated
assert_eq!(foo.simplify_with(&foo), None);
assert_eq!(foobar.simplify_with(&foobar), None);

// Multiple cfgs eliminate a single assumed cfg
assert_eq!(foobar.simplify_with(&foo).as_ref(), Some(&bar));
assert_eq!(foobar.simplify_with(&bar).as_ref(), Some(&foo));

// A single cfg is eliminated by multiple assumed cfg containing it
assert_eq!(foo.simplify_with(&foobar), None);

// Multiple cfgs eliminate the matching subset of multiple assumed cfg
assert_eq!(foobar.simplify_with(&barbaz).as_ref(), Some(&foo));
assert_eq!(foobar.simplify_with(&foobarbaz), None);
});
}
Loading

0 comments on commit 3cef494

Please sign in to comment.