From cdaa7abf7e4593c8a43547130e34bb52f3e64000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Sun, 19 Jan 2014 09:05:48 +0100
Subject: [PATCH] value flow: better handling of conditional code below || &&
 ?: operators

---
 lib/valueflow.cpp      | 34 ++++++++++++++++++++++++++++------
 test/testvalueflow.cpp | 14 ++++++++++++++
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 85c203331..be7194ece 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -25,6 +25,7 @@
 #include "tokenlist.h"
 
 #include <iostream>
+#include <stack>
 
 
 static void printvalues(const Token *tok)
@@ -84,14 +85,35 @@ static bool bailoutFunctionPar(const Token *tok, const ValueFlow::Value &value,
     return arg && !arg->isConst() && arg->isReference();
 }
 
-static const Token * skipValueInConditionalExpression(const Token *tok)
+/**
+ * Should value be skipped because it's hidden inside && || or ?: expression.
+ * Example: ((x!=NULL) && (*x == 123))
+ * If 'valuetok' points at the x in '(*x == 123)'. Then the '&&' will be returned.
+ * @param valuetok original variable token
+ * @return NULL=>don't skip, non-NULL=>The operator token that cause the skip. For instance the '&&'.
+ * */
+static const Token * skipValueInConditionalExpression(const Token * const valuetok)
 {
-    while (tok && !Token::Match(tok, "%oror%|&&|?|:")) {
-        while (Token::Match(tok->astParent(), "%oror%|&&|?") && tok->astParent()->astOperand1() == tok)
-            tok = tok->astParent();
-        tok = tok->astParent();
+    // Walk up the ast
+    for (const Token *tok = valuetok->astParent(); tok; tok = tok->astParent()) {
+        if (!Token::Match(tok, "%oror%|&&|?|:"))
+            continue;
+
+        // Is variable protected in LHS..
+        std::stack<const Token *> tokens;
+        tokens.push(tok->astOperand1());
+        while (!tokens.empty()) {
+            const Token * const tok2 = tokens.top();
+            tokens.pop();
+            if (!tok2)
+                continue;
+            if (tok2 != valuetok && tok2->str() == valuetok->str())
+                return tok;
+            tokens.push(tok2->astOperand2());
+            tokens.push(tok2->astOperand1());
+        }
     }
-    return tok;
+    return NULL;
 }
 
 static bool bailoutSelfAssignment(const Token * const tok)
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index caf803614..6896d8501 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -340,6 +340,20 @@ private:
     }
 
     void valueFlowBeforeConditionIfElse() { // bailout: if/else/etc
+        const char *code;
+
+        code = "void f(X * x) {\n"
+               "  a = x;\n"
+               "  if ((x != NULL) &&\n"
+               "      (a(x->name, html)) &&\n"
+               "      (a(x->name, body))) {}\n"
+               "  if (x != NULL) { }\n"
+               "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 2U, 0));
+        TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 3U, 0));
+        ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
+        ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
+
         bailout("void f(int x) {\n"
                 "    if (x != 123) { b = x; }\n"
                 "    if (x == 123) {}\n"