Fix #13773 (False positive: should not warn when different ternary 2nd and 3rd expressions happens to have same value)

lib/checkother.cpp

@@ -2163,7 +2163,7 @@ static bool isConstant(const Token* tok) {
     return tok && (tok->isEnumerator() || Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL"));
 }
 
-static bool isConstStatement(const Token *tok, const Library& library, bool isNestedBracket = false)
+static bool isConstStatement(const Token *tok, const Library& library, bool platformIndependent, bool isNestedBracket = false)
 {
     if (!tok)
         return false;
@@ -2185,42 +2185,48 @@ static bool isConstStatement(const Token *tok, const Library& library, bool isNe
         tok2 = tok2->astParent();
     }
     if (Token::Match(tok, "&&|%oror%"))
-        return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2(), library);
+        return isConstStatement(tok->astOperand1(), library, platformIndependent) && isConstStatement(tok->astOperand2(), library, platformIndependent);
     if (Token::Match(tok, "!|~|%cop%") && (tok->astOperand1() || tok->astOperand2()))
         return true;
-    if (Token::Match(tok->previous(), "sizeof|alignof|noexcept|typeid (") && tok->previous()->isKeyword())
+    if (Token::simpleMatch(tok->previous(), "sizeof ("))
+        return !platformIndependent;
+    if (Token::Match(tok->previous(), "alignof|noexcept|typeid (") && tok->previous()->isKeyword())
         return true;
     if (isCPPCast(tok)) {
         if (Token::simpleMatch(tok->astOperand1(), "dynamic_cast") && Token::simpleMatch(tok->astOperand1()->linkAt(1)->previous(), "& >"))
             return false;
-        return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2(), library);
+        return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2(), library, platformIndependent);
     }
     if (tok->isCast() && tok->next() && tok->next()->isStandardType())
-        return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1(), library);
+        return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1(), library, platformIndependent);
     if (Token::simpleMatch(tok, "."))
-        return isConstStatement(tok->astOperand2(), library);
+        return isConstStatement(tok->astOperand2(), library, platformIndependent);
     if (Token::simpleMatch(tok, ",")) {
         if (tok->astParent()) // warn about const statement on rhs at the top level
-            return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2(), library);
+            return isConstStatement(tok->astOperand1(), library, platformIndependent) &&
+                   isConstStatement(tok->astOperand2(), library, platformIndependent);
 
         const Token* lml = previousBeforeAstLeftmostLeaf(tok); // don't warn about matrix/vector assignment (e.g. Eigen)
         if (lml)
             lml = lml->next();
         const Token* stream = lml;
         while (stream && Token::Match(stream->astParent(), ".|[|(|*"))
             stream = stream->astParent();
-        return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2(), library);
+        return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2(), library, platformIndependent);
     }
     if (Token::simpleMatch(tok, "?") && Token::simpleMatch(tok->astOperand2(), ":")) // ternary operator
-        return isConstStatement(tok->astOperand1(), library) && isConstStatement(tok->astOperand2()->astOperand1(), library) && isConstStatement(tok->astOperand2()->astOperand2(), library);
+        return isConstStatement(tok->astOperand1(), library, platformIndependent) &&
+               isConstStatement(tok->astOperand2()->astOperand1(), library, platformIndependent) &&
+               isConstStatement(tok->astOperand2()->astOperand2(), library, platformIndependent);
     if (isBracketAccess(tok) && isWithoutSideEffects(tok->astOperand1(), /*checkArrayAccess*/ true, /*checkReference*/ false)) {
         const bool isChained = succeeds(tok->astParent(), tok);
         if (Token::simpleMatch(tok->astParent(), "[")) {
             if (isChained)
-                return isConstStatement(tok->astOperand2(), library) && isConstStatement(tok->astParent(), library);
-            return isNestedBracket && isConstStatement(tok->astOperand2(), library);
+                return isConstStatement(tok->astOperand2(), library, platformIndependent) &&
+                       isConstStatement(tok->astParent(), library, platformIndependent);
+            return isNestedBracket && isConstStatement(tok->astOperand2(), library, platformIndependent);
         }
-        return isConstStatement(tok->astOperand2(), library, /*isNestedBracket*/ !isChained);
+        return isConstStatement(tok->astOperand2(), library, platformIndependent, /*isNestedBracket*/ !isChained);
     }
     if (!tok->astParent() && findLambdaEndToken(tok))
         return true;
@@ -2331,7 +2337,7 @@ void CheckOther::checkIncompleteStatement()
         // Skip statement expressions
         if (Token::simpleMatch(rtok, "; } )"))
             continue;
