Merge pull request #1448 from catchorg/dev-redo-capturer-parsing

Fix CAPTURE macro for nontrivial uses

docs/logging.md

@@ -57,20 +57,37 @@ The message is reported and the test case fails.
 
 AS `FAIL`, but does not abort the test
 
-## Quickly capture a variable value
+## Quickly capture value of variables or expressions
 
-**CAPTURE(** _expression_ **)**
+**CAPTURE(** _expression1_, _expression2_, ... **)**
 
-Sometimes you just want to log the name and value of a variable. While you can easily do this with the INFO macro, above, as a convenience the CAPTURE macro handles the stringising of the variable name for you (actually it works with any expression, not just variables).
+Sometimes you just want to log a value of variable, or expression. For
+convenience, we provide the `CAPTURE` macro, that can take a variable,
+or an expression, and prints out that variable/expression and its value
+at the time of capture.
 
-E.g.
+e.g. `CAPTURE( theAnswer );` will log message "theAnswer := 42", while
-```c++
+```cpp
-CAPTURE( theAnswer );
+int a = 1, b = 2, c = 3;
+CAPTURE( a, b, c, a + b, c > b, a == 1);
+```
+will log a total of 6 messages:
+```
+a := 1
+b := 2
+c := 3
+a + b := 3
+c > b := true
+a == 1 := true
 ```
 
-This would log something like:
+You can also capture expressions that use commas inside parentheses
+(e.g. function calls), brackets, or braces (e.g. initializers). To
+properly capture expression that contains template parameters list
+(in other words, it contains commas between angle brackets), you need
+to enclose the expression inside parentheses:
+`CAPTURE( (std::pair<int, int>{1, 2}) );`
 
-<pre>"theAnswer := 42"</pre>
 
 ---
 

include/internal/catch_message.cpp

@@ -11,6 +11,7 @@
 #include "catch_uncaught_exceptions.h"
 
 #include <cassert>
+#include <stack>
 
 namespace Catch {
 
@@ -60,20 +61,49 @@ namespace Catch {
 
 
     Capturer::Capturer( StringRef macroName, SourceLineInfo const& lineInfo, ResultWas::OfType resultType, StringRef names ) {
-        auto start = std::string::npos;
+        auto trimmed = [&] (size_t start, size_t end) {
-        for( size_t pos = 0; pos <= names.size(); ++pos ) {
+            while (names[start] == ',' || isspace(names[start])) {
+                ++start;
+            }
+            while (names[end] == ',' || isspace(names[end])) {
+                --end;
+            }
+            return names.substr(start, end - start + 1);
+        };
+
+        size_t start = 0;
+        std::stack<char> openings;
+        for (size_t pos = 0; pos < names.size(); ++pos) {
             char c = names[pos];
-            if( pos == names.size() || c == ' ' || c == '\t' || c == ',' || c == ']' ) {
+            switch (c) {
-                if( start != std::string::npos ) {
+            case '[':
-                    m_messages.push_back( MessageInfo( macroName, lineInfo, resultType ) );
+            case '{':
-                    m_messages.back().message = names.substr( start, pos-start) + " := ";
+            case '(':
-                    start = std::string::npos;
+            // It is basically impossible to disambiguate between
-                }
+            // comparison and start of template args in this context
-            }
+//            case '<':
-            else if( c != '[' && c != ']' && start == std::string::npos )
+                openings.push(c);
+                break;
+            case ']':
+            case '}':
+            case ')':
+//           case '>':
+                openings.pop();
+                break;
+            case ',':
+                if (start != pos && openings.size() == 0) {
+                    m_messages.emplace_back(macroName, lineInfo, resultType);
+                    m_messages.back().message = trimmed(start, pos);
+                    m_messages.back().message += " := ";
                     start = pos;
                 }
             }
+        }
+        assert(openings.size() == 0 && "Mismatched openings");
+        m_messages.emplace_back(macroName, lineInfo, resultType);
+        m_messages.back().message = trimmed(start, names.size() - 1);
+        m_messages.back().message += " := ";
+    }
     Capturer::~Capturer() {
         if ( !uncaught_exceptions() ){
             assert( m_captured == m_messages.size() );
@@ -82,7 +112,7 @@ namespace Catch {
         }
     }
 
-    void Capturer::captureValue( size_t index, StringRef value ) {
+    void Capturer::captureValue( size_t index, std::string const& value ) {
         assert( index < m_messages.size() );
         m_messages[index].message += value;
         m_resultCapture.pushScopedMessage( m_messages[index] );

include/internal/catch_message.h

@@ -77,16 +77,16 @@ namespace Catch {
         Capturer( StringRef macroName, SourceLineInfo const& lineInfo, ResultWas::OfType resultType, StringRef names );
         ~Capturer();
 
-        void captureValue( size_t index, StringRef value );
+        void captureValue( size_t index, std::string const& value );
 
         template<typename T>
-        void captureValues( size_t index, T&& value ) {
+        void captureValues( size_t index, T const& value ) {
             captureValue( index, Catch::Detail::stringify( value ) );
         }
 
         template<typename T, typename... Ts>
-        void captureValues( size_t index, T&& value, Ts&&... values ) {
+        void captureValues( size_t index, T const& value, Ts const&... values ) {
-            captureValues( index, value );
+            captureValue( index, Catch::Detail::stringify(value) );
             captureValues( index+1, values... );
         }
     };

projects/SelfTest/Baselines/compact.sw.approved.txt

@@ -228,6 +228,8 @@ Approx.tests.cpp:<line number>: passed: NAN != Approx(NAN) for: nanf != Approx(
 Approx.tests.cpp:<line number>: passed: !(NAN == Approx(NAN)) for: !(nanf == Approx( nan ))
 Tricky.tests.cpp:<line number>: passed: y.v == 0 for: 0 == 0
 Tricky.tests.cpp:<line number>: passed: 0 == y.v for: 0 == 0
+Message.tests.cpp:<line number>: passed: with 7 messages: 'a := 1' and 'b := 2' and 'c := 3' and 'a + b := 3' and 'a+b := 3' and 'c > b := true' and 'a == 1 := true'
+Message.tests.cpp:<line number>: passed: with 7 messages: 'std::vector<int>{1, 2, 3}[0, 1, 2] := 3' and 'std::vector<int>{1, 2, 3}[(0, 1)] := 2' and 'std::vector<int>{1, 2, 3}[0] := 1' and '(helper_1436<int, int>{12, -12}) := { 12, -12 }' and '(helper_1436<int, int>(-12, 12)) := { -12, 12 }' and '(1, 2) := 2' and '(2, 3) := 3'
 ToStringGeneral.tests.cpp:<line number>: passed: true with 1 message: 'i := 2'
 ToStringGeneral.tests.cpp:<line number>: passed: true with 1 message: '3'
 ToStringGeneral.tests.cpp:<line number>: passed: tab == '\t' for: '\t' == '\t'

projects/SelfTest/Baselines/console.std.approved.txt

@@ -1126,6 +1126,6 @@ due to unexpected exception with message:
   Why would you throw a std::string?
 
 ===============================================================================
-test cases:  226 |  170 passed |  52 failed |  4 failed as expected
+test cases:  228 |  172 passed |  52 failed |  4 failed as expected
-assertions: 1308 | 1176 passed | 111 failed | 21 failed as expected
+assertions: 1310 | 1178 passed | 111 failed | 21 failed as expected
 

projects/SelfTest/Baselines/console.sw.approved.txt

@@ -2108,6 +2108,38 @@ Tricky.tests.cpp:<line number>: PASSED:
 with expansion:
   0 == 0
 
+-------------------------------------------------------------------------------
+CAPTURE can deal with complex expressions
+-------------------------------------------------------------------------------
+Message.tests.cpp:<line number>
+...............................................................................
+
+Message.tests.cpp:<line number>: PASSED:
+with messages:
+  a := 1
+  b := 2
+  c := 3
+  a + b := 3
+  a+b := 3
+  c > b := true
+  a == 1 := true
+
+-------------------------------------------------------------------------------
+CAPTURE can deal with complex expressions involving commas
+-------------------------------------------------------------------------------
+Message.tests.cpp:<line number>
+...............................................................................
+
+Message.tests.cpp:<line number>: PASSED:
+with messages:
+  std::vector<int>{1, 2, 3}[0, 1, 2] := 3
+  std::vector<int>{1, 2, 3}[(0, 1)] := 2
+  std::vector<int>{1, 2, 3}[0] := 1
+  (helper_1436<int, int>{12, -12}) := { 12, -12 }
+  (helper_1436<int, int>(-12, 12)) := { -12, 12 }
+  (1, 2) := 2
+  (2, 3) := 3
+
 -------------------------------------------------------------------------------
 Capture and info messages
   Capture should stringify like assertions
@@ -10432,6 +10464,6 @@ Misc.tests.cpp:<line number>
 Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
-test cases:  226 |  157 passed |  65 failed |  4 failed as expected
+test cases:  228 |  159 passed |  65 failed |  4 failed as expected
-assertions: 1322 | 1176 passed | 125 failed | 21 failed as expected
+assertions: 1324 | 1178 passed | 125 failed | 21 failed as expected
 

projects/SelfTest/Baselines/junit.sw.approved.txt

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuitesloose text artifact
 >
-  <testsuite name="<exe-name>" errors="17" failures="109" tests="1323" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="109" tests="1325" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <testcase classname="<exe-name>.global" name="# A test name that starts with a #" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="#1005: Comparing pointer to int and long (NULL can be either on various systems)" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="#1027" time="{duration}"/>
@@ -141,6 +141,8 @@ Exception.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Assertions then sections/A section/Another other section" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Assorted miscellaneous tests" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Bitfields can be captured (#1027)" time="{duration}"/>
+    <testcase classname="<exe-name>.global" name="CAPTURE can deal with complex expressions" time="{duration}"/>
+    <testcase classname="<exe-name>.global" name="CAPTURE can deal with complex expressions involving commas" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Capture and info messages/Capture should stringify like assertions" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Capture and info messages/Info should NOT stringify the way assertions do" time="{duration}"/>
     <testcase classname="<exe-name>.global" name="Character pretty printing/Specifically escaped" time="{duration}"/>

projects/SelfTest/Baselines/xml.sw.approved.txt

@@ -1986,6 +1986,54 @@
       </Expression>
       <OverallResult success="true"/>
     </TestCase>
+    <TestCase name="CAPTURE can deal with complex expressions" tags="[capture][messages]" filename="projects/<exe-name>/UsageTests/Message.tests.cpp" >
+      <Info>
+        a := 1
+      </Info>
+      <Info>
+        b := 2
+      </Info>
+      <Info>
+        c := 3
+      </Info>
+      <Info>
+        a + b := 3
+      </Info>
+      <Info>
+        a+b := 3
+      </Info>
+      <Info>
+        c > b := true
+      </Info>
+      <Info>
+        a == 1 := true
+      </Info>
+      <OverallResult success="true"/>
+    </TestCase>
+    <TestCase name="CAPTURE can deal with complex expressions involving commas" tags="[capture][messages]" filename="projects/<exe-name>/UsageTests/Message.tests.cpp" >
+      <Info>
+        std::vector&lt;int>{1, 2, 3}[0, 1, 2] := 3
+      </Info>
+      <Info>
+        std::vector&lt;int>{1, 2, 3}[(0, 1)] := 2
+      </Info>
+      <Info>
+        std::vector&lt;int>{1, 2, 3}[0] := 1
+      </Info>
+      <Info>
+        (helper_1436&lt;int, int>{12, -12}) := { 12, -12 }
+      </Info>
+      <Info>
+        (helper_1436&lt;int, int>(-12, 12)) := { -12, 12 }
+      </Info>
+      <Info>
+        (1, 2) := 2
+      </Info>
+      <Info>
+        (2, 3) := 3
+      </Info>
+      <OverallResult success="true"/>
+    </TestCase>
     <TestCase name="Capture and info messages" filename="projects/<exe-name>/UsageTests/ToStringGeneral.tests.cpp" >
       <Section name="Capture should stringify like assertions" filename="projects/<exe-name>/UsageTests/ToStringGeneral.tests.cpp" >
         <Info>
@@ -12051,7 +12099,7 @@ loose text artifact
       </Section>
       <OverallResult success="true"/>
     </TestCase>
-    <OverallResults successes="1176" failures="126" expectedFailures="21"/>
+    <OverallResults successes="1178" failures="126" expectedFailures="21"/>
   </Group>
-  <OverallResults successes="1176" failures="125" expectedFailures="21"/>
+  <OverallResults successes="1178" failures="125" expectedFailures="21"/>
 </Catch>

projects/SelfTest/UsageTests/Message.tests.cpp

@@ -9,10 +9,6 @@
 #include "catch.hpp"
 #include <iostream>
 
-#ifdef __clang__
-#pragma clang diagnostic ignored "-Wc++98-compat-pedantic"
-#endif
-
 TEST_CASE( "INFO and WARN do not abort tests", "[messages][.]" ) {
     INFO( "this is a " << "message" );    // This should output the message if a failure occurs
     WARN( "this is a " << "warning" );    // This should always output the message but then continue
@@ -135,3 +131,60 @@ TEST_CASE( "Pointers can be converted to strings", "[messages][.][approvals]" )
     WARN( "actual address of p: " << &p );
     WARN( "toString(p): " << ::Catch::Detail::stringify( &p ) );
 }
+
+TEST_CASE( "CAPTURE can deal with complex expressions", "[messages][capture]" ) {
+    int a = 1;
+    int b = 2;
+    int c = 3;
+    CAPTURE( a, b, c, a + b, a+b, c > b, a == 1 );
+    SUCCEED();
+}
+
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-value" // In (1, 2), the "1" is unused ...
+#endif
+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-value" // All the comma operators are side-effect free
+#endif
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable:4709) // comma in indexing operator
+#endif
+
+template <typename T1, typename T2>
+struct helper_1436 {
+    helper_1436(T1 t1, T2 t2):
+        t1{ t1 },
+        t2{ t2 }
+    {}
+    T1 t1;
+    T2 t2;
+};
+
+template <typename T1, typename T2>
+std::ostream& operator<<(std::ostream& out, helper_1436<T1, T2> const& helper) {
+    out << "{ " << helper.t1 << ", " << helper.t2 << " }";
+    return out;
+}
+
+TEST_CASE("CAPTURE can deal with complex expressions involving commas", "[messages][capture]") {
+    CAPTURE(std::vector<int>{1, 2, 3}[0, 1, 2],
+            std::vector<int>{1, 2, 3}[(0, 1)],
+            std::vector<int>{1, 2, 3}[0]);
+    CAPTURE((helper_1436<int, int>{12, -12}),
+            (helper_1436<int, int>(-12, 12)));
+    CAPTURE( (1, 2), (2, 3) );
+    SUCCEED();
+}
+
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif