[PATCH] D154372: [DebugInfo][RemoveDIs] Plumb remove-DIs command line switch into pass managers for ease of testing

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

> These blocks of code should be considered transitory as we (hopefully) move from one debug-info format to another.

SGTM

> Thus, there /shouldn't/ be an observable change in behaviour to test for. Possibly this means that unit tests are the answer, although I'm open to suggestions.

I guess it could be possible to unittest the print functions. IMO if a bot is running with this mode enabled the IR tests should be providing decent coverage for the print functions, and `convertToNewDbgValues` is tested in other patches (right?), so this is probably ok as-is.



================
Comment at: llvm/include/llvm/IR/PassManager.h:62
 
+extern llvm::cl::opt<bool> UseNewDbgInfoFormat;
+
----------------
Have you tested building LLVM with `-DBUILD_SHARED_LIBS=TRUE`?


================
Comment at: llvm/include/llvm/IR/PassManager.h:73
+template <class IRUnitT> inline void doConvertDebugInfoToOld(IRUnitT &IR) {}
+template <> inline void doConvertDebugInfoToOld(Module &IR) { IR.convertFromNewDbgValues(); }
+
----------------
clang-format


================
Comment at: llvm/lib/IR/AsmWriter.cpp:290
+static const Module *getModuleFromDPI(const DPMarker *Marker) {
+  const Function *M = Marker->getParent() ? Marker->getParent()->getParent() : nullptr;
+  return M ? M->getParent() : nullptr;
----------------
needs clang-formatting?


================
Comment at: llvm/lib/IR/AsmWriter.cpp:4035-4036
 
   if (AnnotationWriter)
     AnnotationWriter->printInfoComment(V, Out);
+  else if (const Instruction *I = dyn_cast<Instruction>(&V)) {
----------------
nit: add braces to `if`


================
Comment at: llvm/lib/IR/BasicBlock.cpp:161
+#ifndef NDEBUG
+void BasicBlock::dumpDbgValues() const {
+  for (auto &Inst : *this) {
----------------
missing declaration in `BasicBlock.h`?


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

https://reviews.llvm.org/D154372