Skip to content

Commit

Permalink
Auto merge of rust-lang#17657 - Veykril:cfg-slim, r=lnicola
Browse files Browse the repository at this point in the history 
internal: Make `CfgExpr` slimmer
  • Loading branch information
@bors
bors committed Jul 21, 2024
2 parents 9e3482b + 272e595 commit 402e176
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 64 deletions.
16 changes: 8 additions & 8 deletions src/tools/rust-analyzer/crates/cfg/src/cfg_expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ impl fmt::Display for CfgAtom {
pub enum CfgExpr {
Invalid,
Atom(CfgAtom),
All(Vec<CfgExpr>),
Any(Vec<CfgExpr>),
All(Box<[CfgExpr]>),
Any(Box<[CfgExpr]>),
Not(Box<CfgExpr>),
}

Expand Down Expand Up @@ -90,12 +90,12 @@ fn next_cfg_expr<S>(it: &mut std::slice::Iter<'_, tt::TokenTree<S>>) -> Option<C
Some(tt::TokenTree::Subtree(subtree)) => {
it.next();
let mut sub_it = subtree.token_trees.iter();
let mut subs = std::iter::from_fn(|| next_cfg_expr(&mut sub_it)).collect();
match &name {
s if *s == sym::all => CfgExpr::All(subs),
s if *s == sym::any => CfgExpr::Any(subs),
s if *s == sym::not => {
CfgExpr::Not(Box::new(subs.pop().unwrap_or(CfgExpr::Invalid)))
let mut subs = std::iter::from_fn(|| next_cfg_expr(&mut sub_it));
match name {
s if s == sym::all => CfgExpr::All(subs.collect()),
s if s == sym::any => CfgExpr::Any(subs.collect()),
s if s == sym::not => {
CfgExpr::Not(Box::new(subs.next().unwrap_or(CfgExpr::Invalid)))
}
_ => CfgExpr::Invalid,
}
Expand Down
79 changes: 42 additions & 37 deletions src/tools/rust-analyzer/crates/cfg/src/dnf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct Literal {
}

impl DnfExpr {
pub fn new(expr: CfgExpr) -> Self {
pub fn new(expr: &CfgExpr) -> Self {
let builder = Builder { expr: DnfExpr { conjunctions: Vec::new() } };

builder.lower(expr)
Expand Down Expand Up @@ -154,9 +154,9 @@ impl fmt::Display for DnfExpr {
}

impl Conjunction {
fn new(parts: Vec<CfgExpr>) -> Self {
fn new(parts: Box<[CfgExpr]>) -> Self {
let mut literals = Vec::new();
for part in parts {
for part in parts.into_vec() {
match part {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::Not(_) => {
literals.push(Literal::new(part));
Expand Down Expand Up @@ -232,27 +232,28 @@ struct Builder {
}

impl Builder {
fn lower(mut self, expr: CfgExpr) -> DnfExpr {
fn lower(mut self, expr: &CfgExpr) -> DnfExpr {
let expr = make_nnf(expr);
let expr = make_dnf(expr);

match expr {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::Not(_) => {
self.expr.conjunctions.push(Conjunction::new(vec![expr]));
self.expr.conjunctions.push(Conjunction::new(Box::new([expr])));
}
CfgExpr::All(conj) => {
self.expr.conjunctions.push(Conjunction::new(conj));
}
CfgExpr::Any(mut disj) => {
CfgExpr::Any(disj) => {
let mut disj = disj.into_vec();
disj.reverse();
while let Some(conj) = disj.pop() {
match conj {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::All(_) | CfgExpr::Not(_) => {
self.expr.conjunctions.push(Conjunction::new(vec![conj]));
self.expr.conjunctions.push(Conjunction::new(Box::new([conj])));
}
CfgExpr::Any(inner_disj) => {
// Flatten.
disj.extend(inner_disj.into_iter().rev());
disj.extend(inner_disj.into_vec().into_iter().rev());
}
}
}
Expand All @@ -266,11 +267,11 @@ impl Builder {
fn make_dnf(expr: CfgExpr) -> CfgExpr {
match expr {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::Not(_) => expr,
CfgExpr::Any(e) => flatten(CfgExpr::Any(e.into_iter().map(make_dnf).collect())),
CfgExpr::Any(e) => flatten(CfgExpr::Any(e.into_vec().into_iter().map(make_dnf).collect())),
CfgExpr::All(e) => {
let e = e.into_iter().map(make_dnf).collect::<Vec<_>>();
let e = e.into_vec().into_iter().map(make_dnf).collect::<Vec<_>>();

flatten(CfgExpr::Any(distribute_conj(&e)))
flatten(CfgExpr::Any(distribute_conj(&e).into_boxed_slice()))
}
}
}
Expand All @@ -281,7 +282,7 @@ fn distribute_conj(conj: &[CfgExpr]) -> Vec<CfgExpr> {
match rest {
[head, tail @ ..] => match head {
CfgExpr::Any(disj) => {
for part in disj {
for part in disj.iter() {
with.push(part.clone());
go(out, with, tail);
with.pop();
Expand All @@ -295,7 +296,7 @@ fn distribute_conj(conj: &[CfgExpr]) -> Vec<CfgExpr> {
},
_ => {
// Turn accumulated parts into a new conjunction.
out.push(CfgExpr::All(with.clone()));
out.push(CfgExpr::All(with.clone().into_boxed_slice()));
}
}
}
Expand All @@ -308,25 +309,27 @@ fn distribute_conj(conj: &[CfgExpr]) -> Vec<CfgExpr> {
out
}

fn make_nnf(expr: CfgExpr) -> CfgExpr {
fn make_nnf(expr: &CfgExpr) -> CfgExpr {
match expr {
CfgExpr::Invalid | CfgExpr::Atom(_) => expr,
CfgExpr::Any(expr) => CfgExpr::Any(expr.into_iter().map(make_nnf).collect()),
CfgExpr::All(expr) => CfgExpr::All(expr.into_iter().map(make_nnf).collect()),
CfgExpr::Not(operand) => match *operand {
CfgExpr::Invalid | CfgExpr::Atom(_) => CfgExpr::Not(operand.clone()), // Original negated expr
CfgExpr::Not(expr) => {
// Remove double negation.
make_nnf(*expr)
}
// Convert negated conjunction/disjunction using DeMorgan's Law.
CfgExpr::Any(inner) => CfgExpr::All(
inner.into_iter().map(|expr| make_nnf(CfgExpr::Not(Box::new(expr)))).collect(),
),
CfgExpr::All(inner) => CfgExpr::Any(
inner.into_iter().map(|expr| make_nnf(CfgExpr::Not(Box::new(expr)))).collect(),
),
},
CfgExpr::Invalid | CfgExpr::Atom(_) => expr.clone(),
CfgExpr::Any(expr) => CfgExpr::Any(expr.iter().map(make_nnf).collect()),
CfgExpr::All(expr) => CfgExpr::All(expr.iter().map(make_nnf).collect()),
CfgExpr::Not(operand) => make_nnf_neg(operand),
}
}

fn make_nnf_neg(operand: &CfgExpr) -> CfgExpr {
match operand {
// Original negated expr
CfgExpr::Invalid => CfgExpr::Not(Box::new(CfgExpr::Invalid)), // Original negated expr
// Original negated expr
CfgExpr::Atom(atom) => CfgExpr::Not(Box::new(CfgExpr::Atom(atom.clone()))),
// Remove double negation.
CfgExpr::Not(expr) => make_nnf(expr),
// Convert negated conjunction/disjunction using DeMorgan's Law.
CfgExpr::Any(inner) => CfgExpr::All(inner.iter().map(make_nnf_neg).collect()),
// Convert negated conjunction/disjunction using DeMorgan's Law.
CfgExpr::All(inner) => CfgExpr::Any(inner.iter().map(make_nnf_neg).collect()),
}
}

Expand All @@ -335,20 +338,22 @@ fn flatten(expr: CfgExpr) -> CfgExpr {
match expr {
CfgExpr::All(inner) => CfgExpr::All(
inner
.into_iter()
.iter()
.flat_map(|e| match e {
CfgExpr::All(inner) => inner,
_ => vec![e],
CfgExpr::All(inner) => inner.as_ref(),
_ => std::slice::from_ref(e),
})
.cloned()
.collect(),
),
CfgExpr::Any(inner) => CfgExpr::Any(
inner
.into_iter()
.iter()
.flat_map(|e| match e {
CfgExpr::Any(inner) => inner,
_ => vec![e],
CfgExpr::Any(inner) => inner.as_ref(),
_ => std::slice::from_ref(e),
})
.cloned()
.collect(),
),
_ => expr,
Expand Down
36 changes: 22 additions & 14 deletions src/tools/rust-analyzer/crates/cfg/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn check_dnf(input: &str, expect: Expect) {
DocCommentDesugarMode::ProcMacro,
);
let cfg = CfgExpr::parse(&tt);
let actual = format!("#![cfg({})]", DnfExpr::new(cfg));
let actual = format!("#![cfg({})]", DnfExpr::new(&cfg));
expect.assert_eq(&actual);
}

Expand All @@ -43,7 +43,7 @@ fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) {
DocCommentDesugarMode::ProcMacro,
);
let cfg = CfgExpr::parse(&tt);
let dnf = DnfExpr::new(cfg);
let dnf = DnfExpr::new(&cfg);
let why_inactive = dnf.why_inactive(opts).unwrap().to_string();
expect.assert_eq(&why_inactive);
}
Expand All @@ -59,7 +59,7 @@ fn check_enable_hints(input: &str, opts: &CfgOptions, expected_hints: &[&str]) {
DocCommentDesugarMode::ProcMacro,
);
let cfg = CfgExpr::parse(&tt);
let dnf = DnfExpr::new(cfg);
let dnf = DnfExpr::new(&cfg);
let hints = dnf.compute_enable_hints(opts).map(|diff| diff.to_string()).collect::<Vec<_>>();
assert_eq!(hints, expected_hints);
}
Expand All @@ -82,20 +82,28 @@ fn test_cfg_expr_parser() {

assert_parse_result(
r#"#![cfg(all(foo, bar = "baz"))]"#,
CfgExpr::All(vec![
CfgAtom::Flag(Symbol::intern("foo")).into(),
CfgAtom::KeyValue { key: Symbol::intern("bar"), value: Symbol::intern("baz") }.into(),
]),
CfgExpr::All(
vec![
CfgAtom::Flag(Symbol::intern("foo")).into(),
CfgAtom::KeyValue { key: Symbol::intern("bar"), value: Symbol::intern("baz") }
.into(),
]
.into_boxed_slice(),
),
);

assert_parse_result(
r#"#![cfg(any(not(), all(), , bar = "baz",))]"#,
CfgExpr::Any(vec![
CfgExpr::Not(Box::new(CfgExpr::Invalid)),
CfgExpr::All(vec![]),
CfgExpr::Invalid,
CfgAtom::KeyValue { key: Symbol::intern("bar"), value: Symbol::intern("baz") }.into(),
]),
CfgExpr::Any(
vec![
CfgExpr::Not(Box::new(CfgExpr::Invalid)),
CfgExpr::All(Box::new([])),
CfgExpr::Invalid,
CfgAtom::KeyValue { key: Symbol::intern("bar"), value: Symbol::intern("baz") }
.into(),
]
.into_boxed_slice(),
),
);
}

Expand Down Expand Up @@ -235,6 +243,6 @@ fn proptest() {

let mut u = Unstructured::new(&buf);
let cfg = CfgExpr::arbitrary(&mut u).unwrap();
DnfExpr::new(cfg);
DnfExpr::new(&cfg);
}
}
6 changes: 3 additions & 3 deletions src/tools/rust-analyzer/crates/hir-expand/src/cfg_process.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ where
}
}
let group = match &name {
s if *s == sym::all => CfgExpr::All(preds),
s if *s == sym::any => CfgExpr::Any(preds),
s if *s == sym::all => CfgExpr::All(preds.into_boxed_slice()),
s if *s == sym::any => CfgExpr::Any(preds.into_boxed_slice()),
s if *s == sym::not => {
CfgExpr::Not(Box::new(preds.pop().unwrap_or(CfgExpr::Invalid)))
}
Expand Down Expand Up @@ -343,7 +343,7 @@ mod tests {
assert_eq!(node.syntax().text_range().start(), 0.into());

let cfg = parse_from_attr_meta(node.meta().unwrap()).unwrap();
let actual = format!("#![cfg({})]", DnfExpr::new(cfg));
let actual = format!("#![cfg({})]", DnfExpr::new(&cfg));
expect.assert_eq(&actual);
}
#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/inactive_code.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub(crate) fn inactive_code(
return None;
}

let inactive = DnfExpr::new(d.cfg.clone()).why_inactive(&d.opts);
let inactive = DnfExpr::new(&d.cfg).why_inactive(&d.opts);
let mut message = "code is inactive due to #[cfg] directives".to_owned();

if let Some(inactive) = inactive {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rust-analyzer/crates/rust-analyzer/src/target_spec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ fn required_features(cfg_expr: &CfgExpr, features: &mut Vec<String>) {
preds.iter().for_each(|cfg| required_features(cfg, features));
}
CfgExpr::Any(preds) => {
for cfg in preds {
for cfg in preds.iter() {
let len_features = features.len();
required_features(cfg, features);
if len_features != features.len() {
Expand Down

0 comments on commit 402e176

Please sign in to comment.