From 0642cd768b80665585c8500bed2933a3b99123dc Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 27 Aug 2024 10:59:31 -0700 Subject: [lldb] Turn lldb_private::Status into a value type. (#106163) This patch removes all of the Set.* methods from Status. This cleanup is part of a series of patches that make it harder use the anti-pattern of keeping a long-lives Status object around and updating it while dropping any errors it contains on the floor. This patch is largely NFC, the more interesting next steps this enables is to: 1. remove Status.Clear() 2. assert that Status::operator=() never overwrites an error 3. remove Status::operator=() Note that step (2) will bring 90% of the benefits for users, and step (3) will dramatically clean up the error handling code in various places. In the end my goal is to convert all APIs that are of the form ` ResultTy DoFoo(Status& error) ` to ` llvm::Expected DoFoo() ` How to read this patch? The interesting changes are in Status.h and Status.cpp, all other changes are mostly ` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git grep -l SetErrorString lldb/source) ` plus the occasional manual cleanup. --- .../Python/ScriptInterpreterPython.cpp | 97 ++++++++++++---------- 1 file changed, 51 insertions(+), 46 deletions(-) (limited to 'lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp') diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 2a94f11..76f2640 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1175,7 +1175,7 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction( llvm::Expected maybe_args = GetMaxPositionalArgumentsForCallable(function_name); if (!maybe_args) { - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "could not get num args: %s", llvm::toString(maybe_args.takeError()).c_str()); return error; @@ -1188,16 +1188,16 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction( function_signature += "(frame, bp_loc, extra_args, internal_dict)"; } else if (max_args >= 3) { if (extra_args_sp) { - error.SetErrorString("cannot pass extra_args to a three argument callback" - ); + error = Status::FromErrorStringWithFormat( + "cannot pass extra_args to a three argument callback"); return error; } uses_extra_args = false; function_signature += "(frame, bp_loc, internal_dict)"; } else { - error.SetErrorStringWithFormat("expected 3 or 4 argument " - "function, %s can only take %zu", - function_name, max_args); + error = Status::FromErrorStringWithFormat("expected 3 or 4 argument " + "function, %s can only take %zu", + function_name, max_args); return error; } @@ -1297,12 +1297,12 @@ Status ScriptInterpreterPythonImpl::GenerateFunction(const char *signature, Status error; int num_lines = input.GetSize(); if (num_lines == 0) { - error.SetErrorString("No input data."); + error = Status::FromErrorString("No input data."); return error; } if (!signature || *signature == 0) { - error.SetErrorString("No output function name."); + error = Status::FromErrorString("No output function name."); return error; } @@ -1329,8 +1329,9 @@ Status ScriptInterpreterPythonImpl::GenerateFunction(const char *signature, sstr.Printf(" __return_val = %s", input.GetStringAtIndex(0)); auto_generated_function.AppendString(sstr.GetData()); } else { - return Status("ScriptInterpreterPythonImpl::GenerateFunction(is_callback=" - "true) = ERROR: python function is multiline."); + return Status::FromErrorString( + "ScriptInterpreterPythonImpl::GenerateFunction(is_callback=" + "true) = ERROR: python function is multiline."); } } else { auto_generated_function.AppendString( @@ -1658,12 +1659,12 @@ StructuredData::GenericSP ScriptInterpreterPythonImpl::CreateScriptedStopHook( const StructuredDataImpl &args_data, Status &error) { if (!target_sp) { - error.SetErrorString("No target for scripted stop-hook."); + error = Status::FromErrorString("No target for scripted stop-hook."); return StructuredData::GenericSP(); } if (class_name == nullptr || class_name[0] == '\0') { - error.SetErrorString("No class name for scripted stop-hook."); + error = Status::FromErrorString("No class name for scripted stop-hook."); return StructuredData::GenericSP(); } @@ -1671,7 +1672,8 @@ StructuredData::GenericSP ScriptInterpreterPythonImpl::CreateScriptedStopHook( GetPythonInterpreter(m_debugger); if (!python_interpreter) { - error.SetErrorString("No script interpreter for scripted stop-hook."); + error = Status::FromErrorString( + "No script interpreter for scripted stop-hook."); return StructuredData::GenericSP(); } @@ -1707,7 +1709,7 @@ StructuredData::ObjectSP ScriptInterpreterPythonImpl::LoadPluginModule(const FileSpec &file_spec, lldb_private::Status &error) { if (!FileSystem::Instance().Exists(file_spec)) { - error.SetErrorString("no such file"); + error = Status::FromErrorString("no such file"); return StructuredData::ObjectSP(); } @@ -1825,7 +1827,7 @@ Status ScriptInterpreterPythonImpl::GenerateBreakpointCommandCallbackData( StreamString sstr; Status error; if (user_input.GetSize() == 0) { - error.SetErrorString("No input data."); + error = Status::FromErrorString("No input data."); return error; } @@ -2239,11 +2241,11 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( Status &error) { bool ret_val; if (!process) { - error.SetErrorString("no process"); + error = Status::FromErrorString("no process"); return false; } if (!impl_function || !impl_function[0]) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2254,7 +2256,7 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( impl_function, m_dictionary_name.c_str(), process->shared_from_this(), output); if (!ret_val) - error.SetErrorString("python script evaluation failed"); + error = Status::FromErrorString("python script evaluation failed"); } return ret_val; } @@ -2263,11 +2265,11 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( const char *impl_function, Thread *thread, std::string &output, Status &error) { if (!thread) { - error.SetErrorString("no thread"); + error = Status::FromErrorString("no thread"); return false; } if (!impl_function || !impl_function[0]) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2280,7 +2282,7 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( output = std::move(*result); return true; } - error.SetErrorString("python script evaluation failed"); + error = Status::FromErrorString("python script evaluation failed"); return false; } @@ -2289,11 +2291,11 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( Status &error) { bool ret_val; if (!target) { - error.SetErrorString("no thread"); + error = Status::FromErrorString("no thread"); return false; } if (!impl_function || !impl_function[0]) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2304,7 +2306,7 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( ret_val = SWIGBridge::LLDBSWIGPythonRunScriptKeywordTarget( impl_function, m_dictionary_name.c_str(), target_sp, output); if (!ret_val) - error.SetErrorString("python script evaluation failed"); + error = Status::FromErrorString("python script evaluation failed"); } return ret_val; } @@ -2313,11 +2315,11 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( const char *impl_function, StackFrame *frame, std::string &output, Status &error) { if (!frame) { - error.SetErrorString("no frame"); + error = Status::FromErrorString("no frame"); return false; } if (!impl_function || !impl_function[0]) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2330,7 +2332,7 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( output = std::move(*result); return true; } - error.SetErrorString("python script evaluation failed"); + error = Status::FromErrorString("python script evaluation failed"); return false; } @@ -2339,11 +2341,11 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( Status &error) { bool ret_val; if (!value) { - error.SetErrorString("no value"); + error = Status::FromErrorString("no value"); return false; } if (!impl_function || !impl_function[0]) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2353,7 +2355,7 @@ bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword( ret_val = SWIGBridge::LLDBSWIGPythonRunScriptKeywordValue( impl_function, m_dictionary_name.c_str(), value->GetSP(), output); if (!ret_val) - error.SetErrorString("python script evaluation failed"); + error = Status::FromErrorString("python script evaluation failed"); } return ret_val; } @@ -2382,7 +2384,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( .SetSetLLDBGlobals(false); if (!pathname || !pathname[0]) { - error.SetErrorString("empty path"); + error = Status::FromErrorString("empty path"); return false; } @@ -2449,14 +2451,16 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( // if not a valid file of any sort, check if it might be a filename still // dot can't be used but / and \ can, and if either is found, reject if (strchr(pathname, '\\') || strchr(pathname, '/')) { - error.SetErrorStringWithFormatv("invalid pathname '{0}'", pathname); + error = Status::FromErrorStringWithFormatv("invalid pathname '{0}'", + pathname); return false; } // Not a filename, probably a package of some sort, let it go through. possible_package = true; } else if (is_directory(st) || is_regular_file(st)) { if (module_file.GetDirectory().IsEmpty()) { - error.SetErrorStringWithFormatv("invalid directory name '{0}'", pathname); + error = Status::FromErrorStringWithFormatv( + "invalid directory name '{0}'", pathname); return false; } if (llvm::Error e = @@ -2466,7 +2470,8 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( } module_name = module_file.GetFilename().GetCString(); } else { - error.SetErrorString("no known way to import this module specification"); + error = Status::FromErrorString( + "no known way to import this module specification"); return false; } } @@ -2481,13 +2486,13 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( } if (!possible_package && module_name.find('.') != llvm::StringRef::npos) { - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "Python does not allow dots in module names: %s", module_name.c_str()); return false; } if (module_name.find('-') != llvm::StringRef::npos) { - error.SetErrorStringWithFormat( + error = Status::FromErrorStringWithFormat( "Python discourages dashes in module names: %s", module_name.c_str()); return false; } @@ -2530,7 +2535,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( if (!SWIGBridge::LLDBSwigPythonCallModuleInit( module_name.c_str(), m_dictionary_name.c_str(), m_debugger.shared_from_this())) { - error.SetErrorString("calling __lldb_init_module failed"); + error = Status::FromErrorString("calling __lldb_init_module failed"); return false; } @@ -2598,7 +2603,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedCommand( lldb_private::CommandReturnObject &cmd_retobj, Status &error, const lldb_private::ExecutionContext &exe_ctx) { if (!impl_function) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2606,7 +2611,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedCommand( lldb::ExecutionContextRefSP exe_ctx_ref_sp(new ExecutionContextRef(exe_ctx)); if (!debugger_sp.get()) { - error.SetErrorString("invalid Debugger pointer"); + error = Status::FromErrorString("invalid Debugger pointer"); return false; } @@ -2629,7 +2634,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedCommand( } if (!ret_val) - error.SetErrorString("unable to execute script function"); + error = Status::FromErrorString("unable to execute script function"); else if (cmd_retobj.GetStatus() == eReturnStatusFailed) return false; @@ -2643,7 +2648,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedCommand( lldb_private::CommandReturnObject &cmd_retobj, Status &error, const lldb_private::ExecutionContext &exe_ctx) { if (!impl_obj_sp || !impl_obj_sp->IsValid()) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2651,7 +2656,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedCommand( lldb::ExecutionContextRefSP exe_ctx_ref_sp(new ExecutionContextRef(exe_ctx)); if (!debugger_sp.get()) { - error.SetErrorString("invalid Debugger pointer"); + error = Status::FromErrorString("invalid Debugger pointer"); return false; } @@ -2674,7 +2679,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedCommand( } if (!ret_val) - error.SetErrorString("unable to execute script function"); + error = Status::FromErrorString("unable to execute script function"); else if (cmd_retobj.GetStatus() == eReturnStatusFailed) return false; @@ -2688,7 +2693,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedParsedCommand( lldb_private::CommandReturnObject &cmd_retobj, Status &error, const lldb_private::ExecutionContext &exe_ctx) { if (!impl_obj_sp || !impl_obj_sp->IsValid()) { - error.SetErrorString("no function to execute"); + error = Status::FromErrorString("no function to execute"); return false; } @@ -2696,7 +2701,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedParsedCommand( lldb::ExecutionContextRefSP exe_ctx_ref_sp(new ExecutionContextRef(exe_ctx)); if (!debugger_sp.get()) { - error.SetErrorString("invalid Debugger pointer"); + error = Status::FromErrorString("invalid Debugger pointer"); return false; } @@ -2725,7 +2730,7 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedParsedCommand( } if (!ret_val) - error.SetErrorString("unable to execute script function"); + error = Status::FromErrorString("unable to execute script function"); else if (cmd_retobj.GetStatus() == eReturnStatusFailed) return false; -- cgit v1.1