New check: Recommend expm1, log1p, erfc (#5392)

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
 //---------------------------------------------------------------------------

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";
     }
 };
 /// @}

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"