[PATCH] D134817: Remove the dependency between lib/DebugInfoDWARF and MC

[Greg Clayton via Phabricator via llvm-commits]

clayborg added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DIContext.h:204
+      GetNameForDWARFReg = nullptr;
+  bool isEH = false;
 
----------------
Case should be "IsEH" according to llvm guidelines.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:165-167
+  void dump(raw_ostream &OS, bool IsEH,
+            std::function<Optional<StringRef>(uint64_t RegNum, bool isEH)>
+                GetNameForDWARFReg = nullptr) const;
----------------
Do we want to just pass in DIDumpOptions here as well?


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:225-227
+  void dump(raw_ostream &OS, bool IsEH,
+            std::function<Optional<StringRef>(uint64_t RegNum, bool isEH)>
+                GetNameForDWARFReg = nullptr) const;
----------------
ditto


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:308-310
+  void dump(raw_ostream &OS, bool IsEH,
+            std::function<Optional<StringRef>(uint64_t RegNum, bool isEH)>
+                GetNameForDWARFReg = nullptr,
----------------
ditto


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:355-357
+  void dump(raw_ostream &OS, bool IsEH,
+            std::function<Optional<StringRef>(uint64_t RegNum, bool isEH)>
+                GetNameForDWARFReg = nullptr,
----------------
ditto


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:118
+  case DWARFExpr: {
+    auto DumpOpts = DIDumpOptions();
+    DumpOpts.GetNameForDWARFReg = GetNameForDWARFReg;
----------------
Seeing this is another reason we might want to pass in DIDumpOptions to this function. Then we wouldn't need to create a temp on here.


================
Comment at: llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:559-560
 
+static Error createRegInfo(const object::ObjectFile &Obj,
+                           std::unique_ptr<MCRegisterInfo> &MCRegInfo) {
+  Triple TT;
----------------
Use Expected<std::unique_ptr<MCRegisterInfo>> as the return value?


================
Comment at: llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp:288
                              "unable to create DWARF context");
-  logAllUnhandledErrors(DICtx->loadRegisterInfo(Obj), OS, "DwarfTransformer: ");
 
----------------
Was this not needed?


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

https://reviews.llvm.org/D134817