Skip to content

Commit

Permalink
add tests for NP-complete marker simplification cases
Browse files Browse the repository at this point in the history
  • Loading branch information
ibraheemdev committed Aug 8, 2024
1 parent 20386bd commit f86e792
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 12 deletions.
14 changes: 12 additions & 2 deletions crates/pep508-rs/src/marker/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,23 @@ fn simplify(dnf: &mut Vec<Vec<MarkerExpression>>) {
// For example, `A or (not A and B)` can be simplified to `A or B`,
// eliminating the `not A` term.
if other_clause.iter().all(|term| {
// For the term to be redundant in this clause, the other clause can
// contain the negation of the term but not the term itself.
if term == skipped_term {
return false;
}
if is_negation(term, skipped_term) {
return true;
}

// TODO(ibraheem): if we intern variables we could reduce this
// from a linear search to an integer `HashSet` lookup
clause.contains(term) || is_negated(term, skipped_term)
clause
.iter()
.position(|x| x == term)
// If the term was already removed from this one, we cannot
// depend on it for further simplification.
.is_some_and(|i| !redundant_terms.contains(&i))
}) {
redundant_terms.push(skipped);
continue 'term;
Expand Down Expand Up @@ -308,7 +318,7 @@ where
}

/// Returns `true` if the LHS is the negation of the RHS, or vice versa.
fn is_negated(left: &MarkerExpression, right: &MarkerExpression) -> bool {
fn is_negation(left: &MarkerExpression, right: &MarkerExpression) -> bool {
match left {
MarkerExpression::Version { key, specifier } => {
let MarkerExpression::Version {
Expand Down
40 changes: 30 additions & 10 deletions crates/pep508-rs/src/marker/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,36 @@ mod test {

#[test]
fn test_complex_marker_simplification() {
// This expression should simplify to:
// `(implementation_name == 'pypy' and sys_platform != 'win32')
// or (sys_platform == 'win32' or os_name != 'nt')
// or (implementation != 'pypy' or os_name == 'nt')`
//
// However, simplifying this expression is NP-complete and requires an exponential
// algorithm such as Quine-McCluskey, which is not currently implemented.
assert_simplifies(
"(implementation_name == 'pypy' and sys_platform != 'win32')
or (implementation_name != 'pypy' and sys_platform == 'win32')
or (sys_platform == 'win32' and os_name != 'nt')
or (sys_platform != 'win32' and os_name == 'nt')",
"(os_name != 'nt' and sys_platform == 'win32') \
or (implementation_name != 'pypy' and os_name == 'nt') \
or (implementation_name == 'pypy' and os_name != 'nt') \
or (os_name == 'nt' and sys_platform != 'win32')",
);

// This is another case we cannot simplify fully, depending on the variable order.
// The expression is equivalent to `sys_platform == 'x' or (os_name == 'Linux' and platform_system == 'win32')`.
assert_simplifies(
"(os_name == 'Linux' and platform_system == 'win32')
or (os_name == 'Linux' and platform_system == 'win32' and sys_platform == 'a')
or (os_name == 'Linux' and platform_system == 'win32' and sys_platform == 'x')
or (os_name != 'Linux' and platform_system == 'win32' and sys_platform == 'x')
or (os_name == 'Linux' and platform_system != 'win32' and sys_platform == 'x')
or (os_name != 'Linux' and platform_system != 'win32' and sys_platform == 'x')",
"(os_name != 'Linux' and sys_platform == 'x') or (platform_system != 'win32' and sys_platform == 'x') or (os_name == 'Linux' and platform_system == 'win32')"
);

assert_simplifies("python_version > '3.7'", "python_version > '3.7'");

assert_simplifies(
Expand All @@ -2063,16 +2093,6 @@ mod test {
or (python_version == '3.8' and sys_platform == 'win32')",
);

assert_simplifies(
"(os_name == 'Linux' and platform_system == 'win32')
or (os_name == 'Linux' and platform_system == 'win32' and sys_platform == 'a')
or (os_name == 'Linux' and platform_system == 'win32' and sys_platform == 'x')
or (os_name != 'Linux' and platform_system == 'win32' and sys_platform == 'x')
or (os_name == 'Linux' and platform_system != 'win32' and sys_platform == 'x')
or (os_name != 'Linux' and platform_system != 'win32' and sys_platform == 'x')",
"sys_platform == 'x' or (os_name == 'Linux' and platform_system == 'win32')",
);

assert_simplifies(
"(implementation_name != 'pypy' and os_name == 'nt' and sys_platform == 'darwin') or (os_name == 'nt' and sys_platform == 'win32')",
"(implementation_name != 'pypy' and os_name == 'nt' and sys_platform == 'darwin') or (os_name == 'nt' and sys_platform == 'win32')"
Expand Down

0 comments on commit f86e792

Please sign in to comment.