[PATCH] D63662: Changing CodeView Debug info Type record output

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:644-650
+    TypeVisitorCallbackPipeline Pipeline;
+    CVMCAdapter CVMCOS(OS);
+    TypeRecordMapping typeMapping(CVMCOS);
+    SmallString<512> CommentBlock;
+    raw_svector_ostream CommentOS(CommentBlock);
+    std::unique_ptr<ScopedPrinter> SP;
+    std::unique_ptr<TypeDumpVisitor> TDV;
----------------
It would be ideal if we didn't re-construct the entire pipeline and heap allocate the visitor and printer inside the loop. Is it possible to set it up outside the loop? I recall that there were problems with the CommentBlock accumulating multiple comments. Is it enough to add `CommentBlock.clear()`, and then move the rest outside the loop?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:654
       // Emit a block comment describing the type record for readability.
-      SmallString<512> CommentBlock;
-      raw_svector_ostream CommentOS(CommentBlock);
-      ScopedPrinter SP(CommentOS);
-      SP.setPrefix(CommentPrefix);
-      TypeDumpVisitor TDV(Table, &SP, false);
-
-      Error E = codeview::visitTypeRecord(Record, *B, TDV);
-      if (E) {
-        logAllUnhandledErrors(std::move(E), errs(), "error: ");
-        llvm_unreachable("produced malformed type record");
-      }
+      SP = std::unique_ptr<ScopedPrinter>(new ScopedPrinter(CommentOS));
+      SP->setPrefix(CommentPrefix);
----------------
`llvm::make_unique<ScopedPrinter>(CommentOS)` is preferred to avoid repeating ScopedPrinter. It is modeled on `std::make_unique` from C++14.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:656
+      SP->setPrefix(CommentPrefix);
+      TDV = std::unique_ptr<TypeDumpVisitor>(new TypeDumpVisitor(Table, SP.get(), false));
+      Pipeline.addCallbackToPipeline(*TDV);
----------------
ditto


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63662