https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/93209
>From c5738598436c7a9e1ba38ba8ffd6f67a2e524785 Mon Sep 17 00:00:00 2001 From: Adrian Prantl <apra...@apple.com> Date: Thu, 23 May 2024 08:36:48 -0700 Subject: [PATCH] Add a createError variant without error code (NFC) For the significant amount of call sites that want to create an incontrovertible error, such a wrapper function creates a significant readability improvement and lowers the cost of entry to add error handling in more places. --- lldb/source/Host/common/Socket.cpp | 3 +- lldb/source/Interpreter/Options.cpp | 29 ++++++--------- .../Plugins/ABI/PowerPC/ABISysV_ppc64.cpp | 8 ++--- .../Python/ScriptInterpreterPython.cpp | 9 ++--- .../SymbolFile/Breakpad/SymbolFileBreakpad.h | 5 ++- .../TypeSystem/Clang/TypeSystemClang.cpp | 15 +++----- lldb/source/Symbol/CompilerType.cpp | 3 +- lldb/source/Symbol/Symbol.cpp | 18 ++++------ lldb/source/Symbol/SymbolFileOnDemand.cpp | 5 ++- lldb/source/Symbol/TypeSystem.cpp | 32 +++++++---------- lldb/source/Target/Target.cpp | 36 ++++++++----------- lldb/source/Utility/Status.cpp | 3 +- llvm/include/llvm/Support/Error.h | 5 +++ 13 files changed, 68 insertions(+), 103 deletions(-) diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp index bd0c127a08956..f9911cf136cbd 100644 --- a/lldb/source/Host/common/Socket.cpp +++ b/lldb/source/Host/common/Socket.cpp @@ -87,8 +87,7 @@ llvm::Error Socket::Initialize() { if (err == 0) { if (wsaData.wVersion < wVersion) { WSACleanup(); - return llvm::make_error<llvm::StringError>( - "WSASock version is not expected.", llvm::inconvertibleErrorCode()); + return llvm::createStringError("WSASock version is not expected."); } } else { return llvm::errorCodeToError(llvm::mapWindowsError(::WSAGetLastError())); diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp index 51b7e6b26b6ef..4e7d074ace1b8 100644 --- a/lldb/source/Interpreter/Options.cpp +++ b/lldb/source/Interpreter/Options.cpp @@ -931,8 +931,7 @@ llvm::Expected<Args> Options::ParseAlias(const Args &args, Option *long_options = GetLongOptions(); if (long_options == nullptr) { - return llvm::make_error<llvm::StringError>("Invalid long options", - llvm::inconvertibleErrorCode()); + return llvm::createStringError("Invalid long options"); } std::string short_options = BuildShortOptions(long_options); @@ -957,8 +956,7 @@ llvm::Expected<Args> Options::ParseAlias(const Args &args, break; if (val == '?') { - return llvm::make_error<llvm::StringError>( - "Unknown or ambiguous option", llvm::inconvertibleErrorCode()); + return llvm::createStringError("Unknown or ambiguous option"); } if (val == 0) @@ -980,9 +978,8 @@ llvm::Expected<Args> Options::ParseAlias(const Args &args, // See if the option takes an argument, and see if one was supplied. if (long_options_index == -1) { - return llvm::make_error<llvm::StringError>( - llvm::formatv("Invalid option with value '{0}'.", char(val)).str(), - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + llvm::formatv("Invalid option with value '{0}'.", char(val)).str()); } StreamString option_str; @@ -995,11 +992,10 @@ llvm::Expected<Args> Options::ParseAlias(const Args &args, switch (has_arg) { case OptionParser::eRequiredArgument: if (OptionParser::GetOptionArgument() == nullptr) { - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( llvm::formatv("Option '{0}' is missing argument specifier.", option_str.GetString()) - .str(), - llvm::inconvertibleErrorCode()); + .str()); } [[fallthrough]]; case OptionParser::eOptionalArgument: @@ -1008,12 +1004,11 @@ llvm::Expected<Args> Options::ParseAlias(const Args &args, case OptionParser::eNoArgument: break; default: - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( llvm::formatv("error with options table; invalid value in has_arg " "field for option '{0}'.", char(val)) - .str(), - llvm::inconvertibleErrorCode()); + .str()); } // Find option in the argument list; also see if it was supposed to take an // argument and if one was supplied. Remove option (and argument, if @@ -1261,8 +1256,7 @@ llvm::Expected<Args> Options::Parse(const Args &args, Status error; Option *long_options = GetLongOptions(); if (long_options == nullptr) { - return llvm::make_error<llvm::StringError>("Invalid long options.", - llvm::inconvertibleErrorCode()); + return llvm::createStringError("Invalid long options."); } std::string short_options = BuildShortOptions(long_options); @@ -1322,9 +1316,8 @@ llvm::Expected<Args> Options::Parse(const Args &args, if (!platform_sp && require_validation) { // Caller requires validation but we cannot validate as we don't have // the mandatory platform against which to validate. - return llvm::make_error<llvm::StringError>( - "cannot validate options: no platform available", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "cannot validate options: no platform available"); } bool validation_failed = false; diff --git a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp index 3d9b4566ca1c9..b96f9d4deb37c 100644 --- a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp +++ b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp @@ -501,14 +501,12 @@ class ReturnValueExtractor { CompilerType &type) { RegisterContext *reg_ctx = thread.GetRegisterContext().get(); if (!reg_ctx) - return llvm::make_error<llvm::StringError>( - LOG_PREFIX "Failed to get RegisterContext", - llvm::inconvertibleErrorCode()); + return llvm::createStringError(LOG_PREFIX + "Failed to get RegisterContext"); ProcessSP process_sp = thread.GetProcess(); if (!process_sp) - return llvm::make_error<llvm::StringError>( - LOG_PREFIX "GetProcess() failed", llvm::inconvertibleErrorCode()); + return llvm::createStringError(LOG_PREFIX "GetProcess() failed"); return ReturnValueExtractor(thread, type, reg_ctx, process_sp); } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index ce52f35952478..6e676de146b3d 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -2494,8 +2494,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( auto ExtendSysPath = [&](std::string directory) -> llvm::Error { if (directory.empty()) { - return llvm::make_error<llvm::StringError>( - "invalid directory name", llvm::inconvertibleErrorCode()); + return llvm::createStringError("invalid directory name"); } replace_all(directory, "\\", "\\\\"); @@ -2508,10 +2507,8 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( directory.c_str(), directory.c_str()); bool syspath_retval = ExecuteMultipleLines(command_stream.GetData(), exc_options).Success(); - if (!syspath_retval) { - return llvm::make_error<llvm::StringError>( - "Python sys.path handling failed", llvm::inconvertibleErrorCode()); - } + if (!syspath_retval) + return llvm::createStringError("Python sys.path handling failed"); return llvm::Error::success(); }; diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h index 83215bf3c87e4..041b388f9f344 100644 --- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h +++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h @@ -120,9 +120,8 @@ class SymbolFileBreakpad : public SymbolFileCommon { llvm::Expected<lldb::TypeSystemSP> GetTypeSystemForLanguage(lldb::LanguageType language) override { - return llvm::make_error<llvm::StringError>( - "SymbolFileBreakpad does not support GetTypeSystemForLanguage", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "SymbolFileBreakpad does not support GetTypeSystemForLanguage"); } CompilerDeclContext FindNamespace(ConstString name, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 1b2235b4b2b5b..369ae46cf264a 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -5272,8 +5272,7 @@ TypeSystemClang::GetNumChildren(lldb::opaque_compiler_type_t type, bool omit_empty_base_classes, const ExecutionContext *exe_ctx) { if (!type) - return llvm::make_error<llvm::StringError>("invalid clang type", - llvm::inconvertibleErrorCode()); + return llvm::createStringError("invalid clang type"); uint32_t num_children = 0; clang::QualType qual_type(RemoveWrappingTypes(GetQualType(type))); @@ -5331,9 +5330,8 @@ TypeSystemClang::GetNumChildren(lldb::opaque_compiler_type_t type, num_children += std::distance(record_decl->field_begin(), record_decl->field_end()); } else - return llvm::make_error<llvm::StringError>( - "incomplete type \"" + GetDisplayTypeName(type).GetString() + "\"", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "incomplete type \"" + GetDisplayTypeName(type).GetString() + "\""); break; case clang::Type::ObjCObject: case clang::Type::ObjCInterface: @@ -6239,9 +6237,7 @@ llvm::Expected<CompilerType> TypeSystemClang::GetChildCompilerTypeAtIndex( std::optional<uint64_t> size = base_class_clang_type.GetBitSize(get_exe_scope()); if (!size) - return llvm::make_error<llvm::StringError>( - "no size info for base class", - llvm::inconvertibleErrorCode()); + return llvm::createStringError("no size info for base class"); uint64_t base_class_clang_type_bit_size = *size; @@ -6274,8 +6270,7 @@ llvm::Expected<CompilerType> TypeSystemClang::GetChildCompilerTypeAtIndex( std::optional<uint64_t> size = field_clang_type.GetByteSize(get_exe_scope()); if (!size) - return llvm::make_error<llvm::StringError>( - "no size info for field", llvm::inconvertibleErrorCode()); + return llvm::createStringError("no size info for field"); child_byte_size = *size; const uint32_t child_bit_size = child_byte_size * 8; diff --git a/lldb/source/Symbol/CompilerType.cpp b/lldb/source/Symbol/CompilerType.cpp index b5269cf66235c..f8da9ef7b7640 100644 --- a/lldb/source/Symbol/CompilerType.cpp +++ b/lldb/source/Symbol/CompilerType.cpp @@ -805,8 +805,7 @@ CompilerType::GetNumChildren(bool omit_empty_base_classes, if (auto type_system_sp = GetTypeSystem()) return type_system_sp->GetNumChildren(m_type, omit_empty_base_classes, exe_ctx); - return llvm::make_error<llvm::StringError>("invalid type", - llvm::inconvertibleErrorCode()); + return llvm::createStringError("invalid type"); } lldb::BasicType CompilerType::GetBasicTypeEnumeration() const { diff --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp index 1895f299cc06a..9b0042ffdb4bf 100644 --- a/lldb/source/Symbol/Symbol.cpp +++ b/lldb/source/Symbol/Symbol.cpp @@ -101,18 +101,15 @@ const Symbol &Symbol::operator=(const Symbol &rhs) { llvm::Expected<Symbol> Symbol::FromJSON(const JSONSymbol &symbol, SectionList *section_list) { if (!section_list) - return llvm::make_error<llvm::StringError>("no section list provided", - llvm::inconvertibleErrorCode()); + return llvm::createStringError("no section list provided"); if (!symbol.value && !symbol.address) - return llvm::make_error<llvm::StringError>( - "symbol must contain either a value or an address", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "symbol must contain either a value or an address"); if (symbol.value && symbol.address) - return llvm::make_error<llvm::StringError>( - "symbol cannot contain both a value and an address", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "symbol cannot contain both a value and an address"); const uint64_t size = symbol.size.value_or(0); const bool is_artificial = false; @@ -133,9 +130,8 @@ llvm::Expected<Symbol> Symbol::FromJSON(const JSONSymbol &symbol, AddressRange(section_sp, offset, size), size_is_valid, contains_linker_annotations, flags); } - return llvm::make_error<llvm::StringError>( - llvm::formatv("no section found for address: {0:x}", *symbol.address), - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + llvm::formatv("no section found for address: {0:x}", *symbol.address)); } // Absolute symbols encode the integer value in the m_offset of the diff --git a/lldb/source/Symbol/SymbolFileOnDemand.cpp b/lldb/source/Symbol/SymbolFileOnDemand.cpp index c6d9f0071c392..0cfe9fc1514b5 100644 --- a/lldb/source/Symbol/SymbolFileOnDemand.cpp +++ b/lldb/source/Symbol/SymbolFileOnDemand.cpp @@ -457,9 +457,8 @@ SymbolFileOnDemand::GetTypeSystemForLanguage(LanguageType language) { Log *log = GetLog(); LLDB_LOG(log, "[{0}] {1} is skipped for language type {2}", GetSymbolFileName(), __FUNCTION__, language); - return llvm::make_error<llvm::StringError>( - "GetTypeSystemForLanguage is skipped by SymbolFileOnDemand", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "GetTypeSystemForLanguage is skipped by SymbolFileOnDemand"); } return m_sym_file_impl->GetTypeSystemForLanguage(language); } diff --git a/lldb/source/Symbol/TypeSystem.cpp b/lldb/source/Symbol/TypeSystem.cpp index 3665771b18890..4956f10a0b0a7 100644 --- a/lldb/source/Symbol/TypeSystem.cpp +++ b/lldb/source/Symbol/TypeSystem.cpp @@ -267,9 +267,8 @@ llvm::Expected<lldb::TypeSystemSP> TypeSystemMap::GetTypeSystemForLanguage( std::optional<CreateCallback> create_callback) { std::lock_guard<std::mutex> guard(m_mutex); if (m_clear_in_progress) - return llvm::make_error<llvm::StringError>( - "Unable to get TypeSystem because TypeSystemMap is being cleared", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "Unable to get TypeSystem because TypeSystemMap is being cleared"); collection::iterator pos = m_map.find(language); if (pos != m_map.end()) { @@ -277,11 +276,10 @@ llvm::Expected<lldb::TypeSystemSP> TypeSystemMap::GetTypeSystemForLanguage( assert(!pos->second->weak_from_this().expired()); return pos->second; } - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( "TypeSystem for language " + - llvm::StringRef(Language::GetNameForLanguageType(language)) + - " doesn't exist", - llvm::inconvertibleErrorCode()); + llvm::StringRef(Language::GetNameForLanguageType(language)) + + " doesn't exist"); } for (const auto &pair : m_map) { @@ -291,31 +289,27 @@ llvm::Expected<lldb::TypeSystemSP> TypeSystemMap::GetTypeSystemForLanguage( m_map[language] = pair.second; if (pair.second) return pair.second; - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( "TypeSystem for language " + - llvm::StringRef(Language::GetNameForLanguageType(language)) + - " doesn't exist", - llvm::inconvertibleErrorCode()); + llvm::StringRef(Language::GetNameForLanguageType(language)) + + " doesn't exist"); } } if (!create_callback) - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( "Unable to find type system for language " + - llvm::StringRef(Language::GetNameForLanguageType(language)), - llvm::inconvertibleErrorCode()); - + llvm::StringRef(Language::GetNameForLanguageType(language))); // Cache even if we get a shared pointer that contains a null type system // back. TypeSystemSP type_system_sp = (*create_callback)(); m_map[language] = type_system_sp; if (type_system_sp) return type_system_sp; - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( "TypeSystem for language " + - llvm::StringRef(Language::GetNameForLanguageType(language)) + - " doesn't exist", - llvm::inconvertibleErrorCode()); + llvm::StringRef(Language::GetNameForLanguageType(language)) + + " doesn't exist"); } llvm::Expected<lldb::TypeSystemSP> diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 77731167995e1..ec0da8a1378a8 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -2414,8 +2414,7 @@ llvm::Expected<lldb::TypeSystemSP> Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language, bool create_on_demand) { if (!m_valid) - return llvm::make_error<llvm::StringError>("Invalid Target", - llvm::inconvertibleErrorCode()); + return llvm::createStringError("Invalid Target"); if (language == eLanguageTypeMipsAssembler // GNU AS and LLVM use it for all // assembly code @@ -2428,9 +2427,8 @@ Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language, // target language. } else { if (languages_for_expressions.Empty()) - return llvm::make_error<llvm::StringError>( - "No expression support for any languages", - llvm::inconvertibleErrorCode()); + return llvm::createStringError( + "No expression support for any languages"); language = (LanguageType)languages_for_expressions.bitvector.find_first(); } } @@ -2574,23 +2572,20 @@ Target::CreateUtilityFunction(std::string expression, std::string name, return type_system_or_err.takeError(); auto ts = *type_system_or_err; if (!ts) - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( llvm::StringRef("Type system for language ") + - Language::GetNameForLanguageType(language) + - llvm::StringRef(" is no longer live"), - llvm::inconvertibleErrorCode()); + Language::GetNameForLanguageType(language) + + llvm::StringRef(" is no longer live")); std::unique_ptr<UtilityFunction> utility_fn = ts->CreateUtilityFunction(std::move(expression), std::move(name)); if (!utility_fn) - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( llvm::StringRef("Could not create an expression for language") + - Language::GetNameForLanguageType(language), - llvm::inconvertibleErrorCode()); + Language::GetNameForLanguageType(language)); DiagnosticManager diagnostics; if (!utility_fn->Install(diagnostics, exe_ctx)) - return llvm::make_error<llvm::StringError>(diagnostics.GetString(), - llvm::inconvertibleErrorCode()); + return llvm::createStringError(diagnostics.GetString()); return std::move(utility_fn); } @@ -2621,8 +2616,7 @@ void Target::SetDefaultArchitecture(const ArchSpec &arch) { llvm::Error Target::SetLabel(llvm::StringRef label) { size_t n = LLDB_INVALID_INDEX32; if (llvm::to_integer(label, n)) - return llvm::make_error<llvm::StringError>( - "Cannot use integer as target label.", llvm::inconvertibleErrorCode()); + return llvm::createStringError("Cannot use integer as target label."); TargetList &targets = GetDebugger().GetTargetList(); for (size_t i = 0; i < targets.GetNumTargets(); i++) { TargetSP target_sp = targets.GetTargetAtIndex(i); @@ -2790,15 +2784,13 @@ llvm::Expected<lldb_private::Address> Target::GetEntryPointAddress() { // We haven't found the entry point address. Return an appropriate error. if (!has_primary_executable) - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( "No primary executable found and could not find entry point address in " - "any executable module", - llvm::inconvertibleErrorCode()); + "any executable module"); - return llvm::make_error<llvm::StringError>( + return llvm::createStringError( "Could not find entry point address for primary executable module \"" + - exe_module->GetFileSpec().GetFilename().GetStringRef() + "\"", - llvm::inconvertibleErrorCode()); + exe_module->GetFileSpec().GetFilename().GetStringRef() + "\""); } lldb::addr_t Target::GetCallableLoadAddress(lldb::addr_t load_addr, diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp index 3bd00bb20da25..18312e87f03ec 100644 --- a/lldb/source/Utility/Status.cpp +++ b/lldb/source/Utility/Status.cpp @@ -92,8 +92,7 @@ llvm::Error Status::ToError() const { if (m_type == ErrorType::eErrorTypePOSIX) return llvm::errorCodeToError( std::error_code(m_code, std::generic_category())); - return llvm::make_error<llvm::StringError>(AsCString(), - llvm::inconvertibleErrorCode()); + return llvm::createStringError(AsCString()); } Status::~Status() = default; diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h index 217130ce293af..d085ffe6d3267 100644 --- a/llvm/include/llvm/Support/Error.h +++ b/llvm/include/llvm/Support/Error.h @@ -1269,6 +1269,11 @@ inline Error createStringError(std::error_code EC, const Twine &S) { return createStringError(EC, S.str().c_str()); } +/// Create a StringError with an inconvertible error code. +inline Error createStringError(const Twine &S) { + return createStringError(llvm::inconvertibleErrorCode(), S); +} + template <typename... Ts> inline Error createStringError(std::errc EC, char const *Fmt, const Ts &... Vals) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits