[PATCH] D125780: [llvm-debuginfo-analyzer] 05 - Select elements

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVOptions.h:556
+
+public:
+  LVPatterns() {
----------------
probinson wrote:
> No need to repeat `public:`
Removed `public:`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVOptions.h:634
+
+public:
+  // Conditions to print an object.
----------------
probinson wrote:
> No need to repeat `public:`
Removed `public:`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVSymbol.h:163
+
 public:
   void print(raw_ostream &OS, bool Full = true) const override;
----------------
probinson wrote:
> No need to repeat `public:`
Removed `public:`.


================
Comment at: llvm/unittests/DebugInfo/LogicalView/SelectElementsTest.cpp:54
+    T *Element = new (std::nothrow) T();
+    EXPECT_NE(Element, nullptr);
+    (Element->*Function)();
----------------
probinson wrote:
> Use `ASSERT_NE` to bail out of the test if the allocation fails.
`ASSERT_NE` can be used only inside functions that don't return a value.

See https://google.github.io/googletest/advanced.html#assertion-placement

> The one constraint is that assertions that generate a fatal failure (FAIL* and ASSERT_*) can only be used in void-returning functions.



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

https://reviews.llvm.org/D125780