labath created this revision. labath added reviewers: davide, zturner, jingham, clayborg. Herald added a subscriber: emaste.
The difference between this and regular LLDB_LOG is that this one clears the error object unconditionally. This was inspired by the ObjectFileELF bug (r322664), where the error object was being cleared only if logging was enabled. Of course, one could argue that since our logging is optional, it does not really qualify as "handling" the error and we should leave it up to the programmer to explicitly clear it if he really wants to. It's not really clear to me which position is better. What do you think? https://reviews.llvm.org/D42182 Files: include/lldb/Utility/Log.h source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp unittests/Utility/LogTest.cpp
Index: unittests/Utility/LogTest.cpp =================================================================== --- unittests/Utility/LogTest.cpp +++ unittests/Utility/LogTest.cpp @@ -240,6 +240,23 @@ logAndTakeOutput("Hello World")); } +TEST_F(LogChannelEnabledTest, LLDB_LOG_ERROR) { + LLDB_LOG_ERROR(getLog(), llvm::Error::success(), "Foo failed: {0}"); + ASSERT_EQ("", takeOutput()); + + LLDB_LOG_ERROR(getLog(), + llvm::make_error<llvm::StringError>( + "My Error", llvm::inconvertibleErrorCode()), + "Foo failed: {0}"); + ASSERT_EQ("Foo failed: My Error\n", takeOutput()); + + // Doesn't log, but doesn't assert either + LLDB_LOG_ERROR(nullptr, + llvm::make_error<llvm::StringError>( + "My Error", llvm::inconvertibleErrorCode()), + "Foo failed: {0}"); +} + TEST_F(LogChannelEnabledTest, LogThread) { // Test that we are able to concurrently write to a log channel and disable // it. Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp =================================================================== --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -3491,19 +3491,18 @@ size_t(section_data.GetByteSize())}, GetByteOrder() == eByteOrderLittle, GetAddressByteSize() == 8); if (!Decompressor) { - LLDB_LOG(log, "Unable to initialize decompressor for section {0}: {1}", - section->GetName(), llvm::toString(Decompressor.takeError())); - consumeError(Decompressor.takeError()); + LLDB_LOG_ERROR(log, Decompressor.takeError(), + "Unable to initialize decompressor for section {0}", + section->GetName()); return result; } auto buffer_sp = std::make_shared<DataBufferHeap>(Decompressor->getDecompressedSize(), 0); if (auto Error = Decompressor->decompress( {reinterpret_cast<char *>(buffer_sp->GetBytes()), size_t(buffer_sp->GetByteSize())})) { - LLDB_LOG(log, "Decompression of section {0} failed: {1}", - section->GetName(), llvm::toString(std::move(Error))); - consumeError(std::move(Error)); + LLDB_LOG_ERROR(log, std::move(Error), "Decompression of section {0} failed", + section->GetName()); return result; } section_data.SetData(buffer_sp); Index: include/lldb/Utility/Log.h =================================================================== --- include/lldb/Utility/Log.h +++ include/lldb/Utility/Log.h @@ -17,6 +17,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringMap.h" // for StringMap #include "llvm/ADT/StringRef.h" // for StringRef, StringLiteral +#include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/ManagedStatic.h" #include "llvm/Support/RWMutex.h" @@ -140,6 +141,15 @@ Format(file, function, llvm::formatv(format, std::forward<Args>(args)...)); } + template <typename... Args> + void FormatError(llvm::Error error, llvm::StringRef file, + llvm::StringRef function, const char *format, + Args &&... args) { + Format(file, function, + llvm::formatv(format, llvm::toString(std::move(error)), + std::forward<Args>(args)...)); + } + void Printf(const char *format, ...) __attribute__((format(printf, 2, 3))); void VAPrintf(const char *format, va_list args); @@ -218,4 +228,17 @@ log_private->Format(__FILE__, __FUNCTION__, __VA_ARGS__); \ } while (0) +// Write message to log, if error is set. In the log message refer to the error +// with {0}. Error is cleared regardless of whether logging is enabled. +#define LLDB_LOG_ERROR(log, error, ...) \ + do { \ + ::lldb_private::Log *log_private = (log); \ + ::llvm::Error error_private = (error); \ + if (log_private && error_private) { \ + log_private->FormatError(::std::move(error_private), __FILE__, \ + __FUNCTION__, __VA_ARGS__); \ + } else \ + ::llvm::consumeError(::std::move(error_private)); \ + } while (0) + #endif // LLDB_UTILITY_LOG_H
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits