[PATCH] D87008: [DebugInfo] Fix methods of AsmPrinter to emit values corresponding to the DWARF format (1/19).

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp:34
+    setupTestPrinter(TripleStr, DwarfVersion, DwarfFormat);
+    return static_cast<bool>(TestPrinter);
+  }
----------------
dblaikie wrote:
> Maybe nicer to write this as "return TestPrinter != nullptr;" ? (if you've got to write it out verbosely anyway due to the explicit conversion)
I'll update that to `return TestPrinter != nullptr;` then, thanks.


================
Comment at: llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp:153
+  ASSERT_TRUE(ActualArg0 != nullptr);
+  EXPECT_EQ(&(ActualArg0->getSymbol()), Val.Symbol);
+}
----------------
dblaikie wrote:
> Ekxtra parens look like they're unnecessary here?
They are added only to improve readability. They are not required for the compiler, sure. Do you prefer them to be removed?


================
Comment at: llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp:176
+  const MCSymbolRefExpr *ActualArg0 = dyn_cast_or_null<MCSymbolRefExpr>(Arg0);
+  ASSERT_TRUE(ActualArg0 != nullptr);
+  EXPECT_EQ(&(ActualArg0->getSymbol()), Val.Symbol);
----------------
dblaikie wrote:
> Is there an assert non-null you could use more directly?
Apart from more exotic variants, we can use `ASSERT_TRUE(ActualArg0)`, `ASSERT_TRUE(ActualArg0 != nullptr)` or `ASSERT_NE(ActualArg0, nullptr)` here. I have chosen a variant based on my understanding of expressibility. What is your preference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87008