Merge pull request #6 from sunshowers/fix-any-all

Fix nested any/all evaluation

src/error.rs

@@ -16,7 +16,7 @@ pub struct ParseError<'a> {
 #[derive(Debug, PartialEq)]
 pub enum Reason {
     /// not() takes exactly 1 predicate, unlike all() and any()
-    InvalidNot(u8),
+    InvalidNot(usize),
     /// The characters are not valid in an SDPX license expression
     InvalidCharacters,
     /// An opening parens was unmatched with a closing parens

src/expr/mod.rs

@@ -13,11 +13,15 @@ pub enum Func {
     /// `all()` with a comma separated list of configuration predicates. It
     /// is false if at least one predicate is false. If there are no predicates,
     /// it is true.
-    All,
+    ///
+    /// The associated `usize` is the number of predicates inside the `all()`.
+    All(usize),
     /// `any()` with a comma separated list of configuration predicates. It
     /// is true if at least one predicate is true. If there are no predicates,
     /// it is false.
-    Any,
+    ///
+    /// The associated `usize` is the number of predicates inside the `any()`.
+    Any(usize),
 }
 
 use crate::targets as targ;
@@ -192,26 +196,28 @@ impl Expression {
                     let pred = pred.to_pred(&self.original);
                     result_stack.push(eval_predicate(&pred));
                 }
-                ExprNode::Fn(Func::All) => {
+                ExprNode::Fn(Func::All(count)) => {
                     // all() with a comma separated list of configuration predicates.
                     // It is false if at least one predicate is false.
                     // If there are no predicates, it is true.
                     let mut result = true;
 
-                    while let Some(rs) = result_stack.pop() {
-                        result = result && rs;
+                    for _ in 0..*count {
+                        let r = result_stack.pop().unwrap();
+                        result = result && r;
                     }
 
                     result_stack.push(result);
                 }
-                ExprNode::Fn(Func::Any) => {
+                ExprNode::Fn(Func::Any(count)) => {
                     // any() with a comma separated list of configuration predicates.
                     // It is true if at least one predicate is true.
                     // If there are no predicates, it is false.
                     let mut result = false;
 
-                    while let Some(rs) = result_stack.pop() {
-                        result = result || rs;
+                    for _ in 0..*count {
+                        let r = result_stack.pop().unwrap();
+                        result = result || r;
                     }
 
                     result_stack.push(result);

src/expr/parser.rs

@@ -21,6 +21,7 @@ impl Expression {
         // the string before we start walking tokens
         let original = lexer.inner;
 
+        #[derive(Debug)]
         struct FuncAndSpan {
             func: Func,
             parens_index: usize,
@@ -248,8 +249,10 @@ impl Expression {
                 Token::All | Token::Any | Token::Not => match last_token {
                     None | Some(Token::OpenParen) | Some(Token::Comma) => {
                         let new_fn = match lt.token {
-                            Token::All => Func::All,
-                            Token::Any => Func::Any,
+                            // the 0 is a dummy value -- it will be substituted for the real
+                            // number of predicates in the `CloseParen` branch below.
+                            Token::All => Func::All(0),
+                            Token::Any => Func::Any(0),
                             Token::Not => Func::Not,
                             _ => unreachable!(),
                         };
@@ -284,16 +287,16 @@ impl Expression {
                             let key = pred_key.take();
                             let val = pred_val.take();
 
+                            let num_predicates = top.predicates.len()
+                                + if key.is_some() { 1 } else { 0 }
+                                + top.nest_level as usize;
+
                             let func = match top.func {
-                                Func::All => Func::All,
-                                Func::Any => Func::Any,
+                                Func::All(_) => Func::All(num_predicates),
+                                Func::Any(_) => Func::Any(num_predicates),
                                 Func::Not => {
                                     // not() doesn't take a predicate list, but only a single predicate,
                                     // so ensure we have exactly 1
-                                    let num_predicates = top.predicates.len() as u8
-                                        + if key.is_some() { 1 } else { 0 }
-                                        + top.nest_level;
-
                                     if num_predicates != 1 {
                                         return Err(ParseError {
                                             original,

tests/eval.rs

@@ -127,6 +127,17 @@ fn complex() {
     assert!(complex.eval(|pred| tg_match!(pred, windows_msvc)));
     assert!(complex.eval(|pred| tg_match!(pred, mac)));
     assert!(!complex.eval(|pred| tg_match!(pred, android)));
+
+    let complex = Expression::parse(r#"all(any(unix, target_arch="x86"), not(any(target_os="android", target_os="emscripten")))"#).unwrap();
+
+    // Should match linuxes and mac
+    assert!(complex.eval(|pred| tg_match!(pred, linux_gnu)));
+    assert!(complex.eval(|pred| tg_match!(pred, linux_musl)));
+    assert!(complex.eval(|pred| tg_match!(pred, mac)));
+
+    // Should *not* match x86_64 windows or android
+    assert!(!complex.eval(|pred| tg_match!(pred, windows_msvc)));
+    assert!(!complex.eval(|pred| tg_match!(pred, android)));
 }
 
 #[test]