bendns' repos / rust-analyzer
Unnamed repository; edit this file 'description' to name the repository.
Merge #10091
10091: fix: fix "disjunction in conjunction" panic r=matklad a=jonas-schievink Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10073 The DNF construction code created expressions that were combined in a way that made us "forget" to make their contents valid DNF again. This PR fixes that by flattening nested `any(any())` and `all(all())` predicates. There was also a typo that led to a redundant call to `make_nnf` instead of the correct recursive call to `make_dnf` (but this didn't seem to break/fix anything). This also adds some light property testing, though I'm not really sure this is the best way to do it. Co-authored-by: Jonas Schievink <[email protected]>
Browse Source
bors[bot] 2021-08-31
parent 70dbf35 · parent 8c9de51 · commit 545cdf9
Diffstat
-rw-r--r--Cargo.lock20
-rw-r--r--crates/cfg/Cargo.toml6
-rw-r--r--crates/cfg/src/cfg_expr.rs15
-rw-r--r--crates/cfg/src/dnf.rs33
-rw-r--r--crates/cfg/src/tests.rs31
5 files changed, 101 insertions, 4 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 3854c35cae..1842bf46da 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -48,6 +48,12 @@ 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"
+
+[[package]]
name = "arrayvec"
version = "0.7.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -147,8 +153,11 @@ checksum = "e70cc2f62c6ce1868963827bd677764c62d07c3d9a3e1fb1177ee1a9ab199eb2"
name = "cfg"
version = "0.0.0"
dependencies = [
+ "arbitrary",
+ "derive_arbitrary",
"expect-test",
"mbe",
+ "oorandom",
"rustc-hash",
"syntax",
"tt",
@@ -292,6 +301,17 @@ dependencies = [
]
[[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"
source = "registry+https://github.com/rust-lang/crates.io-index"
diff --git a/crates/cfg/Cargo.toml b/crates/cfg/Cargo.toml
index f19354adde..132505fbf4 100644
--- a/crates/cfg/Cargo.toml
+++ b/crates/cfg/Cargo.toml
@@ -17,3 +17,9 @@ tt = { path = "../tt", version = "0.0.0" }
mbe = { path = "../mbe" }
syntax = { path = "../syntax" }
expect-test = "1.1"
+oorandom = "11"
+# We depend on both individually instead of using `features = ["derive"]` to microoptimize the
+# build graph: if the feature was enabled, syn would be built early on in the graph if `smolstr`
+# supports `arbitrary`. This way, we avoid feature unification.
+arbitrary = "1"
+derive_arbitrary = "1"
diff --git a/crates/cfg/src/cfg_expr.rs b/crates/cfg/src/cfg_expr.rs
index 8a1a51e6e7..2158fb9f2d 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(derive_arbitrary::Arbitrary))]
pub enum CfgExpr {
Invalid,
Atom(CfgAtom),
@@ -128,3 +129,17 @@ fn next_cfg_expr(it: &mut SliceIter<tt::TokenTree>) -> Option<CfgExpr> {
}
Some(ret)
}
+
+#[cfg(test)]
+impl arbitrary::Arbitrary<'_> for CfgAtom {
+ fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result<Self> {
+ if u.arbitrary()? {
+ Ok(CfgAtom::Flag(String::arbitrary(u)?.into()))
+ } else {
+ Ok(CfgAtom::KeyValue {
+ key: String::arbitrary(u)?.into(),
+ value: String::arbitrary(u)?.into(),
+ })
+ }
+ }
+}
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::<Vec<_>>();
+ let e = e.into_iter().map(make_dnf).collect::<Vec<_>>();
- CfgExpr::Any(distribute_conj(&e))
+ flatten(CfgExpr::Any(distribute_conj(&e)))
}
}
}
@@ -289,7 +289,7 @@ fn distribute_conj(conj: &[CfgExpr]) -> Vec<CfgExpr> {
}
}
- 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};
@@ -131,6 +132,18 @@ fn nested() {
}
#[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);
+ }
+}