-        if (!isConstStatement(tok, mSettings->library))
+        if (!isConstStatement(tok, mSettings->library, false))
             continue;
         if (isVoidStmt(tok))
             continue;
@@ -2526,7 +2532,7 @@ void CheckOther::checkMisusedScopedObject()
                 if (Token::simpleMatch(parTok, "<") && parTok->link())
                     parTok = parTok->link()->next();
                 if (const Token* arg = parTok->astOperand2()) {
-                    if (!isConstStatement(arg, mSettings->library))
+                    if (!isConstStatement(arg, mSettings->library, false))
                         continue;
                     if (parTok->str() == "(") {
                         if (arg->varId() && !(arg->variable() && arg->variable()->nameToken() != arg))
@@ -2985,12 +2991,13 @@ void CheckOther::checkDuplicateExpression()
                     }
                 }
             } else if (tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
-                if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()) &&
-                    !isVariableChanged(tok->astParent(), /*indirect*/ 0, *mSettings) &&
-                    isConstStatement(tok->astOperand1(), mSettings->library) && isConstStatement(tok->astOperand2(), mSettings->library))
-                    duplicateValueTernaryError(tok);
-                else if (isSameExpression(true, tok->astOperand1(), tok->astOperand2(), *mSettings, false, true, &errorPath))
+                if (isSameExpression(true, tok->astOperand1(), tok->astOperand2(), *mSettings, false, true, &errorPath))
                     duplicateExpressionTernaryError(tok, std::move(errorPath));
+
+                else if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()) &&
+                         !isVariableChanged(tok->astParent(), /*indirect*/ 0, *mSettings) &&
+                         isConstStatement(tok->astOperand1(), mSettings->library, true) && isConstStatement(tok->astOperand2(), mSettings->library, true))
+                    duplicateValueTernaryError(tok);
             }
         }
     }

man/checkers/duplicateExpressionTernary.md

@@ -0,0 +1,65 @@
+# duplicateExpressionTernary
+
+**Message**: Same expression in both branches of ternary operator<br/>
+**Category**: Logic<br/>
+**Severity**: Style<br/>
+**Language**: C/C++
+
+## Description
+
+This checker detects when syntactically identical expressions appear in both the true and false branches of a ternary operator (`condition ? true_expr : false_expr`).
+
+The warning is triggered when:
+- The second and third operands of a ternary operator are syntactically identical expressions
+- This includes cases where variables are referenced through aliases that resolve to the same expression
+
+## Motivation
+
+The same expression indicates that there might be some logic error or copy-paste mistake.
+
+## Examples
+
+### Problematic code
+
+```cpp
+// Same expression in both branches
+int result = condition ? x : x;  // Warning: duplicateExpressionTernary
+
+// Same variable referenced through alias
+const int c = a;
+int result = condition ? a : c;  // Warning: duplicateExpressionTernary
+```
+
+### Fixed code
+
+```cpp
+// Different expressions in branches
+int result = condition ? x : y;  // OK
+```
+
+## How to fix
+
+1. **Check for copy-paste errors**: Verify that both branches are supposed to have the same expression
+2. **Use different expressions**: If the branches should differ, update one of them
+3. **Simplify the code**: If both branches are intentionally the same, remove the ternary operator entirely
+
+Before:
+```cpp
+int getValue(bool flag) {
+    return flag ? computeValue() : computeValue();  // Same computation in both branches
+}
+```
+
+After:
+```cpp
+int getValue(bool flag) {
+    return computeValue();  // Simplified - condition doesn't matter
+}
+```
+
+Or if the branches should differ:
+```cpp
+int getValue(bool flag) {
+    return flag ? computeValue() : computeAlternativeValue();  // Different computations
+}
+```

man/checkers/duplicateValueTernary.md

