From 28210d3d7a93014b46f3ec3d45ca575a5ac1e939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mika=C3=ABl=20Capelle?= Date: Thu, 5 May 2022 22:03:18 +0200 Subject: [PATCH] Fix unloading of modules. Up warning levels to 4. --- src/mobase/CMakeLists.txt | 2 +- src/mobase/deprecation.cpp | 7 ++-- src/mobase/wrappers/basic_classes.cpp | 1 - src/mobase/wrappers/game_features.cpp | 6 ++-- src/mobase/wrappers/pyfiletree.cpp | 5 ++- src/mobase/wrappers/pyplugins.h | 5 +++ src/proxy/CMakeLists.txt | 2 +- src/proxy/proxypython.cpp | 4 +-- src/pybind11-qt/CMakeLists.txt | 2 +- .../include/pybind11_qt/pybind11_qt_qflags.h | 2 +- src/pybind11-qt/pybind11_qt_basic.cpp | 26 +++++++------- src/pybind11-qt/pybind11_qt_sip.cpp | 34 ++++++++++--------- .../include/pybind11_utils/arg_wrapper.h | 7 ++-- src/runner/CMakeLists.txt | 2 +- src/runner/pythonrunner.cpp | 2 +- tests/python/CMakeLists.txt | 2 +- 16 files changed, 56 insertions(+), 53 deletions(-) diff --git a/src/mobase/CMakeLists.txt b/src/mobase/CMakeLists.txt index 461ab3f..ee8e0bc 100644 --- a/src/mobase/CMakeLists.txt +++ b/src/mobase/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.16) pybind11_add_module(mobase MODULE) mo2_configure_library(mobase SOURCE_TREE - WARNINGS OFF + WARNINGS 4 AUTOMOC ON TRANSLATIONS OFF PRIVATE_DEPENDS uibase Qt::Core diff --git a/src/mobase/deprecation.cpp b/src/mobase/deprecation.cpp index 860d071..2b86eee 100644 --- a/src/mobase/deprecation.cpp +++ b/src/mobase/deprecation.cpp @@ -25,9 +25,10 @@ namespace mo2::python { auto inspect = py::module_::import("inspect"); auto current_frame = inspect.attr("currentframe")(); py::sequence callable_frame = inspect.attr("getouterframes")(current_frame, 2); - auto filename = callable_frame[-1].attr("filename").cast(); - auto function = callable_frame[-1].attr("function").cast(); - auto lineno = callable_frame[-1].attr("lineno").cast(); + auto last_frame = callable_frame[py::int_(-1)]; + auto filename = last_frame.attr("filename").cast(); + auto function = last_frame.attr("function").cast(); + auto lineno = last_frame.attr("lineno").cast(); // Only show once if requested: if (show_once && DeprecatedLines.contains({filename, lineno})) { diff --git a/src/mobase/wrappers/basic_classes.cpp b/src/mobase/wrappers/basic_classes.cpp index 55b2d9b..eec8c6b 100644 --- a/src/mobase/wrappers/basic_classes.cpp +++ b/src/mobase/wrappers/basic_classes.cpp @@ -30,7 +30,6 @@ namespace mo2::python { namespace py = pybind11; using namespace pybind11::literals; - using namespace mo2::python; void add_versioninfo_classes(py::module_ m) { diff --git a/src/mobase/wrappers/game_features.cpp b/src/mobase/wrappers/game_features.cpp index 432baa2..18f7ca6 100644 --- a/src/mobase/wrappers/game_features.cpp +++ b/src/mobase/wrappers/game_features.cpp @@ -344,7 +344,7 @@ namespace mo2::python { pybind11::object extract_feature(IPluginGame const& game, pybind11::object type) { py::object py_feature = py::none(); - GameFeaturesHelper::apply([&](Feature* feature) { + GameFeaturesHelper::apply([&](Feature*) { if (py::type::of().is(type)) { py_feature = py::cast(game.feature(), py::return_value_policy::reference); @@ -357,7 +357,7 @@ namespace mo2::python { { // constructing a dict from class name to actual object py::dict dict; - GameFeaturesHelper::apply([&](Feature* feature) { + GameFeaturesHelper::apply([&](Feature*) { dict[py::type::of()] = py::cast(game.feature(), py::return_value_policy::reference); }); @@ -368,7 +368,7 @@ namespace mo2::python { convert_feature_list(py::dict const& py_features) { std::map features; - GameFeaturesHelper::apply([&](Feature* feature) { + GameFeaturesHelper::apply([&](Feature*) { const auto py_type = py::type::of(); if (py_features.contains(py_type)) { features[std::type_index(typeid(Feature))] = diff --git a/src/mobase/wrappers/pyfiletree.cpp b/src/mobase/wrappers/pyfiletree.cpp index acff7b1..4fb1560 100644 --- a/src/mobase/wrappers/pyfiletree.cpp +++ b/src/mobase/wrappers/pyfiletree.cpp @@ -49,9 +49,8 @@ namespace mo2::detail { return std::make_shared(parent, name, m_Callback); } - bool - doPopulate(std::shared_ptr parent, - std::vector>& entries) const override + bool doPopulate(std::shared_ptr parent, + std::vector>&) const override { return true; } diff --git a/src/mobase/wrappers/pyplugins.h b/src/mobase/wrappers/pyplugins.h index f6bd94c..00e25d3 100644 --- a/src/mobase/wrappers/pyplugins.h +++ b/src/mobase/wrappers/pyplugins.h @@ -28,6 +28,11 @@ namespace mo2::python { public: using PluginBase::PluginBase; + PyPluginBaseNoFinal(PyPluginBaseNoFinal const&) = delete; + PyPluginBaseNoFinal(PyPluginBaseNoFinal&&) = delete; + PyPluginBaseNoFinal& operator=(PyPluginBaseNoFinal const&) = delete; + PyPluginBaseNoFinal& operator=(PyPluginBaseNoFinal&&) = delete; + bool init(IOrganizer* organizer) override { PYBIND11_OVERRIDE_PURE(bool, PluginBase, init, organizer); diff --git a/src/proxy/CMakeLists.txt b/src/proxy/CMakeLists.txt index 1dc9f1f..81340eb 100644 --- a/src/proxy/CMakeLists.txt +++ b/src/proxy/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.16) add_library(plugin_python SHARED) mo2_configure_plugin(plugin_python - WARNINGS OFF + WARNINGS 4 EXTRA_TRANSLATIONS ${CMAKE_CURRENT_SOURCE_DIR}/../runner ${CMAKE_CURRENT_SOURCE_DIR}/../mobase diff --git a/src/proxy/proxypython.cpp b/src/proxy/proxypython.cpp index 87f2455..1485799 100644 --- a/src/proxy/proxypython.cpp +++ b/src/proxy/proxypython.cpp @@ -294,9 +294,9 @@ QString ProxyPython::fullDescription(unsigned int key) const } } -bool ProxyPython::hasGuidedFix(unsigned int key) const +bool ProxyPython::hasGuidedFix(unsigned int) const { return false; } -void ProxyPython::startGuidedFix(unsigned int key) const {} +void ProxyPython::startGuidedFix(unsigned int) const {} diff --git a/src/pybind11-qt/CMakeLists.txt b/src/pybind11-qt/CMakeLists.txt index 6b39049..3278cf5 100644 --- a/src/pybind11-qt/CMakeLists.txt +++ b/src/pybind11-qt/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.16) add_library(pybind11-qt STATIC) mo2_configure_library(pybind11-qt SOURCE_TREE - WARNINGS OFF + WARNINGS 4 AUTOMOC OFF TRANSLATIONS OFF PRIVATE_DEPENDS Qt::Core Qt::Widgets diff --git a/src/pybind11-qt/include/pybind11_qt/pybind11_qt_qflags.h b/src/pybind11-qt/include/pybind11_qt/pybind11_qt_qflags.h index c9f4eca..f246ed3 100644 --- a/src/pybind11-qt/include/pybind11_qt/pybind11_qt_qflags.h +++ b/src/pybind11-qt/include/pybind11_qt/pybind11_qt_qflags.h @@ -19,7 +19,7 @@ namespace pybind11::detail { * instance or return false upon failure. The second argument * indicates whether implicit conversions should be applied. */ - bool load(handle src, bool implicit) + bool load(handle src, bool) { PyObject* tmp = PyNumber_Long(src.ptr()); diff --git a/src/pybind11-qt/pybind11_qt_basic.cpp b/src/pybind11-qt/pybind11_qt_basic.cpp index c6a7cc6..56ddf4f 100644 --- a/src/pybind11-qt/pybind11_qt_basic.cpp +++ b/src/pybind11-qt/pybind11_qt_basic.cpp @@ -33,7 +33,7 @@ namespace pybind11::detail { * instance or return false upon failure. The second argument * indicates whether implicit conversions should be applied. */ - bool type_caster::load(handle src, bool implicit) + bool type_caster::load(handle src, bool) { PyObject* objPtr = src.ptr(); @@ -72,7 +72,7 @@ namespace pybind11::detail { src.length()); } - bool type_caster::load(handle src, bool implicit) + bool type_caster::load(handle src, bool) { // test for string first otherwise PyList_Check also works if (PyBytes_Check(src.ptr()) || PyUnicode_Check(src.ptr())) { @@ -90,7 +90,7 @@ namespace pybind11::detail { value = src.cast(); return true; } - else if (src == Py_None) { + else if (src.is(pybind11::none())) { value = QVariant(); return true; } @@ -118,27 +118,27 @@ namespace pybind11::detail { handle type_caster::cast(QVariant var, return_value_policy policy, handle parent) { - switch (var.type()) { - case QVariant::Invalid: + switch (var.typeId()) { + case QMetaType::UnknownType: return Py_None; - case QVariant::Int: + case QMetaType::Int: return PyLong_FromLong(var.toInt()); - case QVariant::UInt: + case QMetaType::UInt: return PyLong_FromUnsignedLong(var.toUInt()); - case QVariant::Bool: + case QMetaType::Bool: return PyBool_FromLong(var.toBool()); - case QVariant::String: + case QMetaType::QString: return type_caster::cast(var.toString(), policy, parent); // We need to check for StringList here because these are not considered // List since List is QList will StringList is QList: - case QVariant::StringList: + case QMetaType::QStringList: return type_caster::cast(var.toStringList(), policy, parent); - case QVariant::List: + case QMetaType::QVariantList: return type_caster::cast(var.toList(), policy, parent); - case QVariant::Map: + case QMetaType::QVariantMap: return type_caster::cast(var.toMap(), policy, parent); default: { - PyErr_Format(PyExc_TypeError, "type unsupported: %d", var.type()); + PyErr_Format(PyExc_TypeError, "type unsupported: %d", var.userType()); throw pybind11::error_already_set(); } } diff --git a/src/pybind11-qt/pybind11_qt_sip.cpp b/src/pybind11-qt/pybind11_qt_sip.cpp index 4ef8fb3..e3e0a1f 100644 --- a/src/pybind11-qt/pybind11_qt_sip.cpp +++ b/src/pybind11-qt/pybind11_qt_sip.cpp @@ -15,23 +15,25 @@ namespace pybind11::detail::qt { if (sipApi == nullptr) { PyImport_ImportModule("PyQt6.sip"); - auto errorObj = PyErr_Occurred(); - if (errorObj != NULL) { - PyObject *type, *value, *traceback; - PyErr_Fetch(&type, &value, &traceback); - PyErr_NormalizeException(&type, &value, &traceback); - if (traceback != NULL) { - py::handle h_type(type); - py::handle h_val(value); - py::handle h_tb(traceback); - py::object tb(py::module_::import("traceback")); - py::object fmt_exp(tb.attr("format_exception")); - py::object exp_list(fmt_exp(h_type, h_val, h_tb)); - py::object exp_str(py::str("\n").attr("join")(exp_list)); - exception = exp_str.cast(); + { + auto errorObj = PyErr_Occurred(); + if (errorObj != NULL) { + PyObject *type, *value, *traceback; + PyErr_Fetch(&type, &value, &traceback); + PyErr_NormalizeException(&type, &value, &traceback); + if (traceback != NULL) { + py::handle h_type(type); + py::handle h_val(value); + py::handle h_tb(traceback); + py::object tb(py::module_::import("traceback")); + py::object fmt_exp(tb.attr("format_exception")); + py::object exp_list(fmt_exp(h_type, h_val, h_tb)); + py::object exp_str(py::str("\n").attr("join")(exp_list)); + exception = exp_str.cast(); + } + PyErr_Restore(type, value, traceback); + throw std::runtime_error{"Failed to load SIP API: " + exception}; } - PyErr_Restore(type, value, traceback); - throw std::runtime_error{"Failed to load SIP API: " + exception}; } sipApi = (const sipAPIDef*)PyCapsule_Import("PyQt6.sip._C_API", 0); diff --git a/src/pybind11-utils/include/pybind11_utils/arg_wrapper.h b/src/pybind11-utils/include/pybind11_utils/arg_wrapper.h index 3ab49d0..f0c688d 100644 --- a/src/pybind11-utils/include/pybind11_utils/arg_wrapper.h +++ b/src/pybind11-utils/include/pybind11_utils/arg_wrapper.h @@ -28,7 +28,7 @@ namespace mo2::python { template - auto wrap_fn_impl(std::index_sequence, Fn&& fn, R (*sg)(Args...), + auto wrap_fn_impl(std::index_sequence, Fn&& fn, R (*)(Args...), std::index_sequence) { return [fn = std::forward(fn)]( @@ -63,10 +63,7 @@ namespace mo2::python { template struct load_wrapped_argument_helper { - static bool load(Type& value, pybind11::handle src, bool convert) - { - return false; - } + static bool load(Type&, pybind11::handle, bool) { return false; } }; template diff --git a/src/runner/CMakeLists.txt b/src/runner/CMakeLists.txt index 0080232..d9bff03 100644 --- a/src/runner/CMakeLists.txt +++ b/src/runner/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.16) add_library(pythonrunner SHARED) mo2_configure_library(pythonrunner SOURCE_TREE - WARNINGS OFF + WARNINGS 4 AUTOMOC ON TRANSLATIONS OFF PUBLIC_DEPENDS uibase Qt::Core diff --git a/src/runner/pythonrunner.cpp b/src/runner/pythonrunner.cpp index 3d7a2ff..a2e132a 100644 --- a/src/runner/pythonrunner.cpp +++ b/src/runner/pythonrunner.cpp @@ -268,7 +268,7 @@ void PythonRunner::unload(const QString& identifier) for (std::size_t i = 0; i < py::len(keys); ++i) { py::object mod = modules[keys[i]]; if (PyObject_HasAttrString(mod.ptr(), "__path__")) { - QString mpath = mod.attr("__path__")[0].cast(); + QString mpath = mod.attr("__path__")[py::int_(0)].cast(); if (!folder.relativeFilePath(mpath).startsWith("..")) { // If the path is under identifier, we need to unload diff --git a/tests/python/CMakeLists.txt b/tests/python/CMakeLists.txt index 1bf9f74..0703ca7 100644 --- a/tests/python/CMakeLists.txt +++ b/tests/python/CMakeLists.txt @@ -38,7 +38,7 @@ foreach (test_file ${test_files}) get_filename_component(target ${test_file} NAME_WLE) string(REPLACE "test_" "" pymodule ${target}) - pybind11_add_module(${target} THIN_LTO ${test_file}) + pybind11_add_module(${target} EXCLUDE_FROM_ALL THIN_LTO ${test_file}) set_target_properties(${target} PROPERTIES OUTPUT_NAME ${pymodule}