[PATCH] D14099: [LLVMSymbolize] Move ModuleInfo into a separate class (SymbolizableModule).

[Eric Christopher via llvm-commits]

echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Meta: Not a big fan of SymbolizableModule as a name. Especially as we're adding actual Modules with debug info it's going to get confusing. SymbolizableUnit maybe? *shrug*

Couple of inline comments, but otherwise it looks fine.

-eric


================
Comment at: lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp:213-220
@@ +212,10 @@
+
+  if (DebugInfoContext) {
+    InlinedContext = DebugInfoContext->getInliningInfoForAddress(
+        ModuleOffset, getDILineInfoSpecifier(FNKind));
+  }
+  // Make sure there is at least one frame in context.
+  if (InlinedContext.getNumberOfFrames() == 0) {
+    InlinedContext.addFrame(DILineInfo());
+  }
+  // Override the function name in lower frame with name from symbol table.
----------------
Coding style nit.

================
Comment at: lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp:224-238
@@ +223,17 @@
+    DIInliningInfo PatchedInlinedContext;
+    for (uint32_t i = 0, n = InlinedContext.getNumberOfFrames(); i < n; i++) {
+      DILineInfo LineInfo = InlinedContext.getFrame(i);
+      if (i == n - 1) {
+        std::string FunctionName;
+        uint64_t Start, Size;
+        if (getNameFromSymbolTable(SymbolRef::ST_Function, ModuleOffset,
+                                   FunctionName, Start, Size)) {
+          LineInfo.FunctionName = FunctionName;
+        }
+      }
+      PatchedInlinedContext.addFrame(LineInfo);
+    }
+    InlinedContext = PatchedInlinedContext;
+  }
+  return InlinedContext;
+}
----------------
Could probably use some more comments here, also if there's a way to unindent a bit of this that'd be awesome.


http://reviews.llvm.org/D14099