[PATCH] D96409: [debug-info] refactor emitDwarfUnitLength

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/lib/MC/MCStreamer.cpp:1013
+  MCSymbol *Lo = Context.createTempSymbol();
+  
   emitAbsoluteSymbolDiff(
----------------
The line contains trailing space. Please, remove.


================
Comment at: llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp:330
     Hi = TestPrinter->getCtx().createTempSymbol();
     Lo = TestPrinter->getCtx().createTempSymbol();
+
----------------
`Lo` is no longer used in the tests and should be removed.


================
Comment at: llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp:332
+
+    Sec = TestPrinter->getCtx().getELFSection(".tst", ELF::SHT_PROGBITS, 0);
+    TestPrinter->getMS().SwitchSection(Sec);
----------------
`Sec` should be local to the `init` method.


================
Comment at: llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp:346
 
-  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, Lo, 4));
-  TestPrinter->getAP()->emitDwarfUnitLength(Hi, Lo, "");
+  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, _, 4));
+  TestPrinter->getAP()->emitDwarfUnitLength(Hi, "");
----------------
I'd prefer to check that a label passed as the second argument is emitted after calling `emitAbsoluteSymbolDiff()`, but that requires many changes not directly connected with the purpose of this patch, so that should be done separately. I'll prepare the patch a bit later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96409