From ac5837009960d0d92a8f48a70f3aab1a7b38a156 Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Fri, 24 Jun 2022 11:07:51 +0200 Subject: Add initial support for std::unique_ptr pointer Known limitations: - No rich comparison is generated - Value conversions caused by passing pointers to derived classes do not work. [ChangeLog][shiboken6] Support for std::unique_ptr pointer has been added. Task-number: PYSIDE-454 Change-Id: I5ddf3156bb383598f91bb97d169d1e134918a161 Reviewed-by: Christian Tismer --- .../shiboken6/ApiExtractor/abstractmetatype.cpp | 5 + sources/shiboken6/ApiExtractor/abstractmetatype.h | 1 + sources/shiboken6/ApiExtractor/typesystem.cpp | 11 +- sources/shiboken6/ApiExtractor/typesystem.h | 1 + sources/shiboken6/ApiExtractor/typesystem_enums.h | 1 + .../shiboken6/ApiExtractor/typesystemparser.cpp | 6 ++ .../shiboken6/doc/typesystem_specifying_types.rst | 6 +- .../shiboken6/generator/shiboken/cppgenerator.cpp | 22 +++- .../generator/shiboken/headergenerator.cpp | 18 ++-- .../generator/shiboken/shibokengenerator.cpp | 15 ++- .../generator/shiboken/shibokengenerator.h | 2 + sources/shiboken6/tests/libsmart/CMakeLists.txt | 1 + sources/shiboken6/tests/libsmart/smart.h | 1 + .../tests/libsmart/stduniqueptrtestbench.cpp | 117 +++++++++++++++++++++ .../tests/libsmart/stduniqueptrtestbench.h | 46 ++++++++ .../shiboken6/tests/smartbinding/CMakeLists.txt | 4 + .../tests/smartbinding/std_unique_ptr_test.py | 84 +++++++++++++++ .../tests/smartbinding/typesystem_smart.xml | 10 ++ 18 files changed, 335 insertions(+), 16 deletions(-) create mode 100644 sources/shiboken6/tests/libsmart/stduniqueptrtestbench.cpp create mode 100644 sources/shiboken6/tests/libsmart/stduniqueptrtestbench.h create mode 100644 sources/shiboken6/tests/smartbinding/std_unique_ptr_test.py diff --git a/sources/shiboken6/ApiExtractor/abstractmetatype.cpp b/sources/shiboken6/ApiExtractor/abstractmetatype.cpp index 6084a9b54..a56557e76 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetatype.cpp +++ b/sources/shiboken6/ApiExtractor/abstractmetatype.cpp @@ -799,6 +799,11 @@ bool AbstractMetaType::isObjectType() const return d->m_typeEntry->isObject(); } +bool AbstractMetaType::isUniquePointer() const +{ + return isSmartPointer() && d->m_typeEntry->isUniquePointer(); +} + bool AbstractMetaType::isPointer() const { return !d->m_indirections.isEmpty() diff --git a/sources/shiboken6/ApiExtractor/abstractmetatype.h b/sources/shiboken6/ApiExtractor/abstractmetatype.h index a531958fa..561cac317 100644 --- a/sources/shiboken6/ApiExtractor/abstractmetatype.h +++ b/sources/shiboken6/ApiExtractor/abstractmetatype.h @@ -102,6 +102,7 @@ public: // returns true if the type was used as a smart pointer bool isSmartPointer() const { return typeUsagePattern() == SmartPointerPattern; } + bool isUniquePointer() const; // returns true if the type was used as a flag bool isFlags() const { return typeUsagePattern() == FlagsPattern; } diff --git a/sources/shiboken6/ApiExtractor/typesystem.cpp b/sources/shiboken6/ApiExtractor/typesystem.cpp index 76f9b0ae6..f0a4a9027 100644 --- a/sources/shiboken6/ApiExtractor/typesystem.cpp +++ b/sources/shiboken6/ApiExtractor/typesystem.cpp @@ -299,6 +299,14 @@ bool TypeEntry::isSmartPointer() const return m_d->m_type == SmartPointerType; } +bool TypeEntry::isUniquePointer() const +{ + if (m_d->m_type != SmartPointerType) + return false; + auto *ste = static_cast(this); + return ste->smartPointerType() == TypeSystem::SmartPointerType::Unique; +} + bool TypeEntry::isArray() const { return m_d->m_type == ArrayType; @@ -2489,7 +2497,8 @@ void SmartPointerTypeEntry::formatDebug(QDebug &debug) const ComplexTypeEntry::formatDebug(debug); if (!d->m_instantiations.isEmpty()) { - debug << ", instantiations[" << d->m_instantiations.size() << "]=("; + debug << "type=" << d->m_type << ", instantiations[" + << d->m_instantiations.size() << "]=("; for (auto i : d->m_instantiations) debug << i->name() << ','; debug << ')'; diff --git a/sources/shiboken6/ApiExtractor/typesystem.h b/sources/shiboken6/ApiExtractor/typesystem.h index 0a3aabb06..186d82675 100644 --- a/sources/shiboken6/ApiExtractor/typesystem.h +++ b/sources/shiboken6/ApiExtractor/typesystem.h @@ -82,6 +82,7 @@ public: bool isNamespace() const; bool isContainer() const; bool isSmartPointer() const; + bool isUniquePointer() const; bool isArray() const; bool isTemplateArgument() const; bool isVoid() const; diff --git a/sources/shiboken6/ApiExtractor/typesystem_enums.h b/sources/shiboken6/ApiExtractor/typesystem_enums.h index f087ff6b5..58c40b3d0 100644 --- a/sources/shiboken6/ApiExtractor/typesystem_enums.h +++ b/sources/shiboken6/ApiExtractor/typesystem_enums.h @@ -92,6 +92,7 @@ enum class QtMetaTypeRegistration enum class SmartPointerType { Shared, + Unique, Handle, ValueHandle }; diff --git a/sources/shiboken6/ApiExtractor/typesystemparser.cpp b/sources/shiboken6/ApiExtractor/typesystemparser.cpp index 8f1cf9c61..d91fa9193 100644 --- a/sources/shiboken6/ApiExtractor/typesystemparser.cpp +++ b/sources/shiboken6/ApiExtractor/typesystemparser.cpp @@ -398,6 +398,7 @@ ENUM_LOOKUP_BEGIN(TypeSystem::SmartPointerType, Qt::CaseSensitive, smartPointerTypeFromAttribute) { {u"handle", TypeSystem::SmartPointerType::Handle}, + {u"unique", TypeSystem::SmartPointerType::Unique}, {u"value-handle", TypeSystem::SmartPointerType::ValueHandle}, {u"shared", TypeSystem::SmartPointerType::Shared} }; @@ -1394,6 +1395,11 @@ SmartPointerTypeEntry * return nullptr; } + if (smartPointerType == TypeSystem::SmartPointerType::Unique && resetMethod.isEmpty()) { + m_error = u"Unique pointers require a reset() method."_s; + return nullptr; + } + auto *type = new SmartPointerTypeEntry(name, getter, smartPointerType, refCountMethodName, since, currentParentTypeEntry()); if (!applyCommonAttributes(reader, type, attributes)) diff --git a/sources/shiboken6/doc/typesystem_specifying_types.rst b/sources/shiboken6/doc/typesystem_specifying_types.rst index 87f890518..4f5817402 100644 --- a/sources/shiboken6/doc/typesystem_specifying_types.rst +++ b/sources/shiboken6/doc/typesystem_specifying_types.rst @@ -629,7 +629,7 @@ smart-pointer-type isUniquePointer(); + + if (isUniquePointer) { + c << "auto *source = reinterpret_cast<" << typeName + << " *>(const_cast(cppIn));\n"; + } else { + c << "auto *source = reinterpret_cast(cppIn);\n"; + } c << "return Shiboken::Object::newObject(" << cpythonType - << ", new ::" << classContext.effectiveClassName() - << "(*reinterpret_cast(cppIn)), true, true);"; + << ", new ::" << classContext.effectiveClassName() << '(' + << (isUniquePointer ? "std::move(*source)" : "*source") + << "), true, true);"; writeCppToPythonFunction(s, c.toString(), sourceTypeName, targetTypeName); s << '\n'; @@ -1700,9 +1710,9 @@ return result;)"; c << "*ptr = {};\n"; else c << "ptr->" << resetMethod << "();\n"; + const QString value = u'*' + cpythonWrapperCPtr(classContext.preciseType(), pyInVariable); c << outdent << "else\n" << indent - << "*ptr = *" - << cpythonWrapperCPtr(classContext.preciseType(), pyInVariable) << ';'; + << "*ptr = " << (isUniquePointer ? stdMove(value) : value) << ';'; } writePythonToCppFunction(s, c.toString(), sourceTypeName, targetTypeName); @@ -3769,6 +3779,10 @@ void CppGenerator::writeMethodCall(TextStream &s, const AbstractMetaFunctionCPtr userArgs.append(argName); } } + // "Pass unique ptr by value" pattern: Apply std::move() + auto type = arg.type(); + if (type.isUniquePointer() && type.passByValue()) + userArgs.last() = stdMove(userArgs.constLast()); } // If any argument's default value was modified the method must be called diff --git a/sources/shiboken6/generator/shiboken/headergenerator.cpp b/sources/shiboken6/generator/shiboken/headergenerator.cpp index 28d565ae3..21d5c8f6e 100644 --- a/sources/shiboken6/generator/shiboken/headergenerator.cpp +++ b/sources/shiboken6/generator/shiboken/headergenerator.cpp @@ -216,15 +216,19 @@ void HeaderGenerator::writeMemberFunctionWrapper(TextStream &s, if (i > 0) s << ", "; const AbstractMetaArgument &arg = arguments.at(i); + const auto &type = arg.type(); const TypeEntry *enumTypeEntry = nullptr; - if (arg.type().isFlags()) - enumTypeEntry = static_cast(arg.type().typeEntry())->originator(); - else if (arg.type().isEnum()) - enumTypeEntry = arg.type().typeEntry(); - if (enumTypeEntry) - s << arg.type().cppSignature() << '(' << arg.name() << ')'; - else + if (type.isFlags()) + enumTypeEntry = static_cast(type.typeEntry())->originator(); + else if (type.isEnum()) + enumTypeEntry = type.typeEntry(); + if (enumTypeEntry) { + s << type.cppSignature() << '(' << arg.name() << ')'; + } else if (type.passByValue() && type.isUniquePointer()) { + s << stdMove(arg.name()); + } else { s << arg.name(); + } } s << "); }\n"; } diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp index e8562a254..264c8cfea 100644 --- a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp +++ b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp @@ -1120,10 +1120,14 @@ void ShibokenGenerator::writeArgumentNames(TextStream &s, const int index = argument.argumentIndex() + 1; if (options.testFlag(Generator::SkipRemovedArguments) && argument.isModifiedRemoved()) continue; + const auto &type = argument.type(); + if (argCount > 0) + s << ", "; + const bool isVirtualCall = options.testFlag(Option::VirtualCall); + const bool useStdMove = isVirtualCall && type.isUniquePointer() && type.passByValue(); + s << (useStdMove ? stdMove(argument.name()) : argument.name()); - s << ((argCount > 0) ? ", " : "") << argument.name(); - - if (((options & Generator::VirtualCall) == 0) + if (!isVirtualCall && (func->hasConversionRule(TypeSystem::NativeCode, index) || func->hasConversionRule(TypeSystem::TargetLangCode, index)) && !func->isConstructor()) { @@ -2283,3 +2287,8 @@ void ShibokenGenerator::replaceTemplateVariables(QString &code, code.replace(u"%ARGUMENTS"_s, aux_stream); } } + +QString ShibokenGenerator::stdMove(const QString &c) +{ + return u"std::move("_s + c + u')'; +} diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.h b/sources/shiboken6/generator/shiboken/shibokengenerator.h index 6802cf9e6..767af579d 100644 --- a/sources/shiboken6/generator/shiboken/shibokengenerator.h +++ b/sources/shiboken6/generator/shiboken/shibokengenerator.h @@ -311,6 +311,8 @@ protected: /// Return the format character for C++->Python->C++ conversion (Py_BuildValue) static const QHash &formatUnits(); + static QString stdMove(const QString &c); + private: static QString getModuleHeaderFileBaseName(const QString &moduleName = QString()); static QString cpythonGetterFunctionName(const QString &name, diff --git a/sources/shiboken6/tests/libsmart/CMakeLists.txt b/sources/shiboken6/tests/libsmart/CMakeLists.txt index 52a85e2a1..1a3a989da 100644 --- a/sources/shiboken6/tests/libsmart/CMakeLists.txt +++ b/sources/shiboken6/tests/libsmart/CMakeLists.txt @@ -4,6 +4,7 @@ set(libsmart_SRC smart.cpp stdsharedptrtestbench.cpp stdoptionaltestbench.cpp +stduniqueptrtestbench.cpp ) add_library(libsmart SHARED ${libsmart_SRC}) diff --git a/sources/shiboken6/tests/libsmart/smart.h b/sources/shiboken6/tests/libsmart/smart.h index 0683559d1..9819ef43d 100644 --- a/sources/shiboken6/tests/libsmart/smart.h +++ b/sources/shiboken6/tests/libsmart/smart.h @@ -10,5 +10,6 @@ #include "smart_registry.h" #include "stdsharedptrtestbench.h" #include "stdoptionaltestbench.h" +#include "stduniqueptrtestbench.h" #endif // SMART_H diff --git a/sources/shiboken6/tests/libsmart/stduniqueptrtestbench.cpp b/sources/shiboken6/tests/libsmart/stduniqueptrtestbench.cpp new file mode 100644 index 000000000..e1056412c --- /dev/null +++ b/sources/shiboken6/tests/libsmart/stduniqueptrtestbench.cpp @@ -0,0 +1,117 @@ +// Copyright (C) 2022 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "stduniqueptrtestbench.h" +#include "smart_integer.h" + +#include + +std::ostream &operator<<(std::ostream &str, const std::unique_ptr &p) +{ + str << "unique_ptr("; + if (p.get()) + str << p->value(); + else + str << "nullptr"; + str << ')'; + return str; +} + +std::ostream &operator<<(std::ostream &str, const std::unique_ptr &p) +{ + str << "unique_ptr("; + if (p.get()) + str << *p; + else + str << "nullptr"; + str << ')'; + return str; +} + +StdUniquePtrTestBench::StdUniquePtrTestBench() = default; +StdUniquePtrTestBench::~StdUniquePtrTestBench() = default; + +std::unique_ptr StdUniquePtrTestBench::createInteger(int v) +{ + auto result = std::make_unique(); + result->setValue(v); + return result; +} + +std::unique_ptr StdUniquePtrTestBench::createNullInteger() +{ + return {}; +} + +void StdUniquePtrTestBench::printInteger(const std::unique_ptr &p) +{ + std::cerr << __FUNCTION__ << ' ' << p << '\n'; +} + +void StdUniquePtrTestBench::takeInteger(std::unique_ptr p) +{ + std::cerr << __FUNCTION__ << ' ' << p << '\n'; +} + +std::unique_ptr StdUniquePtrTestBench::createInt(int v) +{ + return std::make_unique(v); +} + +std::unique_ptr StdUniquePtrTestBench::createNullInt() +{ + return {}; +} + +void StdUniquePtrTestBench::printInt(const std::unique_ptr &p) +{ + std::cerr << __FUNCTION__ << ' ' << p << '\n'; +} + +void StdUniquePtrTestBench::takeInt(std::unique_ptr p) +{ + std::cerr << __FUNCTION__ << ' ' << p << '\n'; +} + +StdUniquePtrVirtualMethodTester::StdUniquePtrVirtualMethodTester() = default; + +StdUniquePtrVirtualMethodTester::~StdUniquePtrVirtualMethodTester() = default; + +bool StdUniquePtrVirtualMethodTester::testModifyIntegerByRef(int value, int expectedValue) +{ + auto p = std::make_unique(); + p->setValue(value); + const int actualValue = doModifyIntegerByRef(p); + return p.get() != nullptr && actualValue == expectedValue; +} + +bool StdUniquePtrVirtualMethodTester::testModifyIntegerValue(int value, int expectedValue) +{ + auto p = std::make_unique(); + p->setValue(value); + const int actualValue = doModifyIntegerByValue(std::move(p)); + return p.get() == nullptr && actualValue == expectedValue; +} + +bool StdUniquePtrVirtualMethodTester::testCreateInteger(int value, int expectedValue) +{ + auto p = doCreateInteger(value); + return p.get() != nullptr && p->value() == expectedValue; +} + +std::unique_ptr StdUniquePtrVirtualMethodTester::doCreateInteger(int v) +{ + auto result = std::make_unique(); + result->setValue(v); + return result; +} + +int StdUniquePtrVirtualMethodTester::doModifyIntegerByRef(const std::unique_ptr &p) +{ + return p->value() + 1; +} + +int StdUniquePtrVirtualMethodTester::doModifyIntegerByValue(std::unique_ptr p) +{ + return p->value() + 1; +} diff --git a/sources/shiboken6/tests/libsmart/stduniqueptrtestbench.h b/sources/shiboken6/tests/libsmart/stduniqueptrtestbench.h new file mode 100644 index 000000000..8d3db740a --- /dev/null +++ b/sources/shiboken6/tests/libsmart/stduniqueptrtestbench.h @@ -0,0 +1,46 @@ +// Copyright (C) 2022 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#ifndef STDUNIQUEPTRTESTBENCH_H +#define STDUNIQUEPTRTESTBENCH_H + +#include "libsmartmacros.h" + +#include + +class Integer; + +class LIB_SMART_API StdUniquePtrTestBench +{ +public: + StdUniquePtrTestBench(); + ~StdUniquePtrTestBench(); + + static std::unique_ptr createInteger(int v = 42); + static std::unique_ptr createNullInteger(); + static void printInteger(const std::unique_ptr &p); + static void takeInteger(std::unique_ptr p); // Call with std::move() + + static std::unique_ptr createInt(int v = 42); + static std::unique_ptr createNullInt(); + static void printInt(const std::unique_ptr &p); + static void takeInt(std::unique_ptr p); // Call with std::move() +}; + +class LIB_SMART_API StdUniquePtrVirtualMethodTester +{ +public: + StdUniquePtrVirtualMethodTester(); + virtual ~StdUniquePtrVirtualMethodTester(); + + bool testModifyIntegerByRef(int value, int expectedValue); + bool testModifyIntegerValue(int value, int expectedValue); + bool testCreateInteger(int value, int expectedValue); + +protected: + virtual std::unique_ptr doCreateInteger(int v); + virtual int doModifyIntegerByRef(const std::unique_ptr &p); + virtual int doModifyIntegerByValue(std::unique_ptr p); +}; + +#endif // STDUNIQUEPTRTESTBENCH_H diff --git a/sources/shiboken6/tests/smartbinding/CMakeLists.txt b/sources/shiboken6/tests/smartbinding/CMakeLists.txt index 196470f96..b4b86d733 100644 --- a/sources/shiboken6/tests/smartbinding/CMakeLists.txt +++ b/sources/shiboken6/tests/smartbinding/CMakeLists.txt @@ -20,7 +20,11 @@ ${CMAKE_CURRENT_BINARY_DIR}/smart/std_shared_ptr_int_wrapper.cpp ${CMAKE_CURRENT_BINARY_DIR}/smart/std_wrapper.cpp ${CMAKE_CURRENT_BINARY_DIR}/smart/std_optional_int_wrapper.cpp ${CMAKE_CURRENT_BINARY_DIR}/smart/std_optional_integer_wrapper.cpp +${CMAKE_CURRENT_BINARY_DIR}/smart/std_unique_ptr_integer_wrapper.cpp +${CMAKE_CURRENT_BINARY_DIR}/smart/std_unique_ptr_int_wrapper.cpp ${CMAKE_CURRENT_BINARY_DIR}/smart/stdoptionaltestbench_wrapper.cpp +${CMAKE_CURRENT_BINARY_DIR}/smart/stduniqueptrtestbench_wrapper.cpp +${CMAKE_CURRENT_BINARY_DIR}/smart/stduniqueptrvirtualmethodtester_wrapper.cpp ) configure_file("${CMAKE_CURRENT_SOURCE_DIR}/smart-binding.txt.in" diff --git a/sources/shiboken6/tests/smartbinding/std_unique_ptr_test.py b/sources/shiboken6/tests/smartbinding/std_unique_ptr_test.py new file mode 100644 index 000000000..f21466302 --- /dev/null +++ b/sources/shiboken6/tests/smartbinding/std_unique_ptr_test.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python +# Copyright (C) 2022 The Qt Company Ltd. +# SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +import gc +import os +import sys +import unittest + +from pathlib import Path +sys.path.append(os.fspath(Path(__file__).resolve().parents[1])) +from shiboken_paths import init_paths +init_paths() +from smart import Integer, StdUniquePtrTestBench, StdUniquePtrVirtualMethodTester, std + + +def call_func_on_ptr(ptr): + ptr.printInteger() + + +class VirtualTester(StdUniquePtrVirtualMethodTester): + + def doCreateInteger(self, v): + iv = Integer() # Construct from pointee + iv.setValue(2 * v) + return std.unique_ptr_Integer(iv) + + def doModifyIntegerByRef(self, p): + return 2 * p.value() + + def doModifyIntegerByValue(self, p): + return 2 * p.value() + + +class StdUniquePtrTests(unittest.TestCase): + def testInteger(self): + p = StdUniquePtrTestBench.createInteger() + StdUniquePtrTestBench.printInteger(p) # unique_ptr by ref + self.assertTrue(p) + + call_func_on_ptr(p) + self.assertTrue(p) + + StdUniquePtrTestBench.takeInteger(p) # unique_ptr by value, takes pointee + self.assertFalse(p) + + np = StdUniquePtrTestBench.createNullInteger() + StdUniquePtrTestBench.printInteger(np) + self.assertFalse(np) + self.assertRaises(AttributeError, call_func_on_ptr, np) + + iv = Integer() # Construct from pointee + iv.setValue(42) + np = std.unique_ptr_Integer(iv) + self.assertEqual(np.value(), 42) + + def testInt(self): + p = StdUniquePtrTestBench.createInt() # unique_ptr by ref + StdUniquePtrTestBench.printInt(p) + StdUniquePtrTestBench.takeInt(p) # unique_ptr by value, takes pointee + self.assertFalse(p) + + np = StdUniquePtrTestBench.createNullInt() + StdUniquePtrTestBench.printInt(np) + self.assertFalse(np) + + def testVirtuals(self): + """Test whether code generating virtual function overrides is generated + correctly.""" + p = StdUniquePtrTestBench.createInteger() + p.setValue(42) + v = StdUniquePtrVirtualMethodTester() + self.assertTrue(v.testCreateInteger(42, 42)) + self.assertTrue(v.testModifyIntegerByRef(42, 43)) # Default implementation increments + self.assertTrue(v.testModifyIntegerValue(42, 43)) + + v = VirtualTester() # Reimplemented methods double values + self.assertTrue(v.testCreateInteger(42, 84)) + self.assertTrue(v.testModifyIntegerByRef(42, 84)) + self.assertTrue(v.testModifyIntegerValue(42, 84)) + + +if __name__ == '__main__': + unittest.main() diff --git a/sources/shiboken6/tests/smartbinding/typesystem_smart.xml b/sources/shiboken6/tests/smartbinding/typesystem_smart.xml index a1bfe03aa..748e5a0d3 100644 --- a/sources/shiboken6/tests/smartbinding/typesystem_smart.xml +++ b/sources/shiboken6/tests/smartbinding/typesystem_smart.xml @@ -54,6 +54,13 @@ + + + + @@ -64,4 +71,7 @@ + + + -- cgit v1.2.3