pull request for ticket 8180 (better diagnostic output for bailouts) (#964)

* sync build instructions from readme.txt

* refactored the patch from ticket 8180. Moved logic from macros to the bailoutInternal function

* adapt to new bailout message format

* adapt to new bailout message format

* adapt to new bailout message format

* compile fix for Microsoft platform

* remove directory part from file locations in bailout message (normalize)

* remove directory part from valueflow message filter

* adapt tests to file format without directory part

* adapt tests to file format without directory part

* new line number agnostic assert_equals methods

* new line number agnostic assert_equals methods

* adapt to new method assertEqualsWithoutLineNumbers()

* adapt to new method assertEqualsWithoutLineNumbers()

* Bugfix: do not replace line number with spaces, remove it

* review changes: const char * -> std::string, size_t -> int, std::to_string() -> MathLib::toString()

* set #line at the beginning to guard against insertions from match compiler

* Bugfix: counting lines can be difficult :-) #line 1 -> #line 2

* added method stripDirectoryPart()

* added method stripDirectoryPart()

* used new method Path::stripDirectoryPart()

* new dependency path.h in lib/valueFlow.cpp

* code cleanup, removing redundant temporary objects and casts

Makefile

@@ -443,7 +443,7 @@ $(SRCDIR)/tokenize.o: lib/tokenize.cpp lib/cxx11emu.h lib/tokenize.h lib/config.
 $(SRCDIR)/tokenlist.o: lib/tokenlist.cpp lib/cxx11emu.h lib/tokenlist.h lib/config.h lib/errorlogger.h lib/suppressions.h lib/mathlib.h lib/path.h lib/settings.h lib/importproject.h lib/platform.h lib/utils.h lib/library.h lib/standards.h lib/timer.h lib/token.h lib/valueflow.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(SRCDIR)/tokenlist.o $(SRCDIR)/tokenlist.cpp
 
-$(SRCDIR)/valueflow.o: lib/valueflow.cpp lib/cxx11emu.h lib/valueflow.h lib/config.h lib/astutils.h lib/errorlogger.h lib/suppressions.h lib/library.h lib/mathlib.h lib/standards.h lib/platform.h lib/settings.h lib/importproject.h lib/utils.h lib/timer.h lib/symboldatabase.h lib/token.h lib/tokenlist.h
+$(SRCDIR)/valueflow.o: lib/valueflow.cpp lib/cxx11emu.h lib/valueflow.h lib/config.h lib/astutils.h lib/errorlogger.h lib/suppressions.h lib/library.h lib/mathlib.h lib/standards.h lib/platform.h lib/settings.h lib/importproject.h lib/utils.h lib/timer.h lib/symboldatabase.h lib/token.h lib/tokenlist.h lib/path.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(SRCDIR)/valueflow.o $(SRCDIR)/valueflow.cpp
 
 cli/cmdlineparser.o: cli/cmdlineparser.cpp lib/cxx11emu.h cli/cmdlineparser.h lib/check.h lib/config.h lib/errorlogger.h lib/suppressions.h lib/settings.h lib/importproject.h lib/platform.h lib/utils.h lib/library.h lib/mathlib.h lib/standards.h lib/timer.h lib/token.h lib/valueflow.h lib/tokenize.h lib/tokenlist.h cli/cppcheckexecutor.h cli/filelister.h lib/path.h cli/threadexecutor.h

lib/path.cpp

@@ -225,3 +225,20 @@ std::string Path::getAbsoluteFilePath(const std::string& filePath)
 #endif
     return absolute_path;
 }
+
+std::string Path::stripDirectoryPart(const std::string &file)
+{
+    std::string fileName(file);
+#if defined(_WIN32)
+    const char native = '\\';
+#else
+    const char native = '/';
+#endif
+
+    std::string::size_type p = fileName.rfind(native);
+
+    if (p != std::string::npos) {
+        fileName = fileName.substr(p + 1);
+    }
+    return fileName;
+}

lib/path.h

