Fix #12036 FN knownConditionTrueFalse comparing enum with number

danmar · Oct 4, 2023 · ea167e5 · ea167e5
1 parent ca20152
commit ea167e5
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 7 deletions.
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -1403,10 +1403,10 @@ static inline bool isSameConstantValue(bool macro, const Token* tok1, const Toke
     };
 
     tok1 = adjustForCast(tok1);
-    if (!tok1->isNumber())
+    if (!tok1->isNumber() && !tok1->enumerator())
         return false;
     tok2 = adjustForCast(tok2);
-    if (!tok2->isNumber())
+    if (!tok2->isNumber() && !tok2->enumerator())
         return false;
 
     if (macro && (tok1->isExpandedMacro() || tok2->isExpandedMacro() || tok1->isTemplateArg() || tok2->isTemplateArg()))
@@ -1523,7 +1523,7 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
         return true;
 
     // Follow variable
-    if (followVar && !tok_str_eq && (tok1->varId() || tok2->varId())) {
+    if (followVar && !tok_str_eq && (tok1->varId() || tok2->varId() || tok1->enumerator() || tok2->enumerator())) {
         const Token * varTok1 = followVariableExpression(tok1, cpp, tok2);
         if ((varTok1->str() == tok2->str()) || isSameConstantValue(macro, varTok1, tok2)) {
             followVariableExpressionError(tok1, varTok1, errors);

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -2595,10 +2595,11 @@ void CheckOther::checkDuplicateExpression()
                     if (isWithoutSideEffects(cpp, tok->astOperand1())) {
                         const Token* loopTok = isInLoopCondition(tok);
                         if (!loopTok || !isExpressionChanged(tok, tok, loopTok->link()->next()->link(), mSettings, cpp)) {
-                            const bool assignment = tok->str() == "=";
+                            const bool isEnum = tok->scope()->type == Scope::eEnum;
+                            const bool assignment = !isEnum && tok->str() == "=";
                             if (assignment && warningEnabled)
                                 selfAssignmentError(tok, tok->astOperand1()->expressionString());
-                            else if (styleEnabled) {
+                            else if (styleEnabled && !isEnum) {
                                 if (cpp && mSettings->standards.cpp >= Standards::CPP11 && tok->str() == "==") {
                                     const Token* parent = tok->astParent();
                                     while (parent && parent->astParent()) {

diff --git a/test/testother.cpp b/test/testother.cpp
@@ -175,6 +175,7 @@ class TestOther : public TestFixture {
         TEST_CASE(duplicateExpression14); // #9871
         TEST_CASE(duplicateExpression15); // #10650
         TEST_CASE(duplicateExpression16); // #10569
+        TEST_CASE(duplicateExpression17); // #12036
         TEST_CASE(duplicateExpressionLoop);
         TEST_CASE(duplicateValueTernary);
         TEST_CASE(duplicateExpressionTernary); // #6391
@@ -6291,7 +6292,8 @@ class TestOther : public TestFixture {
               "    enum { Four = 4 };\n"
               "    if (Four == 4) {}"
               "}", nullptr, true, false);
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) The comparison 'Four == 4' is always true.\n",
+                      errout.str());
 
         check("void f() {\n"
               "    enum { Four = 4 };\n"
@@ -6310,7 +6312,8 @@ class TestOther : public TestFixture {
               "    enum { FourInEnumTwo = 4 };\n"
               "    if (FourInEnumOne == FourInEnumTwo) {}\n"
               "}", nullptr, true, false);
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (style) The comparison 'FourInEnumOne == FourInEnumTwo' is always true because 'FourInEnumOne' and 'FourInEnumTwo' represent the same value.\n",
+                      errout.str());
 
         check("void f() {\n"
               "    enum { FourInEnumOne = 4 };\n"
@@ -6853,6 +6856,19 @@ class TestOther : public TestFixture {
         ASSERT_EQUALS("[test.cpp:3]: (style) Same expression '!s[1]' found multiple times in chain of '||' operators.\n", errout.str());
     }
 
+    void duplicateExpression17() {
+        check("enum { E0 };\n" // #12036
+              "void f() {\n"
+              "    if (0 > E0) {}\n"
+              "    if (E0 > 0) {}\n"
+              "    if (E0 == 0) {}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) The comparison '0 > E0' is always false.\n"
+                      "[test.cpp:4]: (style) The comparison 'E0 > 0' is always false.\n"
+                      "[test.cpp:5]: (style) The comparison 'E0 == 0' is always true.\n",
+                      errout.str());
+    }
+
     void duplicateExpressionLoop() {
         check("void f() {\n"
               "    int a = 1;\n"