From e6255356d2e4da2072f587b4fda559f775ad13d2 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 30 Aug 2021 22:26:35 +0200 Subject: [PATCH] Fix DNF construction, add proptest --- Cargo.lock | 22 ++++++++++++++++++++++ crates/cfg/Cargo.toml | 2 ++ crates/cfg/src/cfg_expr.rs | 15 +++++++++++++++ crates/cfg/src/dnf.rs | 33 +++++++++++++++++++++++++++++---- crates/cfg/src/tests.rs | 31 +++++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3854c35cae..057d23cd28 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -47,6 +47,15 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33954243bd79057c2de7338850b85983a44588021f8a5fee574a8888c6de4344" +[[package]] +name = "arbitrary" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "577b08a4acd7b99869f863c50011b01eb73424ccc798ecd996f2e24817adfca7" +dependencies = [ + "derive_arbitrary", +] + [[package]] name = "arrayvec" version = "0.7.1" @@ -147,8 +156,10 @@ checksum = "e70cc2f62c6ce1868963827bd677764c62d07c3d9a3e1fb1177ee1a9ab199eb2" name = "cfg" version = "0.0.0" dependencies = [ + "arbitrary", "expect-test", "mbe", + "oorandom", "rustc-hash", "syntax", "tt", @@ -291,6 +302,17 @@ dependencies = [ "num_cpus", ] +[[package]] +name = "derive_arbitrary" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b24629208e87a2d8b396ff43b15c4afb0a69cea3fbbaa9ed9b92b7c02f0aed73" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "dissimilar" version = "1.0.2" diff --git a/crates/cfg/Cargo.toml b/crates/cfg/Cargo.toml index f19354adde..4a0fdd30e0 100644 --- a/crates/cfg/Cargo.toml +++ b/crates/cfg/Cargo.toml @@ -17,3 +17,5 @@ tt = { path = "../tt", version = "0.0.0" } mbe = { path = "../mbe" } syntax = { path = "../syntax" } expect-test = "1.1" +arbitrary = { version = "1", features = ["derive"] } +oorandom = "11" diff --git a/crates/cfg/src/cfg_expr.rs b/crates/cfg/src/cfg_expr.rs index 8a1a51e6e7..a06f0f4480 100644 --- a/crates/cfg/src/cfg_expr.rs +++ b/crates/cfg/src/cfg_expr.rs @@ -50,6 +50,7 @@ impl fmt::Display for CfgAtom { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[cfg_attr(test, derive(arbitrary::Arbitrary))] pub enum CfgExpr { Invalid, Atom(CfgAtom), @@ -128,3 +129,17 @@ fn next_cfg_expr(it: &mut SliceIter) -> Option { } Some(ret) } + +#[cfg(test)] +impl arbitrary::Arbitrary<'_> for CfgAtom { + fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result { + match u.int_in_range(0..=1)? { + 0 => Ok(CfgAtom::Flag(String::arbitrary(u)?.into())), + 1 => Ok(CfgAtom::KeyValue { + key: String::arbitrary(u)?.into(), + value: String::arbitrary(u)?.into(), + }), + _ => unreachable!(), + } + } +} diff --git a/crates/cfg/src/dnf.rs b/crates/cfg/src/dnf.rs index 75ded9aa1f..0ae685a7d0 100644 --- a/crates/cfg/src/dnf.rs +++ b/crates/cfg/src/dnf.rs @@ -255,11 +255,11 @@ impl Builder { fn make_dnf(expr: CfgExpr) -> CfgExpr { match expr { CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::Not(_) => expr, - CfgExpr::Any(e) => CfgExpr::Any(e.into_iter().map(make_dnf).collect()), + CfgExpr::Any(e) => flatten(CfgExpr::Any(e.into_iter().map(make_dnf).collect())), CfgExpr::All(e) => { - let e = e.into_iter().map(make_nnf).collect::>(); + let e = e.into_iter().map(make_dnf).collect::>(); - CfgExpr::Any(distribute_conj(&e)) + flatten(CfgExpr::Any(distribute_conj(&e))) } } } @@ -289,7 +289,7 @@ fn distribute_conj(conj: &[CfgExpr]) -> Vec { } } - let mut out = Vec::new(); + let mut out = Vec::new(); // contains only `all()` let mut with = Vec::new(); go(&mut out, &mut with, conj); @@ -318,3 +318,28 @@ fn make_nnf(expr: CfgExpr) -> CfgExpr { }, } } + +/// Collapses nested `any()` and `all()` predicates. +fn flatten(expr: CfgExpr) -> CfgExpr { + match expr { + CfgExpr::All(inner) => CfgExpr::All( + inner + .into_iter() + .flat_map(|e| match e { + CfgExpr::All(inner) => inner, + _ => vec![e], + }) + .collect(), + ), + CfgExpr::Any(inner) => CfgExpr::Any( + inner + .into_iter() + .flat_map(|e| match e { + CfgExpr::Any(inner) => inner, + _ => vec![e], + }) + .collect(), + ), + _ => expr, + } +} diff --git a/crates/cfg/src/tests.rs b/crates/cfg/src/tests.rs index ea04114d3a..bdc3f854e0 100644 --- a/crates/cfg/src/tests.rs +++ b/crates/cfg/src/tests.rs @@ -1,3 +1,4 @@ +use arbitrary::{Arbitrary, Unstructured}; use expect_test::{expect, Expect}; use mbe::syntax_node_to_token_tree; use syntax::{ast, AstNode}; @@ -130,6 +131,18 @@ fn nested() { check_dnf("#![cfg(not(all(all(a, b))))]", expect![[r#"#![cfg(any(not(a), not(b)))]"#]]); } +#[test] +fn regression() { + check_dnf("#![cfg(all(not(not(any(any(any()))))))]", expect![[r##"#![cfg(any())]"##]]); + check_dnf("#![cfg(all(any(all(any()))))]", expect![[r##"#![cfg(any())]"##]]); + check_dnf("#![cfg(all(all(any())))]", expect![[r##"#![cfg(any())]"##]]); + + check_dnf("#![cfg(all(all(any(), x)))]", expect![[r##"#![cfg(any())]"##]]); + check_dnf("#![cfg(all(all(any()), x))]", expect![[r##"#![cfg(any())]"##]]); + check_dnf("#![cfg(all(all(any(x))))]", expect![[r##"#![cfg(x)]"##]]); + check_dnf("#![cfg(all(all(any(x), x)))]", expect![[r##"#![cfg(all(x, x))]"##]]); +} + #[test] fn hints() { let mut opts = CfgOptions::default(); @@ -191,3 +204,21 @@ fn why_inactive() { expect![["test and test2 are enabled and a is disabled"]], ); } + +#[test] +fn proptest() { + const REPEATS: usize = 512; + + let mut rng = oorandom::Rand32::new(123456789); + let mut buf = Vec::new(); + for _ in 0..REPEATS { + buf.clear(); + while buf.len() < 512 { + buf.extend(rng.rand_u32().to_ne_bytes()); + } + + let mut u = Unstructured::new(&buf); + let cfg = CfgExpr::arbitrary(&mut u).unwrap(); + DnfExpr::new(cfg); + } +}