Author: David Spickett Date: 2026-07-14T11:29:00Z New Revision: edd94b8fbfae72059cbe31c78ac4580056b6bb9a
URL: https://github.com/llvm/llvm-project/commit/edd94b8fbfae72059cbe31c78ac4580056b6bb9a DIFF: https://github.com/llvm/llvm-project/commit/edd94b8fbfae72059cbe31c78ac4580056b6bb9a.diff LOG: [lldb] Return llvm::Error from DisableLogChannel (#207004) Follow up to #206479. llvm:Error better describes the success exor error message states that we were previously doing with a boolean plus an error stream. Added: Modified: lldb/include/lldb/Utility/Log.h lldb/source/Commands/CommandObjectLog.cpp lldb/source/Utility/Log.cpp lldb/unittests/Utility/LogTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h index a904dd5680767..636584dac3cce 100644 --- a/lldb/include/lldb/Utility/Log.h +++ b/lldb/include/lldb/Utility/Log.h @@ -203,9 +203,8 @@ class Log final { uint32_t log_options, llvm::StringRef channel, llvm::ArrayRef<const char *> categories); - static bool DisableLogChannel(llvm::StringRef channel, - llvm::ArrayRef<const char *> categories, - llvm::raw_ostream &error_stream); + static llvm::Error DisableLogChannel(llvm::StringRef channel, + llvm::ArrayRef<const char *> categories); static bool DumpLogChannel(llvm::StringRef channel, llvm::raw_ostream &output_stream, diff --git a/lldb/source/Commands/CommandObjectLog.cpp b/lldb/source/Commands/CommandObjectLog.cpp index edce90e864fc9..07abbbc919c7d 100644 --- a/lldb/source/Commands/CommandObjectLog.cpp +++ b/lldb/source/Commands/CommandObjectLog.cpp @@ -271,13 +271,11 @@ class CommandObjectLogDisable : public CommandObjectParsed { Log::DisableAllLogChannels(); result.SetStatus(eReturnStatusSuccessFinishNoResult); } else { - std::string error; - llvm::raw_string_ostream error_stream(error); - if (Log::DisableLogChannel(channel, args.GetArgumentArrayRef(), - error_stream)) - result.SetStatus(eReturnStatusSuccessFinishNoResult); + if (llvm::Error err = + Log::DisableLogChannel(channel, args.GetArgumentArrayRef())) + result.AppendError(toString(std::move(err))); else - result.AppendError(error); + result.SetStatus(eReturnStatusSuccessFinishNoResult); } } }; diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp index ed5dc5701cd88..2ed1444fbb5e6 100644 --- a/lldb/source/Utility/Log.cpp +++ b/lldb/source/Utility/Log.cpp @@ -247,28 +247,23 @@ Log::EnableLogChannel(const std::shared_ptr<LogHandler> &log_handler_sp, return llvm::Error::success(); } -bool Log::DisableLogChannel(llvm::StringRef channel, - llvm::ArrayRef<const char *> categories, - llvm::raw_ostream &error_stream) { +llvm::Error Log::DisableLogChannel(llvm::StringRef channel, + llvm::ArrayRef<const char *> categories) { auto iter = g_channel_map->find(channel); - if (iter == g_channel_map->end()) { - error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel); - return false; - } + if (iter == g_channel_map->end()) + return llvm::createStringErrorV("Invalid log channel '{0}'.\n", channel); if (categories.empty()) { iter->second.Disable(std::nullopt); - return true; + return llvm::Error::success(); } llvm::Expected<MaskType> flags = GetFlags(*iter, categories); - if (!flags) { - error_stream << toString(flags.takeError()) << "\n"; - return false; - } + if (!flags) + return flags.takeError(); iter->second.Disable(*flags); - return true; + return llvm::Error::success(); } bool Log::DumpLogChannel(llvm::StringRef channel, diff --git a/lldb/unittests/Utility/LogTest.cpp b/lldb/unittests/Utility/LogTest.cpp index 328e20fd53b2a..f8514930eac2b 100644 --- a/lldb/unittests/Utility/LogTest.cpp +++ b/lldb/unittests/Utility/LogTest.cpp @@ -40,15 +40,7 @@ namespace lldb_private { template <> Log::Channel &LogChannelFor<TestChannel>() { return test_channel; } } // namespace lldb_private -// Wrap disable and list functions to make them easier to test. -static bool DisableChannel(llvm::StringRef channel, - llvm::ArrayRef<const char *> categories, - std::string &error) { - error.clear(); - llvm::raw_string_ostream error_stream(error); - return Log::DisableLogChannel(channel, categories, error_stream); -} - +// Wrap list function to make it easier to test. static bool ListCategories(llvm::StringRef channel, std::string &result) { result.clear(); llvm::raw_string_ostream result_stream(result); @@ -263,17 +255,22 @@ TEST_F(LogChannelTest, Disable) { EXPECT_NE(nullptr, GetLog(TestChannel::BAR)); std::string error; - EXPECT_TRUE(DisableChannel("chan", {"bar"}, error)); + EXPECT_THAT_ERROR(Log::DisableLogChannel("chan", {"bar"}), llvm::Succeeded()); EXPECT_NE(nullptr, GetLog(TestChannel::FOO)); EXPECT_EQ(nullptr, GetLog(TestChannel::BAR)); - EXPECT_FALSE(DisableChannel("chan", {"baz"}, error)); - EXPECT_NE(std::string::npos, error.find("unrecognized log category 'baz'")) - << "error: " << error; + EXPECT_THAT_ERROR( + Log::DisableLogChannel("chan", {"baz"}), + llvm::FailedWithMessage("error: unrecognized log category 'baz'\n" + "Logging categories for 'chan':\n" + " all - all available logging categories\n" + " default - default set of logging categories\n" + " foo - log foo\n" + " bar - log bar\n")); EXPECT_NE(nullptr, GetLog(TestChannel::FOO)); EXPECT_EQ(nullptr, GetLog(TestChannel::BAR)); - EXPECT_TRUE(DisableChannel("chan", {}, error)); + EXPECT_THAT_ERROR(Log::DisableLogChannel("chan", {}), llvm::Succeeded()); EXPECT_EQ(nullptr, GetLog(TestChannel::FOO | TestChannel::BAR)); } @@ -413,11 +410,9 @@ TEST_F(LogChannelEnabledTest, LLDB_LOG_ERROR) { TEST_F(LogChannelEnabledTest, LogThread) { // Test that we are able to concurrently write to a log channel and disable // it. - std::string err; - // Start logging on one thread. Concurrently, try disabling the log channel. std::thread log_thread([this] { LLDB_LOG(getLog(), "Hello World"); }); - EXPECT_TRUE(DisableChannel("chan", {}, err)); + EXPECT_THAT_ERROR(Log::DisableLogChannel("chan", {}), llvm::Succeeded()); log_thread.join(); // The log thread either managed to write to the log in time, or it didn't. In @@ -447,13 +442,12 @@ TEST_F(LogChannelEnabledTest, LogVerboseThread) { TEST_F(LogChannelEnabledTest, LogGetLogThread) { // Test that we are able to concurrently get mask of a Log object and disable // it. - std::string err; // Try fetching the log mask on one thread. Concurrently, try disabling the // log channel. uint64_t mask; std::thread log_thread([this, &mask] { mask = getLog()->GetMask(); }); - EXPECT_TRUE(DisableChannel("chan", {}, err)); + EXPECT_THAT_ERROR(Log::DisableLogChannel("chan", {}), llvm::Succeeded()); log_thread.join(); // The mask should be either zero of "FOO". In either case, we should not trip _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
