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

[Shubham Sandeep Rastogi via Phabricator via llvm-commits]

rastogishubham added inline comments.


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:73
+
+  std::function<llvm::Optional<llvm::StringRef>(uint64_t DwarfRegNum,
+                                                bool isEH)>
----------------
aprantl wrote:
> Since a `StringRef` can be `empty()`, it usually doesn't make a lot of sense to wrap it inside of `Optional`. I would just use a plain `StringRef`.
The reason I used an Optional is because in my mind the RegName for a register can be empty itself, maybe? The Optional tells me that there was no RegName or no LLVMRegNum found and helps me output any error messages if needed.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:338
     } else {
-      Loc.dumpRange(Offset, EndOffset - Offset, OS, MRI, Obj, DumpOpts);
+      Loc.dumpRange(Offset, EndOffset - Offset, OS, Obj, DumpOpts, Callback);
     }
----------------
aprantl wrote:
> Would there any benefit of having DWARFContext own Callback instead of passing it through here everywhere?
Not sure what you mean, DWARFContext dows own Callback, but this is a static function which is not related to class DWARFContext, so Callback has to be passed as an argument


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

https://reviews.llvm.org/D134817