shiboken6: Implement opaque containers for getters (non-const)

Extract helpers from the opaque containers generation for fields
and use them for function returns if the type is modified  accordingly.

[ChangeLog][shiboken6] Getters returning containers by reference can
now be modified to return an opaque container by modifying the return
type accordingly.

Pick-to: 6.2
Task-number: PYSIDE-1605
Change-Id: Ieaf5eb92d248d3a23e511222e5f61823e85540c0
Reviewed-by: Christian Tismer <tismer@stackless.com>
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

10 files changed, 95 insertions, 16 deletions

diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
index d220275b6..4264eda3c 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
@@ -780,6 +780,14 @@ const QString &AbstractMetaFunction::modifiedTypeName() const
     return d->m_modifiedTypeName;
 }
 
+bool AbstractMetaFunction::generateOpaqueContainerReturn() const
+{
+    return isTypeModified()
+        && d->m_type.typeUsagePattern() == AbstractMetaType::ContainerPattern
+        && d->m_type.referenceType() == LValueReference
+        && d->m_type.generateOpaqueContainerForGetter(d->m_modifiedTypeName);
+}
+
 bool AbstractMetaFunction::isModifiedToArray(int argumentIndex) const
 {
     for (const auto &modification : modifications(declaringClass())) {
diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.h b/sources/shiboken6/ApiExtractor/abstractmetafunction.h
index 9bad1cd44..1cbae5ed6 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafunction.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.h
@@ -310,6 +310,7 @@ public:
 
     const QString &modifiedTypeName() const;
     bool isTypeModified() const { return !modifiedTypeName().isEmpty(); }
+    bool generateOpaqueContainerReturn() const;
 
     bool isModifiedToArray(int argumentIndex) const;
 
diff --git a/sources/shiboken6/ApiExtractor/abstractmetatype.cpp b/sources/shiboken6/ApiExtractor/abstractmetatype.cpp
index 3cfc5b67f..80ab1828d 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetatype.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetatype.cpp
@@ -136,6 +136,8 @@ public:
     QString formatSignature(bool minimal) const;
     QString formatPythonSignature() const;
     bool equals(const AbstractMetaTypeData &rhs) const;
+    template <class Predicate>
+    bool generateOpaqueContainer(Predicate p) const;
 
     const TypeEntry *m_typeEntry;
     AbstractMetaTypeList m_instantiations;
@@ -929,15 +931,16 @@ AbstractMetaType AbstractMetaType::fromAbstractMetaClass(const AbstractMetaClass
     return fromTypeEntry(metaClass->typeEntry());
 }
 
-bool AbstractMetaType::generateOpaqueContainer() const
+template <class Predicate> // Predicate(containerTypeEntry, signature)
+bool AbstractMetaTypeData::generateOpaqueContainer(Predicate pred) const
 {
-    if (!isContainer())
+    if (m_pattern != AbstractMetaType::ContainerPattern)
         return false;
-    auto *containerTypeEntry = static_cast<const ContainerTypeEntry *>(typeEntry());
+    auto *containerTypeEntry = static_cast<const ContainerTypeEntry *>(m_typeEntry);
     auto kind = containerTypeEntry->containerKind();
     if (kind != ContainerTypeEntry::ListContainer)
         return false;
-    const auto &instantation =  d->m_instantiations.constFirst();
+    const auto &instantation =  m_instantiations.constFirst();
     if (instantation.referenceType() != NoReference)
         return false;
     const QString signature = instantation.cppSignature();
@@ -951,7 +954,7 @@ bool AbstractMetaType::generateOpaqueContainer() const
     case TypeEntry::BasicValueType:
     case TypeEntry::ObjectType:
     case TypeEntry::CustomType:
-        result = containerTypeEntry->generateOpaqueContainer(signature);
+        result = pred(containerTypeEntry, signature);
         break;
     default:
         break;
@@ -959,6 +962,29 @@ bool AbstractMetaType::generateOpaqueContainer() const
     return result;
 }
 
+// Simple predicate for checking whether an opaque container should be generated
+static bool opaqueContainerPredicate(const ContainerTypeEntry *t,
+                                     const QString &signature)
+{
+    return t->generateOpaqueContainer(signature);
+}
+
+bool AbstractMetaType::generateOpaqueContainer() const
+{
+    return d->generateOpaqueContainer(opaqueContainerPredicate);
+}
+
+// Helper for determining whether a function should return an opaque container,
+// that is, the function return type is modified accordingly
+// (cf AbstractMetaFunction::generateOpaqueContainerReturn())
+bool AbstractMetaType::generateOpaqueContainerForGetter(const QString &modifiedType) const
+{
+    auto predicate = [&modifiedType](const ContainerTypeEntry *t, const QString &signature) {
+        return t->opaqueContainerName(signature) == modifiedType;
+    };
+    return d->generateOpaqueContainer(predicate);
+}
+
 #ifndef QT_NO_DEBUG_STREAM
 void AbstractMetaType::formatDebug(QDebug &debug) const
 {
diff --git a/sources/shiboken6/ApiExtractor/abstractmetatype.h b/sources/shiboken6/ApiExtractor/abstractmetatype.h
index f3c6a22ab..e06535799 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetatype.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetatype.h
@@ -250,6 +250,8 @@ public:
     bool valueTypeWithCopyConstructorOnlyPassed() const;
     /// Returns whether to generate an opaque container for the type
     bool generateOpaqueContainer() const;
+    /// Returns whether to generate an opaque container for a getter
+    bool generateOpaqueContainerForGetter(const QString &modifiedType) const;
 
     /// Types for which libshiboken has built-in primitive converters
     static const QSet<QString> &cppFloatTypes();
diff --git a/sources/shiboken6/doc/typesystem_containers.rst b/sources/shiboken6/doc/typesystem_containers.rst
index d74563b04..ac22df558 100644
--- a/sources/shiboken6/doc/typesystem_containers.rst
+++ b/sources/shiboken6/doc/typesystem_containers.rst
@@ -26,6 +26,10 @@ they are converted to Python containers on read access. By a field modification,
 (see :ref:`modify-field`), it is possible to obtain an opaque container
 which avoids the conversion and allows for direct modification of elements.
 
+Getters returning references can also be modified to return opaque containers.
+This is done by modifying the return type to the name of the opaque container
+(see :ref:`replace-type`).
+
 The table below lists the functions supported for opaque sequence containers
 besides the sequence protocol (element access via index and ``len()``). Both
 the STL and the Qt naming convention (which resembles Python's) are supported:
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index b7d826d3d..a6abeb077 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -190,6 +190,26 @@ const ProtocolEntries &sequenceProtocols()
     return result;
 }
 
+// Return name of function to create PyObject wrapping a container
+static QString opaqueContainerCreationFunc(const AbstractMetaType &type)
+{
+    const auto *containerTypeEntry =
+        static_cast<const ContainerTypeEntry *>(type.typeEntry());
+    const auto *instantiationTypeEntry =
+        type.instantiations().constFirst().typeEntry();
+    return u"create"_qs
+           + containerTypeEntry->opaqueContainerName(instantiationTypeEntry->name());
+}
+
+// Write declaration of the function to create PyObject wrapping a container
+static void writeOpaqueContainerCreationFuncDecl(TextStream &s, const QString &name,
+                                                 AbstractMetaType type)
+{
+    type.setReferenceType(NoReference);
+    type.setConstant(false);
+    s << "PyObject *" << name << '(' << type.cppSignature() << "*);\n";
+}
+
 CppGenerator::CppGenerator() = default;
 
 QString CppGenerator::fileNameSuffix() const
@@ -3796,17 +3816,23 @@ void CppGenerator::writeMethodCall(TextStream &s, const AbstractMetaFunctionCPtr
             }
 
             // Convert result
+            const auto funcType = func->type();
             if (!func->conversionRule(TypeSystem::TargetLangCode, 0).isEmpty()) {
                 writeConversionRule(s, func, TypeSystem::TargetLangCode, QLatin1String(PYTHON_RETURN_VAR));
             } else if (!isCtor && !func->isInplaceOperator() && !func->isVoid()
                 && !func->injectedCodeHasReturnValueAttribution(TypeSystem::TargetLangCode)) {
-                s << PYTHON_RETURN_VAR << " = ";
                 if (func->type().isObjectTypeUsedAsValueType()) {
-                    s << "Shiboken::Object::newObject("
+                    s << PYTHON_RETURN_VAR << " = Shiboken::Object::newObject("
                         << cpythonTypeNameExt(func->type().typeEntry())
                         << ", " << CPP_RETURN_VAR << ", true, true)";
+                } else if (func->generateOpaqueContainerReturn()) {
+                    const QString creationFunc = opaqueContainerCreationFunc(funcType);
+                    writeOpaqueContainerCreationFuncDecl(s, creationFunc, funcType);
+                    s << PYTHON_RETURN_VAR << " = " << creationFunc
+                        << "(&" << CPP_RETURN_VAR << ");\n";
                 } else {
-                    writeToPythonConversion(s, func->type(), func->ownerClass(), QLatin1String(CPP_RETURN_VAR));
+                    s << PYTHON_RETURN_VAR << " = ";
+                    writeToPythonConversion(s, funcType, func->ownerClass(), QLatin1String(CPP_RETURN_VAR));
                 }
                 s << ";\n";
             }
@@ -4716,14 +4742,9 @@ void CppGenerator::writeGetterFunction(TextStream &s,
 
     if (metaField.generateOpaqueContainer()
         && fieldType.generateOpaqueContainer()) {
-        const auto *containerTypeEntry =
-            static_cast<const ContainerTypeEntry *>(fieldType.typeEntry());
-        const auto *instantiationTypeEntry =
-            fieldType.instantiations().constFirst().typeEntry();
-        const QString creationFunc =
-            u"create"_qs + containerTypeEntry->opaqueContainerName(instantiationTypeEntry->name());
-        s << "PyObject *" << creationFunc << '(' << fieldType.cppSignature() << "*);\n"
-            << "PyObject *pyOut = " << creationFunc
+        const QString creationFunc = opaqueContainerCreationFunc(fieldType);
+        writeOpaqueContainerCreationFuncDecl(s, creationFunc, fieldType);
+        s << "PyObject *pyOut = " << creationFunc
             << "(&" << cppField << ");\nPy_IncRef(pyOut);\n"
             << "return pyOut;\n" << outdent << "}\n";
         return;
diff --git a/sources/shiboken6/tests/libminimal/listuser.cpp b/sources/shiboken6/tests/libminimal/listuser.cpp
index 402696acd..5ad570faa 100644
--- a/sources/shiboken6/tests/libminimal/listuser.cpp
+++ b/sources/shiboken6/tests/libminimal/listuser.cpp
@@ -123,3 +123,8 @@ void ListUser::setStdIntList(const std::list<int> &l)
 {
     m_stdIntList = l;
 }
+
+std::list<int> &ListUser::getIntList()
+{
+    return m_stdIntList;
+}
diff --git a/sources/shiboken6/tests/libminimal/listuser.h b/sources/shiboken6/tests/libminimal/listuser.h
index 31c4efbd1..e3b38049c 100644
--- a/sources/shiboken6/tests/libminimal/listuser.h
+++ b/sources/shiboken6/tests/libminimal/listuser.h
@@ -71,6 +71,7 @@ struct LIBMINIMAL_API ListUser
     int callSumListOfIntLists(std::list<std::list<int> > intListList) { return sumListOfIntLists(intListList); }
 
     void setStdIntList(const std::list<int> &l);
+    std::list<int> &getIntList();
 
     std::list<int> m_stdIntList;
 };
diff --git a/sources/shiboken6/tests/minimalbinding/listuser_test.py b/sources/shiboken6/tests/minimalbinding/listuser_test.py
index 1a11ccfbf..7bb65d359 100644
--- a/sources/shiboken6/tests/minimalbinding/listuser_test.py
+++ b/sources/shiboken6/tests/minimalbinding/listuser_test.py
@@ -345,6 +345,12 @@ class ListOfIntListConversionTest(unittest.TestCase):
         self.assertEqual(len(lu.m_stdIntList), 3)
         self.assertEqual(lu.m_stdIntList[2], 5)
 
+        # Access list via getter
+        l = lu.getIntList()
+        l.append(6)
+        self.assertEqual(len(lu.m_stdIntList), 4)
+        self.assertEqual(lu.m_stdIntList[3], 6)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/sources/shiboken6/tests/minimalbinding/typesystem_minimal.xml b/sources/shiboken6/tests/minimalbinding/typesystem_minimal.xml
index 40973fa5f..94ec4a649 100644
--- a/sources/shiboken6/tests/minimalbinding/typesystem_minimal.xml
+++ b/sources/shiboken6/tests/minimalbinding/typesystem_minimal.xml
@@ -48,6 +48,11 @@
     </value-type>
     <value-type name="ListUser">
         <modify-field name="m_stdIntList" opaque-container="yes"/>
+        <modify-function signature="getIntList()">
+            <modify-argument index="return">
+                <replace-type modified-type="StdIntList"/>
+            </modify-argument>
+        </modify-function>
     </value-type>
     <value-type name="MinBoolUser"/>