From 06752d75a5b2de3c9f83b4e50c749fef8edae250 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 14 Dec 2019 02:15:00 -0600 Subject: [PATCH] Fix issue 9485: knownConditionTrueFalse false positive with integer constants (#2447) * Fix issue 9485: knownConditionTrueFalse false positive with integer constants * Formatting --- lib/checkother.cpp | 38 ++++++++++++++++++++++++++++++++++---- test/testother.cpp | 13 +++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 1aa4436a7..d5372f7f8 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1987,7 +1987,15 @@ void CheckOther::checkDuplicateExpression() if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) { if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true)) continue; - if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true, true, &errorPath)) { + const bool followVar = !isConstVarExpression(tok) || Token::Match(tok, "%comp%|%oror%|&&"); + if (isSameExpression(mTokenizer->isCPP(), + true, + tok->astOperand1(), + tok->astOperand2(), + mSettings->library, + true, + followVar, + &errorPath)) { if (isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { const bool assignment = tok->str() == "="; if (assignment && warningEnabled) @@ -2005,17 +2013,39 @@ void CheckOther::checkDuplicateExpression() duplicateExpressionError(tok->astOperand1(), tok->astOperand2(), tok, errorPath); } } - } else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "=") && isSameExpression(mTokenizer->isCPP(), false, tok->astOperand1(), tok->astOperand2()->astOperand1(), mSettings->library, true, false)) { + } else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "=") && + isSameExpression(mTokenizer->isCPP(), + false, + tok->astOperand1(), + tok->astOperand2()->astOperand1(), + mSettings->library, + true, + false)) { if (warningEnabled && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { selfAssignmentError(tok, tok->astOperand1()->expressionString()); } } else if (styleEnabled && - isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, true, &errorPath) && + isOppositeExpression(mTokenizer->isCPP(), + tok->astOperand1(), + tok->astOperand2(), + mSettings->library, + false, + true, + &errorPath) && !Token::Match(tok, "=|-|-=|/|/=") && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { oppositeExpressionError(tok, errorPath); } else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative - if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), mSettings->library, true, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2())) + if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && + isSameExpression(mTokenizer->isCPP(), + true, + tok->astOperand2(), + tok->astOperand1()->astOperand2(), + mSettings->library, + true, + followVar, + &errorPath) && + isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2())) duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath); else if (tok->astOperand2() && isConstExpression(tok->astOperand1(), mSettings->library, true, mTokenizer->isCPP())) { const Token *ast1 = tok->astOperand1(); diff --git a/test/testother.cpp b/test/testother.cpp index 8f3ea3488..7800f2879 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -139,6 +139,7 @@ private: TEST_CASE(duplicateExpression7); TEST_CASE(duplicateExpression8); TEST_CASE(duplicateExpression9); // #9320 + TEST_CASE(duplicateExpression10); // #9485 TEST_CASE(duplicateExpressionLoop); TEST_CASE(duplicateValueTernary); TEST_CASE(duplicateExpressionTernary); // #6391 @@ -4657,6 +4658,18 @@ private: ASSERT_EQUALS("", errout.str()); } + void duplicateExpression10() + { + // #9485 + check("int f() {\n" + " const int a = 1;\n" + " const int b = a-1;\n" + " const int c = a+1;\n" + " return c;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void duplicateExpressionLoop() { check("void f() {\n" " int a = 1;\n"