[PATCH] D74308: [Debuginfo][NFC] Unify error reporting routines inside DebugInfoDWARF.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 18 03:08:47 PST 2020
jhenderson added inline comments.
================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:115
+ std::string DWPName = "",
+ std::function<void(Error)> TheRecoverableErrorHandler =
+ WithColor::defaultErrorHandler,
----------------
This can just be `RecoverablErrorHandler`. No need for the `The` - constructor arguments can be named the same as the thing they're initialising. Same goes throughout most of the other functions too.
================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:362
/// message and returns Continue, so DWARF context ignores the error.
- static ErrorPolicy defaultErrorHandler(Error E);
+ static ErrorPolicy objectFileErrorsHandler(Error E);
----------------
objectFileErrorsHandler -> objectFileErrorHandler
================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:366-367
create(const object::ObjectFile &Obj, const LoadedObjectInfo *L = nullptr,
- function_ref<ErrorPolicy(Error)> HandleError = defaultErrorHandler,
- std::string DWPName = "");
+ function_ref<ErrorPolicy(Error)> TheObjectFileErrorsHandler =
+ objectFileErrorsHandler,
+ std::string DWPName = "",
----------------
I don't think you need to pass this in as well as a RecoverableErrorHandler. The two shouldn't need to diverge, I think. Rather, have ObjectFileErrorHandler be defined in terms of RecoverableErrorHandler, as suggested below.
================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1432
-ErrorPolicy DWARFContext::defaultErrorHandler(Error E) {
+ErrorPolicy DWARFContext::objectFileErrorsHandler(Error E) {
WithColor::defaultErrorHandler(std::move(E));
----------------
Shouldn't this function use the provided error handler rather than the default one?
In fact, maybe this should just be an inline lambda now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74308/new/
https://reviews.llvm.org/D74308
More information about the llvm-commits
mailing list