[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.





================
Comment at: unittests/Signals/UnixSignalsTest.cpp:43
+
+#define ASSERT_EQ_ARRAYS(expected, observed)                                   \
+  AssertEqArrays((expected), (observed), __FILE__, __LINE__);
----------------
eugene wrote:
> labath wrote:
> > This (and the function) should probably have an EXPECT_.. prefix instead, as it does not abort the evaluation of the function it is in (like other ASSERT_*** macros).
> This function calls ASSERT_EQ so it does abort evaluation.
Unfortunately it does not. If you look up what ASSERT_EQ does, you'll see that it basically amounts to `EXPECT_EQ(a,b); return;` <https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#assertion-placement>, <https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#using-assertions-in-sub-routines>. So it does abort, but only the top most frame. Since your topmost frame is the `AssertEqArrays` function, it will just do an early return from that, and the caller will happily continue. If you wanted to be fancy you could define the macro to something like `CompareArrays(...); if(HasFailure()) return;`. Another solution would be to write a comparison function that returns an `AssertionResult` <https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#using-a-function-that-returns-an-assertionresult> and then use that in EXPECT_TRUE/ASSERT_TRUE  as you see fit (but that's probably overkill).


https://reviews.llvm.org/D30520