[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.

Looks great, just a couple of style nits in the tests.



================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:82
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
----------------
Is this include still necessary?


================
Comment at: unittests/Signals/UnixSignalsTest.cpp:43
+
+#define ASSERT_EQ_ARRAYS(expected, observed)                                   \
+  AssertEqArrays((expected), (observed), __FILE__, __LINE__);
----------------
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).


================
Comment at: unittests/Signals/UnixSignalsTest.cpp:64
+      signals.GetSignalInfo(signo, should_suppress, should_stop, should_notify);
+  EXPECT_EQ(name, "SIG4");
+  EXPECT_EQ(should_suppress, true);
----------------
We generally put the "expected" value first, and the observed second. The ASSERT_ macro does that, but here you seem to have inverted it.


https://reviews.llvm.org/D30520