@@ -165,6 +165,13 @@ public:
      * @return true if filename extension is meant for headers
      */
     static bool isHeader(const std::string &path);
+
+    /**
+     * @brief Get filename without a directory path part.
+     * @param file filename to be stripped. path info is optional
+     * @return filename without directory path part.
+     */
+    static std::string stripDirectoryPart(const std::string &file);
 };
 
 /// @}

lib/valueflow.cpp

@@ -1,3 +1,4 @@
+#line 2
 /*
  * Cppcheck - A tool for static C/C++ code analysis
  * Copyright (C) 2007-2016 Cppcheck team.
@@ -29,6 +30,7 @@
 #include "token.h"
 #include "tokenlist.h"
 #include "utils.h"
+#include "path.h"
 
 #include <algorithm>
 #include <cstddef>
@@ -89,14 +91,23 @@ static void execute(const Token *expr,
                     MathLib::bigint *result,
                     bool *error);
 
-static void bailout(TokenList *tokenlist, ErrorLogger *errorLogger, const Token *tok, const std::string &what)
+static void bailoutInternal(TokenList *tokenlist, ErrorLogger *errorLogger, const Token *tok, const std::string &what, const std::string &file, int line, const std::string &function)
 {
     std::list<ErrorLogger::ErrorMessage::FileLocation> callstack;
     callstack.push_back(ErrorLogger::ErrorMessage::FileLocation(tok, tokenlist));
-    ErrorLogger::ErrorMessage errmsg(callstack, tokenlist->getSourceFilePath(), Severity::debug, "ValueFlow bailout: " + what, "valueFlowBailout", false);
+    ErrorLogger::ErrorMessage errmsg(callstack, tokenlist->getSourceFilePath(), Severity::debug,
+        Path::stripDirectoryPart(file) + ":" + MathLib::toString(line) + ":" + function + " bailout: " + what, "valueFlowBailout", false);
     errorLogger->reportErr(errmsg);
 }
 
+#if (defined __cplusplus) && __cplusplus >= 201103L
+#define bailout(tokenlist, errorLogger, tok, what) bailoutInternal(tokenlist, errorLogger, tok, what, __FILE__, __LINE__, __func__)
+#elif (defined __GNUC__) || (defined __clang__) || (defined _MSC_VER)
+#define bailout(tokenlist, errorLogger, tok, what) bailoutInternal(tokenlist, errorLogger, tok, what, __FILE__, __LINE__, __FUNCTION__)
+#else
+#define bailout(tokenlist, errorLogger, tok, what) bailoutInternal(tokenlist, errorLogger, tok, what, __FILE__, __LINE__, "(valueFlow)")
+#endif
+
 /**
  * Is condition always false when variable has given value?
  * \param condition   top ast token in condition

test/testsimplifytypedef.cpp

@@ -1143,7 +1143,7 @@ private:
                                 "}";
 
         ASSERT_EQUALS(expected, tok(code, false));
-        ASSERT_EQUALS("[test.cpp:28]: (debug) ValueFlow bailout: function return; nontrivial function body\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:28]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n", errout.str());
     }
 
     void simplifyTypedef36() {
@@ -2165,7 +2165,7 @@ private:
                             "    return fred;\n"
                             "}";
         tok(code);
-        ASSERT_EQUALS("[test.cpp:2]: (debug) ValueFlow bailout: function return; nontrivial function body\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n", errout.str());
     }
 
     void simplifyTypedef101() { // ticket #3003 (segmentation fault)

test/testsuite.cpp

@@ -138,7 +138,38 @@ void TestFixture::assertEquals(const char *filename, unsigned int linenr, const
         errmsg << "_____" << std::endl;
     }
 }
+
+std::string TestFixture::deleteLineNumber(const std::string &message) const
+{
+    std::string result(message);
+    // delete line number in "...:NUMBER:..."
+    std::string::size_type pos = 0;
+    std::string::size_type after = 0;
+    while ((pos = result.find(':', pos)) != std::string::npos) {
+        // get number
+        if (pos + 1 == result.find_first_of("0123456789", pos + 1)) {
+            if ((after = result.find_first_not_of("0123456789", pos + 1)) != std::string::npos
+                 && result.at(after) == ':') {
+                // erase NUMBER
+                result.erase(pos + 1, after - pos - 1);
+                pos = after;
+            } else {
+                ++pos;
+            }
+        } else {
+            ++pos;
+        }
+    }
+    return result;
+}
+
+void TestFixture::assertEqualsWithoutLineNumbers(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg) const
+{
+    assertEquals(filename, linenr, deleteLineNumber(expected), deleteLineNumber(actual));
+}
+
 void TestFixture::assertEquals(const char *filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg) const
+
 {
     assertEquals(filename, linenr, std::string(expected), actual, msg);
 }

test/testsuite.h

@@ -50,6 +50,7 @@ protected:
     void assert_(const char *filename, unsigned int linenr, bool condition) const;
 
     void assertEquals(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = emptyString) const;
+    void assertEqualsWithoutLineNumbers(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = emptyString) const;
     void assertEquals(const char *filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg = emptyString) const;
     void assertEquals(const char *filename, unsigned int linenr, const char expected[], const char actual[], const std::string &msg = emptyString) const;
     void assertEquals(const char *filename, unsigned int linenr, const std::string& expected, const char actual[], const std::string &msg = emptyString) const;
@@ -64,6 +65,8 @@ protected:
     void assertThrowFail(const char *filename, unsigned int linenr) const;
     void assertNoThrowFail(const char *filename, unsigned int linenr) const;
     void complainMissingLib(const char* libname) const;
+    std::string deleteLineNumber(const std::string &message) const;
+
 
     void processOptions(const options& args);
 public:
@@ -83,6 +86,7 @@ extern std::ostringstream output;
 #define TEST_CASE( NAME )  if ( prepareTest(#NAME) ) { NAME(); }
 #define ASSERT( CONDITION )  assert_(__FILE__, __LINE__, CONDITION)
 #define ASSERT_EQUALS( EXPECTED , ACTUAL )  assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL)
+#define ASSERT_EQUALS_WITHOUT_LINENUMBERS( EXPECTED , ACTUAL )  assertEqualsWithoutLineNumbers(__FILE__, __LINE__, EXPECTED, ACTUAL)
 #define ASSERT_EQUALS_DOUBLE( EXPECTED , ACTUAL )  assertEqualsDouble(__FILE__, __LINE__, EXPECTED, ACTUAL)
 #define ASSERT_EQUALS_MSG( EXPECTED , ACTUAL, MSG )  assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL, MSG)
 #define ASSERT_THROW( CMD, EXCEPTION ) try { CMD ; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&) { } catch (...) { assertThrowFail(__FILE__, __LINE__); }

test/testtokenize.cpp

@@ -491,7 +491,7 @@ private:
         std::istringstream istr2(debugwarnings.c_str());
         std::string line;
         while (std::getline(istr2,line)) {
-            if (line.find("ValueFlow") == std::string::npos)
+            if (line.find("valueflow.cpp") == std::string::npos)
                 errout << line << "\n";
         }
 
@@ -521,7 +521,7 @@ private:
         std::istringstream istr2(debugwarnings.c_str());
         std::string line;
         while (std::getline(istr2,line)) {
-            if (line.find("ValueFlow") == std::string::npos)
+            if (line.find("valueflow.cpp") == std::string::npos)
                 errout << line << "\n";
         }
 

test/testvalueflow.cpp

@@ -773,7 +773,7 @@ private:
                 "    x = y;\n"
                 "    if (x == 123) {}\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:2]: (debug) ValueFlow bailout: assignment of x\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp:1035:valueFlowReverse bailout: assignment of x\n", errout.str());
     }
 
     void valueFlowBeforeConditionAndAndOrOrGuard() { // guarding by &&
@@ -888,13 +888,13 @@ private:
         bailout("void f(int x) {\n"
                 "    y = ((x<0) ? x : ((x==2)?3:4));\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:2]: (debug) ValueFlow bailout: no simplification of x within ?: expression\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp:1113:valueFlowReverse bailout: no simplification of x within ?: expression\n", errout.str());
 
         bailout("int f(int x) {\n"
                 "  int r = x ? 1 / x : 0;\n"
                 "  if (x == 0) {}\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:2]: (debug) ValueFlow bailout: no simplification of x within ?: expression\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp:1113:valueFlowReverse bailout: no simplification of x within ?: expression\n", errout.str());
 
         code = "void f(int x) {\n"
                "    int a =v x;\n"
@@ -952,7 +952,7 @@ private:
                 "    if (x != 123) { b = x; }\n"
                 "    if (x == 123) {}\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:2]: (debug) ValueFlow bailout: variable x stopping on }\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp:1144:valueFlowReverse bailout: variable x stopping on }\n", errout.str());
 
         code = "void f(int x) {\n"
                "  a = x;\n"
@@ -969,7 +969,7 @@ private:
                 "    int a = x;\n"
                 "    if (x == 123) {}\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:4]: (debug) ValueFlow bailout: global variable x\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:4]: (debug) valueflow.cpp:1226:valueFlowBeforeCondition bailout: global variable x\n", errout.str());
 
         // class variable
         const char *code;
@@ -991,7 +991,7 @@ private:
                 "    case 2: if (x==5) {} break;\n"
                 "    };\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:3]: (debug) ValueFlow bailout: variable x stopping on break\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1180:valueFlowReverse bailout: variable x stopping on break\n", errout.str());
 
         bailout("void f(int x, int y) {\n"
                 "    switch (y) {\n"
@@ -999,7 +999,7 @@ private:
                 "    case 2: if (x==5) {} break;\n"
                 "    };\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:3]: (debug) ValueFlow bailout: variable x stopping on return\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1180:valueFlowReverse bailout: variable x stopping on return\n", errout.str());
     }
 
     void valueFlowBeforeConditionMacro() {
@@ -1009,7 +1009,7 @@ private:
                 "    a = x;\n"
                 "    M;\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:4]: (debug) ValueFlow bailout: variable x, condition is defined in macro\n", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:4]: (debug) valueflow.cpp:1260:valueFlowBeforeCondition bailout: variable x, condition is defined in macro\n", errout.str());
     }
 
     void valueFlowBeforeConditionGoto() {
@@ -1020,8 +1020,8 @@ private:
                 "out:"
                 "    if (x==123){}\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:4]: (debug) ValueFlow bailout: variable x stopping on goto label\n"
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:4]: (debug) valueflow.cpp:1131:valueFlowReverse bailout: variable x stopping on goto label\n"
-                      "[test.cpp:2]: (debug) ValueFlow bailout: variable x. noreturn conditional scope.\n"
+                      "[test.cpp:2]: (debug) valueflow.cpp:1813:valueFlowForward bailout: variable x. noreturn conditional scope.\n"
                       , errout.str());
 
         // #5721 - FP
@@ -1035,9 +1035,9 @@ private:
                 "out:\n"
                 "    if (abc) {}\n"
                 "}\n");
-        ASSERT_EQUALS("[test.cpp:2]: (debug) ValueFlow bailout: assignment of abc\n"
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:2]: (debug) valueflow.cpp:1035:valueFlowReverse bailout: assignment of abc\n"
-                      "[test.cpp:8]: (debug) ValueFlow bailout: variable abc stopping on goto label\n"
+                      "[test.cpp:8]: (debug) valueflow.cpp:1131:valueFlowReverse bailout: variable abc stopping on goto label\n"
-                      "[test.cpp:3]: (debug) ValueFlow bailout: variable abc. noreturn conditional scope.\n",
+                      "[test.cpp:3]: (debug) valueflow.cpp:1813:valueFlowForward bailout: variable abc. noreturn conditional scope.\n",
                       errout.str());
     }