mib updated this revision to Diff 476361. mib marked an inline comment as done. mib added a comment.
Implement @labath suggestions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138248/new/ https://reviews.llvm.org/D138248 Files: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1501,50 +1501,29 @@ StructuredData::ObjectSP os_plugin_object_sp) { Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); - static char callee_name[] = "get_register_info"; - if (!os_plugin_object_sp) - return StructuredData::DictionarySP(); + return {}; StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); if (!generic) - return nullptr; + return {}; PythonObject implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); if (!implementor.IsAllocated()) - return StructuredData::DictionarySP(); - - PythonObject pmeth(PyRefType::Owned, - PyObject_GetAttrString(implementor.get(), callee_name)); - - if (PyErr_Occurred()) - PyErr_Clear(); - - if (!pmeth.IsAllocated()) - return StructuredData::DictionarySP(); + return {}; - if (PyCallable_Check(pmeth.get()) == 0) { - if (PyErr_Occurred()) - PyErr_Clear(); + llvm::Expected<PythonObject> expected_py_return = + implementor.CallMethod("get_register_info"); - return StructuredData::DictionarySP(); + if (!expected_py_return) { + llvm::consumeError(expected_py_return.takeError()); + return {}; } - if (PyErr_Occurred()) - PyErr_Clear(); - - // right now we know this function exists and is callable.. - PythonObject py_return( - PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, nullptr)); + PythonObject py_return = std::move(expected_py_return.get()); - // if it fails, print the error but otherwise go on - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } if (py_return.get()) { PythonDictionary result_dict(PyRefType::Borrowed, py_return.get()); return result_dict.CreateStructuredDictionary(); @@ -1555,51 +1534,28 @@ StructuredData::ArraySP ScriptInterpreterPythonImpl::OSPlugin_ThreadsInfo( StructuredData::ObjectSP os_plugin_object_sp) { Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); - - static char callee_name[] = "get_thread_info"; - if (!os_plugin_object_sp) - return StructuredData::ArraySP(); + return {}; StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); if (!generic) - return nullptr; + return {}; PythonObject implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); if (!implementor.IsAllocated()) - return StructuredData::ArraySP(); - - PythonObject pmeth(PyRefType::Owned, - PyObject_GetAttrString(implementor.get(), callee_name)); - - if (PyErr_Occurred()) - PyErr_Clear(); - - if (!pmeth.IsAllocated()) - return StructuredData::ArraySP(); + return {}; - if (PyCallable_Check(pmeth.get()) == 0) { - if (PyErr_Occurred()) - PyErr_Clear(); + llvm::Expected<PythonObject> expected_py_return = + implementor.CallMethod("get_thread_info"); - return StructuredData::ArraySP(); + if (!expected_py_return) { + llvm::consumeError(expected_py_return.takeError()); + return {}; } - if (PyErr_Occurred()) - PyErr_Clear(); - - // right now we know this function exists and is callable.. - PythonObject py_return( - PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, nullptr)); - - // if it fails, print the error but otherwise go on - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } + PythonObject py_return = std::move(expected_py_return.get()); if (py_return.get()) { PythonList result_list(PyRefType::Borrowed, py_return.get()); @@ -1613,56 +1569,33 @@ StructuredData::ObjectSP os_plugin_object_sp, lldb::tid_t tid) { Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); - static char callee_name[] = "get_register_data"; - static char *param_format = - const_cast<char *>(GetPythonValueFormatString(tid)); - if (!os_plugin_object_sp) - return StructuredData::StringSP(); + return {}; StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); if (!generic) - return nullptr; + return {}; PythonObject implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); if (!implementor.IsAllocated()) - return StructuredData::StringSP(); - - PythonObject pmeth(PyRefType::Owned, - PyObject_GetAttrString(implementor.get(), callee_name)); - - if (PyErr_Occurred()) - PyErr_Clear(); + return {}; - if (!pmeth.IsAllocated()) - return StructuredData::StringSP(); + llvm::Expected<PythonObject> expected_py_return = + implementor.CallMethod("get_register_data", tid); - if (PyCallable_Check(pmeth.get()) == 0) { - if (PyErr_Occurred()) - PyErr_Clear(); - return StructuredData::StringSP(); + if (!expected_py_return) { + llvm::consumeError(expected_py_return.takeError()); + return {}; } - if (PyErr_Occurred()) - PyErr_Clear(); - - // right now we know this function exists and is callable.. - PythonObject py_return( - PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, param_format, tid)); - - // if it fails, print the error but otherwise go on - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } + PythonObject py_return = std::move(expected_py_return.get()); if (py_return.get()) { PythonBytes result(PyRefType::Borrowed, py_return.get()); return result.CreateStructuredString(); } - return StructuredData::StringSP(); + return {}; } StructuredData::DictionarySP ScriptInterpreterPythonImpl::OSPlugin_CreateThread( @@ -1670,52 +1603,28 @@ lldb::addr_t context) { Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); - static char callee_name[] = "create_thread"; - std::string param_format; - param_format += GetPythonValueFormatString(tid); - param_format += GetPythonValueFormatString(context); - if (!os_plugin_object_sp) - return StructuredData::DictionarySP(); + return {}; StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); if (!generic) - return nullptr; + return {}; PythonObject implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); if (!implementor.IsAllocated()) - return StructuredData::DictionarySP(); - - PythonObject pmeth(PyRefType::Owned, - PyObject_GetAttrString(implementor.get(), callee_name)); - - if (PyErr_Occurred()) - PyErr_Clear(); + return {}; - if (!pmeth.IsAllocated()) - return StructuredData::DictionarySP(); + llvm::Expected<PythonObject> expected_py_return = + implementor.CallMethod("create_thread", tid, context); - if (PyCallable_Check(pmeth.get()) == 0) { - if (PyErr_Occurred()) - PyErr_Clear(); - return StructuredData::DictionarySP(); + if (!expected_py_return) { + llvm::consumeError(expected_py_return.takeError()); + return {}; } - if (PyErr_Occurred()) - PyErr_Clear(); - - // right now we know this function exists and is callable.. - PythonObject py_return(PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, - ¶m_format[0], tid, context)); - - // if it fails, print the error but otherwise go on - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } + PythonObject py_return = std::move(expected_py_return.get()); if (py_return.get()) { PythonDictionary result_dict(PyRefType::Borrowed, py_return.get()); @@ -2436,51 +2345,31 @@ Locker py_lock(this, Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN); - static char callee_name[] = "get_type_name"; - - ConstString ret_val; - bool got_string = false; - std::string buffer; - if (!implementor_sp) - return ret_val; + return {}; StructuredData::Generic *generic = implementor_sp->GetAsGeneric(); if (!generic) - return ret_val; + return {}; + PythonObject implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); if (!implementor.IsAllocated()) - return ret_val; + return {}; - PythonObject pmeth(PyRefType::Owned, - PyObject_GetAttrString(implementor.get(), callee_name)); + llvm::Expected<PythonObject> expected_py_return = + implementor.CallMethod("get_type_name"); - if (PyErr_Occurred()) - PyErr_Clear(); - - if (!pmeth.IsAllocated()) - return ret_val; - - if (PyCallable_Check(pmeth.get()) == 0) { - if (PyErr_Occurred()) - PyErr_Clear(); - return ret_val; + if (!expected_py_return) { + llvm::consumeError(expected_py_return.takeError()); + return {}; } - if (PyErr_Occurred()) - PyErr_Clear(); - - // right now we know this function exists and is callable.. - PythonObject py_return( - PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, nullptr)); + PythonObject py_return = std::move(expected_py_return.get()); - // if it fails, print the error but otherwise go on - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } + ConstString ret_val; + bool got_string = false; + std::string buffer; if (py_return.IsAllocated() && PythonString::Check(py_return.get())) { PythonString py_string(PyRefType::Borrowed, py_return.get()); @@ -2986,8 +2875,6 @@ Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); - static char callee_name[] = "get_short_help"; - if (!cmd_obj_sp) return false; @@ -2997,34 +2884,15 @@ if (!implementor.IsAllocated()) return false; - PythonObject pmeth(PyRefType::Owned, - PyObject_GetAttrString(implementor.get(), callee_name)); + llvm::Expected<PythonObject> expected_py_return = + implementor.CallMethod("get_short_help"); - if (PyErr_Occurred()) - PyErr_Clear(); - - if (!pmeth.IsAllocated()) - return false; - - if (PyCallable_Check(pmeth.get()) == 0) { - if (PyErr_Occurred()) - PyErr_Clear(); + if (!expected_py_return) { + llvm::consumeError(expected_py_return.takeError()); return false; } - if (PyErr_Occurred()) - PyErr_Clear(); - - // Right now we know this function exists and is callable. - PythonObject py_return( - PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, nullptr)); - - // If it fails, print the error but otherwise go on. - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } + PythonObject py_return = std::move(expected_py_return.get()); if (py_return.IsAllocated() && PythonString::Check(py_return.get())) { PythonString py_string(PyRefType::Borrowed, py_return.get()); @@ -3087,13 +2955,10 @@ bool ScriptInterpreterPythonImpl::GetLongHelpForCommandObject( StructuredData::GenericSP cmd_obj_sp, std::string &dest) { - bool got_string = false; dest.clear(); Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); - static char callee_name[] = "get_long_help"; - if (!cmd_obj_sp) return false; @@ -3103,36 +2968,17 @@ if (!implementor.IsAllocated()) return false; - PythonObject pmeth(PyRefType::Owned, - PyObject_GetAttrString(implementor.get(), callee_name)); - - if (PyErr_Occurred()) - PyErr_Clear(); - - if (!pmeth.IsAllocated()) - return false; - - if (PyCallable_Check(pmeth.get()) == 0) { - if (PyErr_Occurred()) - PyErr_Clear(); + llvm::Expected<PythonObject> expected_py_return = + implementor.CallMethod("get_long_help"); + if (!expected_py_return) { + llvm::consumeError(expected_py_return.takeError()); return false; } - if (PyErr_Occurred()) - PyErr_Clear(); - - // right now we know this function exists and is callable.. - PythonObject py_return( - PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, nullptr)); - - // if it fails, print the error but otherwise go on - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } + PythonObject py_return = std::move(expected_py_return.get()); + bool got_string = false; if (py_return.IsAllocated() && PythonString::Check(py_return.get())) { PythonString str(PyRefType::Borrowed, py_return.get()); llvm::StringRef str_data(str.GetString()); Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -185,22 +185,34 @@ enum class PyInitialValue { Invalid, Empty }; +// DOC: https://docs.python.org/3/c-api/arg.html#building-values template <typename T, typename Enable = void> struct PythonFormat; -template <> struct PythonFormat<unsigned long long> { - static constexpr char format = 'K'; - static auto get(unsigned long long value) { return value; } +template <typename T, char F> struct PassthroughFormat { + static constexpr char format = F; + static constexpr T get(T t) { return t; } }; -template <> struct PythonFormat<long long> { - static constexpr char format = 'L'; - static auto get(long long value) { return value; } -}; - -template <> struct PythonFormat<PyObject *> { - static constexpr char format = 'O'; - static auto get(PyObject *value) { return value; } -}; +template <> struct PythonFormat<char *> : PassthroughFormat<char *, 's'> {}; +template <> struct PythonFormat<char> : PassthroughFormat<char, 'b'> {}; +template <> +struct PythonFormat<unsigned char> : PassthroughFormat<unsigned char, 'B'> {}; +template <> struct PythonFormat<short> : PassthroughFormat<short, 'h'> {}; +template <> +struct PythonFormat<unsigned short> : PassthroughFormat<unsigned short, 'H'> {}; +template <> struct PythonFormat<int> : PassthroughFormat<int, 'i'> {}; +template <> +struct PythonFormat<unsigned int> : PassthroughFormat<unsigned int, 'I'> {}; +template <> struct PythonFormat<long> : PassthroughFormat<long, 'l'> {}; +template <> +struct PythonFormat<unsigned long> : PassthroughFormat<unsigned long, 'k'> {}; +template <> +struct PythonFormat<long long> : PassthroughFormat<long long, 'L'> {}; +template <> +struct PythonFormat<unsigned long long> + : PassthroughFormat<unsigned long long, 'K'> {}; +template <> +struct PythonFormat<PyObject *> : PassthroughFormat<PyObject *, 'O'> {}; template <typename T> struct PythonFormat<
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits