[PATCH] D74308: [Debuginfo][NFC] Unify error reporting routines inside DebugInfoDWARF.

[Alexey Lapshin via Phabricator via llvm-commits]

avl marked 3 inline comments as done.
avl added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:355
+  };
+  std::function<void(Error E)> getRecoverableErrorHandler() {
+    return RecoverableErrorHandler;
----------------
jhenderson wrote:
> Having a setter and getter for these seems a bit verbose, especially when you've got cases where you use the getter and then immediately call the return. Is there any particular reason to hide the handlers behind the interface?
There is no particular reason. Having getter/setter allows to add some code like checking for nullptr, or dumping, or something else... If it looks verbose I would remove it. 


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:362
+             Unit.getOrigUnit().getContext().getRecoverableErrorHandler(),
+             8 /* Indent */, DumpOpts);
   }
----------------
jhenderson wrote:
> I know it's not related to your change, but since you are touching the line anyway, could you change `8 /* Indent */` to `/*Indent=*/8`, please, as clang-format plays well with that.
Ok.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDie.cpp:642
+LLVM_DUMP_METHOD void DWARFDie::dump() const {
+  dump(llvm::errs(), defaultErrorHandler, 0);
+}
----------------
jhenderson wrote:
> Have you considered using DWARFContext::defaultErrorHandler here?
Ah, indeed. Would use it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74308