[Lldb-commits] [PATCH] D83441: [ldb/Reproducers] Add YamlRecorder and MultiProvider

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Jul 10 03:52:44 PDT 2020


labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This seems fine. A bit more doxygen wouldn't hurt...



================
Comment at: lldb/source/Utility/Reproducer.cpp:10
 #include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBAssert.h"
----------------
It doesn't look like this is actually needed.


================
Comment at: lldb/unittests/Utility/ReproducerTest.cpp:65-66
+struct YamlData {
+  YamlData() : i(-1){};
+  YamlData(int i) : i(i){};
+  int i;
----------------
superfluous semicolons


================
Comment at: lldb/unittests/Utility/ReproducerTest.cpp:74-75
+
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(YamlData)
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(std::vector<YamlData>)
+
----------------
Are both of these needed? I would expect the first one to be enough...


================
Comment at: lldb/unittests/Utility/ReproducerTest.cpp:240-242
+      ASSERT_EQ(data.size(), 2U);
+      EXPECT_EQ(data[0], data0);
+      EXPECT_EQ(data[1], data1);
----------------
`EXPECT_THAT(data, testing::ElementsAre(data0, data1));`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83441/new/

https://reviews.llvm.org/D83441





More information about the lldb-commits mailing list