[PATCH] D30897: [WIP] New AsmPrinterHandler that is able to interleave source code

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

Hi Roger,

thank you for working on this. I have some minor comments, good job!

A test would also make clear what would be the desired output of this new functionality.

Francesco



================
Comment at: include/llvm/MC/MCTargetOptions.h:26-31
+enum class AsmSourceOutput {
+    None,
+    Alone,
+    WithDebug
+};
+
----------------
Please describe this enumeration.


================
Comment at: lib/CodeGen/AsmPrinter/SourceInterleave.cpp:55-64
+std::string SourceInterleave::getFullPathname(const DebugLoc &DL) {
+  std::string Dir;
+  std::string Fn;
+  if (DIScope *S = DL->getScope()) {
+    Dir = S->getDirectory();
+    Fn = S->getFilename();
+  }
----------------
You could use `DebugLoc::print` (or factor out part of it if you don't need column and line number).


================
Comment at: lib/CodeGen/AsmPrinter/SourceInterleave.cpp:99
+
+  std::string FullPathname = getFullPathname(DL);
+  SourceCodeInfo *SI = getOrCreateSourceCodeInfo(FullPathname);
----------------
Nitpicking: `const std:string`?


================
Comment at: lib/CodeGen/AsmPrinter/SourceInterleave.cpp:107
+
+  int CurrentLine = DL->getLine();
+  if (CurrentLine == 0)
----------------
Nitpicking: `const int`?


================
Comment at: lib/CodeGen/AsmPrinter/SourceInterleave.cpp:136
+void SourceInterleave::endInstruction() {}
+}
----------------
`}// end namespace llvm`


https://reviews.llvm.org/D30897