From c5207350096aab8d4a4dae914e5fde75f874ce64 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Fri, 11 May 2018 03:22:06 -0500
Subject: [PATCH] Improve checking of size condition against empty to avoid FPs
 (#1213)

* Improve checking of size condition against empty to avoid FPs

* Add const and todo for reverse conditions
---
 lib/astutils.cpp       | 23 +++++++++++++++++++++--
 test/testcondition.cpp | 26 ++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index af0442619..cb0fa25c3 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -248,6 +248,24 @@ bool isDifferentKnownValues(const Token * const tok1, const Token * const tok2)
     return tok1->hasKnownValue() && tok2->hasKnownValue() && tok1->values() != tok2->values();
 }
 
+static bool isZeroBoundCond(const Token * const cond)
+{
+    if(cond == nullptr)
+        return false;
+    // Assume unsigned
+    // TODO: Handle reverse conditions
+    const bool isZero = cond->astOperand2()->getValue(0);
+    if (cond->str() == "==" || cond->str() == ">=")
+        return isZero;
+    if (cond->str() == "<=")
+        return true;
+    if (cond->str() == "<")
+        return !isZero;
+    if (cond->str() == ">")
+        return false;
+    return false;
+}
+
 bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure)
 {
     if (!cond1 || !cond2)
@@ -273,16 +291,17 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
             if (isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure))
                 return isDifferentKnownValues(cond1->astOperand1(), cond2->astOperand1());
         }
+        // TODO: Handle reverse conditions
         if (Library::isContainerYield(cond1, Library::Container::EMPTY, "empty") &&
             Library::isContainerYield(cond2->astOperand1(), Library::Container::SIZE, "size") &&
             cond1->astOperand1()->astOperand1()->varId() == cond2->astOperand1()->astOperand1()->astOperand1()->varId()) {
-            return !(cond2->str() == "==" && cond2->astOperand2()->getValue(0));
+            return !isZeroBoundCond(cond2);
         }
 
         if (Library::isContainerYield(cond2, Library::Container::EMPTY, "empty") &&
             Library::isContainerYield(cond1->astOperand1(), Library::Container::SIZE, "size") &&
             cond2->astOperand1()->astOperand1()->varId() == cond1->astOperand1()->astOperand1()->astOperand1()->varId()) {
-            return !(cond1->str() == "==" && cond1->astOperand2()->getValue(0));
+            return !isZeroBoundCond(cond1);
         }
     }
 
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index 2ac56b277..4ea80d75d 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -1851,6 +1851,12 @@ private:
         check("void f1(const std::string &s) { if(s.size() > 42) if(s.empty()) {}} ");
         ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
 
+        check("void f1(const std::string &s) { if(s.size() > 0) if(s.empty()) {}} ");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
+
+        check("void f1(const std::string &s) { if(s.size() < 0) if(s.empty()) {}} ");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
+
         check("void f1(const std::string &s) { if(s.empty()) if(s.size() > 42) {}} ");
         ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
 
@@ -1877,6 +1883,26 @@ private:
 
         check("void f1(const std::string v[10]) { if(v[0].size() > 42) if(v[1].empty()) {}} ");
         ASSERT_EQUALS("", errout.str());
+
+        check("void f1(const std::string &s) { if(s.size() <= 1) if(s.empty()) {}} ");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f1(const std::string &s) { if(s.size() <= 2) if(s.empty()) {}} ");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f1(const std::string &s) { if(s.size() < 2) if(s.empty()) {}} ");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f1(const std::string &s) { if(s.size() >= 0) if(s.empty()) {}} ");
+        ASSERT_EQUALS("", errout.str());
+
+        // TODO: These are identical condition since size cannot be negative
+        check("void f1(const std::string &s) { if(s.size() <= 0) if(s.empty()) {}} ");
+        ASSERT_EQUALS("", errout.str());
+
+        // TODO: These are identical condition since size cannot be negative
+        check("void f1(const std::string &s) { if(s.size() < 1) if(s.empty()) {}} ");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void identicalInnerCondition() {