diff --git a/crates/pep508-rs/src/marker/simplify.rs b/crates/pep508-rs/src/marker/simplify.rs index c34581ceb00bd..d3338ed61b10e 100644 --- a/crates/pep508-rs/src/marker/simplify.rs +++ b/crates/pep508-rs/src/marker/simplify.rs @@ -193,13 +193,23 @@ fn simplify(dnf: &mut Vec>) { // 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; @@ -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 { diff --git a/crates/pep508-rs/src/marker/tree.rs b/crates/pep508-rs/src/marker/tree.rs index 9f93ec9b215e0..1939401a3686f 100644 --- a/crates/pep508-rs/src/marker/tree.rs +++ b/crates/pep508-rs/src/marker/tree.rs @@ -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( @@ -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')"