@@ -0,0 +1,77 @@
+# duplicateValueTernary
+
+**Message**: Same value in both branches of ternary operator<br/>
+**Category**: Logic<br/>
+**Severity**: Style<br/>
+**Language**: C/C++
+
+## Description
+
+This checker detects when both branches of a ternary operator (`condition ? true_expr : false_expr`) evaluate to the same value, even though the expressions themselves may be syntactically different.
+
+The warning is triggered when:
+- The second and third operands of a ternary operator evaluate to the same constant value
+- The expressions are constant statements that don't change during evaluation
+- The expressions have different syntax but equivalent values
+
+However, no warning is generated when:
+- The expressions have different values on different platforms
+- Variables are modified between evaluations
+- The expressions involve non-constant computations
+
+## Motivation
+
+The same value indicates that there might be some logic error or copy-paste mistake.
+
+## Examples
+
+### Problematic code
+
+```cpp
+// Different expressions, same value
+int result = condition ? (int)1 : 1;  // Warning: duplicateValueTernary
+
+// Different cast syntax, same value
+int result = condition ? 1 : (int)1;  // Warning: duplicateValueTernary
+```
+
+### Fixed code
+
+```cpp
+// Different values in branches
+int result = condition ? 1 : 2;  // OK
+
+// Simplified - condition doesn't matter
+int result = 1;  // OK - removed unnecessary ternary
+
+// Platform-dependent values are allowed
+int size = is_64bit ? sizeof(long) : sizeof(int);  // OK - may differ on platforms
+```
+
+## How to fix
+
+1. **Check for logic errors**: Verify if both branches should actually have the same value
+2. **Simplify the code**: If both branches always have the same value, remove the ternary operator
+3. **Use different values**: If the branches should differ, update one of them
+4. **Remove unnecessary casts**: If the difference is only in casting, consider if the cast is needed
+
+Before:
+```cpp
+int getValue(bool flag) {
+    return flag ? (int)42 : 42;  // Same value, different expressions
+}
+```
+
+After (simplified):
+```cpp
+int getValue(bool flag) {
+    return 42;  // Simplified - condition doesn't affect result
+}
+```
+
+Or (if branches should differ):
+```cpp
+int getValue(bool flag) {
+    return flag ? 42 : 0;  // Different values for different conditions
+}
+```

test/testother.cpp

@@ -197,6 +197,7 @@ class TestOther : public TestFixture {
         TEST_CASE(duplicateExpression18);
         TEST_CASE(duplicateExpressionLoop);
         TEST_CASE(duplicateValueTernary);
+        TEST_CASE(duplicateValueTernarySizeof); // #13773
         TEST_CASE(duplicateExpressionTernary); // #6391
         TEST_CASE(duplicateExpressionTemplate); // #6930
         TEST_CASE(duplicateExpressionCompareWithZero);
@@ -8106,31 +8107,31 @@ class TestOther : public TestFixture {
         check("void f() {\n"
               "    if( a ? (b ? false:false): false ) ;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2:23]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
+        ASSERT_EQUALS("[test.cpp:2:23]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
 
         check("int f1(int a) {return (a == 1) ? (int)1 : 1; }");
         ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
 
         check("int f2(int a) {return (a == 1) ? (int)1 : (int)1; }");
-        ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
+        ASSERT_EQUALS("[test.cpp:1:41]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
 
         check("int f3(int a) {return (a == 1) ? 1 : (int)1; }");
         ASSERT_EQUALS("[test.cpp:1:36]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
 
         check("int f4(int a) {return (a == 1) ? 1 : 1; }");
-        ASSERT_EQUALS("[test.cpp:1:36]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
+        ASSERT_EQUALS("[test.cpp:1:36]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
 
         check("int f5(int a) {return (a == (int)1) ? (int)1 : 1; }");
         ASSERT_EQUALS("[test.cpp:1:46]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
 
         check("int f6(int a) {return (a == (int)1) ? (int)1 : (int)1; }");
-        ASSERT_EQUALS("[test.cpp:1:46]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
+        ASSERT_EQUALS("[test.cpp:1:46]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
 
         check("int f7(int a) {return (a == (int)1) ? 1 : (int)1; }");
         ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
 
         check("int f8(int a) {return (a == (int)1) ? 1 : 1; }");
-        ASSERT_EQUALS("[test.cpp:1:41]: (style) Same value in both branches of ternary operator. [duplicateValueTernary]\n", errout_str());
+        ASSERT_EQUALS("[test.cpp:1:41]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
 
         check("struct Foo {\n"
               "  std::vector<int> bar{1,2,3};\n"
@@ -8170,6 +8171,14 @@ class TestOther : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
 
+    void duplicateValueTernarySizeof() { // #13773
+        check("int f() { return x ? sizeof(uint32_t) : sizeof(unsigned int); }");
+        ASSERT_EQUALS("", errout_str());
+
+        check("int f() { return x ? sizeof(uint32_t) : sizeof(uint32_t); }");
+        ASSERT_EQUALS("[test.cpp:1:39]: (style) Same expression in both branches of ternary operator. [duplicateExpressionTernary]\n", errout_str());
+    }
+
     void duplicateExpressionTemplate() {
         check("template <int I> void f() {\n" // #6930
               "    if (I >= 0 && I < 3) {}\n"