llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-support Author: Adrian Prantl (adrian-prantl) <details> <summary>Changes</summary> 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. --- Patch is 21.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/93209.diff 13 Files Affected: - (modified) lldb/source/Host/common/Socket.cpp (+2-2) - (modified) lldb/source/Interpreter/Options.cpp (+11-18) - (modified) lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp (+3-5) - (modified) lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (+3-6) - (modified) lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h (+2-3) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+5-10) - (modified) lldb/source/Symbol/CompilerType.cpp (+1-2) - (modified) lldb/source/Symbol/Symbol.cpp (+7-11) - (modified) lldb/source/Symbol/SymbolFileOnDemand.cpp (+2-3) - (modified) lldb/source/Symbol/TypeSystem.cpp (+13-19) - (modified) lldb/source/Target/Target.cpp (+14-22) - (modified) lldb/source/Utility/Status.cpp (+1-2) - (modified) llvm/include/llvm/Support/Error.h (+4) ``````````diff diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp index bd0c127a08956..e060c174326b8 100644 --- a/lldb/source/Host/common/Socket.cpp +++ b/lldb/source/Host/common/Socket.cpp @@ -87,8 +87,8 @@ 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.", + llvm::inconvertibleErrorCode()); } } 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 exe... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/93209 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
