From ff4fc6a2347dba691bca8b9f0440b96ceebb3780 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sun, 7 Sep 2014 11:37:58 +0200 Subject: [PATCH] New check: Recommend expm1, log1p, erfc (#5392) --- lib/checkother.cpp | 19 +++++++++++++++++++ lib/checkother.h | 5 ++++- test/testother.cpp | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index c0cfacb8a..64eabd45a 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1892,6 +1892,8 @@ void CheckOther::nanInArithmeticExpressionError(const Token *tok) //--------------------------------------------------------------------------- void CheckOther::checkMathFunctions() { + bool styleC99 = _settings->isEnabled("style") && _settings->standards.c != Standards::C89 && _settings->standards.cpp != Standards::CPP03; + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); const std::size_t functions = symbolDatabase->functionScopes.size(); for (std::size_t i = 0; i < functions; ++i) { @@ -1943,6 +1945,18 @@ void CheckOther::checkMathFunctions() MathLib::isNegative(tok->strAt(4))) { mathfunctionCallWarning(tok, 2); } + + if (styleC99) { + if (Token::Match(tok, "%num% - erf (") && MathLib::toDoubleNumber(tok->str()) == 1.0 && tok->next()->astOperand2() == tok->tokAt(3)) { + mathfunctionCallWarning(tok, "1 - erf(x)", "erfc(x)"); + } else if (Token::simpleMatch(tok, "exp (") && Token::Match(tok->linkAt(1), ") - %num%") && MathLib::toDoubleNumber(tok->linkAt(1)->strAt(2)) == 1.0 && tok->linkAt(1)->next()->astOperand1() == tok->next()) { + mathfunctionCallWarning(tok, "exp(x) - 1", "expm1(x)"); + } else if (Token::Match(tok, "log (") && tok->next()->astOperand2()) { + const Token* plus = tok->next()->astOperand2(); + if (plus->str() == "+" && ((plus->astOperand1() && MathLib::toDoubleNumber(plus->astOperand1()->str()) == 1.0) || (plus->astOperand2() && MathLib::toDoubleNumber(plus->astOperand2()->str()) == 1.0))) + mathfunctionCallWarning(tok, "log(1 + x)", "log10(x)"); + } + } } } } @@ -1958,6 +1972,11 @@ void CheckOther::mathfunctionCallWarning(const Token *tok, const unsigned int nu reportError(tok, Severity::warning, "wrongmathcall", "Passing value '#' to #() leads to implementation-defined result."); } +void CheckOther::mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp) +{ + reportError(tok, Severity::style, "unpreciseMathCall", "Expression '" + oldexp + "' can be replaced by '" + newexp + "' to avoid loss of precision."); +} + //--------------------------------------------------------------------------- // Creating instance of clases which are destroyed immediately //--------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index ec7290fbc..452dd0151 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -258,6 +258,7 @@ private: void zerodivcondError(const Token *tokcond, const Token *tokdiv, bool inconclusive); void nanInArithmeticExpressionError(const Token *tok); void mathfunctionCallWarning(const Token *tok, const unsigned int numParam = 1); + void mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp); void redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive); void redundantAssignmentInSwitchError(const Token *tok1, const Token *tok2, const std::string &var); void redundantCopyError(const Token *tok1, const Token* tok2, const std::string& var); @@ -326,6 +327,7 @@ private: c.suspiciousEqualityComparisonError(0); c.selfAssignmentError(0, "varname"); c.mathfunctionCallWarning(0); + c.mathfunctionCallWarning(0, "1 - erf(x)", "erfc(x)"); c.memsetZeroBytesError(0, "varname"); c.memsetFloatError(0, "varname"); c.memsetValueOutOfRangeError(0, "varname"); @@ -405,7 +407,8 @@ private: "* redundant get and set function of user id (--std=posix).\n" "* Passing NULL pointer to function with variable number of arguments leads to UB on some platforms.\n" "* NaN (not a number) value used in arithmetic expression.\n" - "* comma in return statement (the comma can easily be misread as a semicolon).\n"; + "* comma in return statement (the comma can easily be misread as a semicolon).\n" + "* prefer erfc, expm1 or log1p to avoid loss of precision.\n"; } }; /// @} diff --git a/test/testother.cpp b/test/testother.cpp index 4e49a0cfd..affd54eee 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -88,6 +88,7 @@ private: TEST_CASE(mathfunctionCall_asin); TEST_CASE(mathfunctionCall_pow); TEST_CASE(mathfunctionCall_atan2); + TEST_CASE(mathfunctionCall_precision); TEST_CASE(switchRedundantAssignmentTest); TEST_CASE(switchRedundantOperationTest); @@ -1689,6 +1690,41 @@ private: ASSERT_EQUALS("", errout.str()); } + void mathfunctionCall_precision() { + check("void foo() {\n" + " print(exp(x) - 1);\n" + " print(log(1 + x));\n" + " print(1 - erf(x));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Expression 'exp(x) - 1' can be replaced by 'expm1(x)' to avoid loss of precision.\n" + "[test.cpp:3]: (style) Expression 'log(1 + x)' can be replaced by 'log10(x)' to avoid loss of precision.\n" + "[test.cpp:4]: (style) Expression '1 - erf(x)' can be replaced by 'erfc(x)' to avoid loss of precision.\n", errout.str()); + + check("void foo() {\n" + " print(exp(x) - 1.0);\n" + " print(log(1.0 + x));\n" + " print(1.0 - erf(x));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Expression 'exp(x) - 1' can be replaced by 'expm1(x)' to avoid loss of precision.\n" + "[test.cpp:3]: (style) Expression 'log(1 + x)' can be replaced by 'log10(x)' to avoid loss of precision.\n" + "[test.cpp:4]: (style) Expression '1 - erf(x)' can be replaced by 'erfc(x)' to avoid loss of precision.\n", errout.str()); + + check("void foo() {\n" + " print(exp(3 + x*f(a)) - 1);\n" + " print(log(x*4 + 1));\n" + " print(1 - erf(34*x + f(x) - c));\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Expression 'exp(x) - 1' can be replaced by 'expm1(x)' to avoid loss of precision.\n" + "[test.cpp:3]: (style) Expression 'log(1 + x)' can be replaced by 'log10(x)' to avoid loss of precision.\n" + "[test.cpp:4]: (style) Expression '1 - erf(x)' can be replaced by 'erfc(x)' to avoid loss of precision.\n", errout.str()); + + check("void foo() {\n" + " print(2*exp(x) - 1);\n" + " print(1 - erf(x)/2.0);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void switchRedundantAssignmentTest() { check("void foo()\n"