shiboken6: Generate rich comparison for smart pointers

Add the pointee class to the GeneratorContext for smart pointers
and generate a comparison operator for the pointee.
Use the pointee's comparison operators if there are any; else
generate a simple equality check based on pointee address.

[ChangeLog][shiboken6] Comparison operators for pointees of
smart pointers are now generated.

Fixes: PYSIDE-1711
Change-Id: Ib21b90a4ccfe635ea051831a5b66a79ded06b194
Reviewed-by: Christian Tismer <tismer@stackless.com>

8 files changed, 252 insertions, 12 deletions

diff --git a/sources/shiboken6/generator/generator.cpp b/sources/shiboken6/generator/generator.cpp
index ef9cf3d47..49652a94d 100644
--- a/sources/shiboken6/generator/generator.cpp
+++ b/sources/shiboken6/generator/generator.cpp
@@ -465,12 +465,14 @@ GeneratorContext Generator::contextForClass(const AbstractMetaClass *c) const
 }
 
 GeneratorContext Generator::contextForSmartPointer(const AbstractMetaClass *c,
-                                                   const AbstractMetaType &t)
+                                                   const AbstractMetaType &t,
+                                                   const AbstractMetaClass *pointeeClass)
 {
     GeneratorContext result;
     result.m_metaClass = c;
     result.m_preciseClassType = t;
     result.m_type = GeneratorContext::SmartPointer;
+    result.m_pointeeClass = pointeeClass;
     return result;
 }
 
@@ -493,8 +495,14 @@ bool Generator::generate()
                                                            smartPointers)));
             return false;
         }
-        if (!generateFileForContext(contextForSmartPointer(smartPointerClass, type)))
+        const AbstractMetaClass *pointeeClass = nullptr;
+        const auto *instantiatedType = type.instantiations().constFirst().typeEntry();
+        if (instantiatedType->isComplex()) // not a C++ primitive
+            pointeeClass = AbstractMetaClass::findClass(m_d->api.classes(), instantiatedType);
+        if (!generateFileForContext(contextForSmartPointer(smartPointerClass, type,
+                                                           pointeeClass))) {
             return false;
+        }
     }
     return finishGeneration();
 }
diff --git a/sources/shiboken6/generator/generator.h b/sources/shiboken6/generator/generator.h
index 82f231b93..62c5e5aeb 100644
--- a/sources/shiboken6/generator/generator.h
+++ b/sources/shiboken6/generator/generator.h
@@ -154,6 +154,7 @@ public:
 
     const AbstractMetaClass *metaClass() const { return m_metaClass; }
     const AbstractMetaType &preciseType() const { return m_preciseClassType; }
+    const AbstractMetaClass *pointeeClass() const { return m_pointeeClass; }
 
     bool forSmartPointer() const { return m_type == SmartPointer; }
     bool useWrapper() const { return m_type ==  WrappedClass; }
@@ -168,6 +169,7 @@ public:
 
 private:
     const AbstractMetaClass *m_metaClass = nullptr;
+    const AbstractMetaClass *m_pointeeClass = nullptr;
     AbstractMetaType m_preciseClassType;
     QString m_wrappername;
     Type m_type = Class;
@@ -259,7 +261,8 @@ protected:
 
     virtual GeneratorContext contextForClass(const AbstractMetaClass *c) const;
     static GeneratorContext contextForSmartPointer(const AbstractMetaClass *c,
-                                                   const AbstractMetaType &t);
+                                                   const AbstractMetaType &t,
+                                                   const AbstractMetaClass *pointeeClass = nullptr);
 
     /// Generates a file for given AbstractMetaClass or AbstractMetaType (smart pointer case).
     bool generateFileForContext(const GeneratorContext &context);
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index a9d41f1f0..61f2c8ac8 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -392,6 +392,12 @@ static void writePyGetSetDefEntry(TextStream &s, const QString &name,
         << (setFunc.isEmpty() ? QLatin1String(NULL_PTR) : setFunc) << "},\n";
 }
 
+static bool generateRichComparison(const GeneratorContext &c)
+{
+    return c.forSmartPointer()
+        || (!c.metaClass()->isNamespace() && c.metaClass()->hasComparisonOperatorOverload());
+}
+
 /*!
     Function used to write the class generated binding code on the buffer
     \param s the output buffer
@@ -737,9 +743,12 @@ void CppGenerator::generateClass(TextStream &s, const GeneratorContext &classCon
         writeMappingMethods(s, metaClass, classContext);
     }
 
-    if (!metaClass->isNamespace() && metaClass->hasComparisonOperatorOverload()) {
+    if (generateRichComparison(classContext)) {
         s << "// Rich comparison\n";
-        writeRichCompareFunction(s, classContext);
+        if (classContext.forSmartPointer())
+            writeSmartPointerRichCompareFunction(s, classContext);
+        else
+            writeRichCompareFunction(s, classContext);
     }
 
     if (shouldGenerateGetSetList(metaClass) && !classContext.forSmartPointer()) {
@@ -4370,7 +4379,7 @@ void CppGenerator::writeClassDefinition(TextStream &s,
     tp_flags.append(QLatin1String("|Py_TPFLAGS_HAVE_GC"));
 
     QString tp_richcompare;
-    if (!metaClass->isNamespace() && metaClass->hasComparisonOperatorOverload())
+    if (generateRichComparison(classContext))
         tp_richcompare = cpythonBaseName(metaClass) + QLatin1String("_richcompare");
 
     QString tp_getset;
@@ -4904,11 +4913,10 @@ void CppGenerator::writeSetterFunction(TextStream &s, const QPropertySpec &prope
         << "return 0;\n" << outdent << "}\n\n";
 }
 
-void CppGenerator::writeRichCompareFunction(TextStream &s,
-                                            const GeneratorContext &context) const
+void CppGenerator::writeRichCompareFunctionHeader(TextStream &s,
+                                                  const QString &baseName,
+                                                  const GeneratorContext &context) const
 {
-    const AbstractMetaClass *metaClass = context.metaClass();
-    QString baseName = cpythonBaseName(metaClass);
     s << "static PyObject * ";
     s << baseName << "_richcompare(PyObject *self, PyObject *" << PYTHON_ARG
         << ", int op)\n{\n" << indent;
@@ -4918,6 +4926,17 @@ void CppGenerator::writeRichCompareFunction(TextStream &s,
         << PYTHON_TO_CPPCONVERSION_STRUCT << ' ' << PYTHON_TO_CPP_VAR << ";\n";
     writeUnusedVariableCast(s, QLatin1String(PYTHON_TO_CPP_VAR));
     s << '\n';
+}
+
+static const char richCompareComment[] =
+    "// PYSIDE-74: By default, we redirect to object's tp_richcompare (which is `==`, `!=`).\n";
+
+void CppGenerator::writeRichCompareFunction(TextStream &s,
+                                            const GeneratorContext &context) const
+{
+    const AbstractMetaClass *metaClass = context.metaClass();
+    QString baseName = cpythonBaseName(metaClass);
+    writeRichCompareFunctionHeader(s, baseName, context);
 
     s << "switch (op) {\n";
     {
@@ -5012,13 +5031,19 @@ void CppGenerator::writeRichCompareFunction(TextStream &s,
         s << "default:\n";
         {
             Indentation indent(s);
-            s << "// PYSIDE-74: By default, we redirect to object's tp_richcompare (which is `==`, `!=`).\n"
+            s << richCompareComment
                 << "return FallbackRichCompare(self, " << PYTHON_ARG << ", op);\n"
                 << "goto " << baseName << "_RichComparison_TypeError;\n";
         }
     }
     s << "}\n\n";
 
+    writeRichCompareFunctionFooter(s, baseName);
+}
+
+void CppGenerator::writeRichCompareFunctionFooter(TextStream &s,
+                                                  const QString &baseName) const
+{
     s << "if (" << PYTHON_RETURN_VAR << " && !PyErr_Occurred())\n";
     {
         Indentation indent(s);
@@ -5030,6 +5055,116 @@ void CppGenerator::writeRichCompareFunction(TextStream &s,
         << outdent << "}\n\n";
 }
 
+using ComparisonOperatorList = QList<AbstractMetaFunction::ComparisonOperatorType>;
+
+// Return the available comparison operators for smart pointers
+static ComparisonOperatorList smartPointeeComparisons(const GeneratorContext &context)
+{
+    Q_ASSERT(context.forSmartPointer());
+    auto *te = context.preciseType().instantiations().constFirst().typeEntry();
+    if (te->isExtendedCppPrimitive()) { // Primitive pointee types have all
+        return {AbstractMetaFunction::OperatorEqual,
+                AbstractMetaFunction::OperatorNotEqual,
+                AbstractMetaFunction::OperatorLess,
+                AbstractMetaFunction::OperatorLessEqual,
+                AbstractMetaFunction::OperatorGreater,
+                AbstractMetaFunction::OperatorGreaterEqual};
+    }
+
+    auto *pointeeClass = context.pointeeClass();
+    if (!pointeeClass)
+        return {};
+
+    ComparisonOperatorList result;
+    const auto &comparisons =
+        pointeeClass->operatorOverloads(OperatorQueryOption::SymmetricalComparisonOp);
+    for (const auto &f : comparisons) {
+        const auto ct = f->comparisonOperatorType().value();
+        if (!result.contains(ct))
+            result.append(ct);
+    }
+    return result;
+}
+
+void CppGenerator::writeSmartPointerRichCompareFunction(TextStream &s,
+                                                        const GeneratorContext &context) const
+{
+    static const char selfPointeeVar[] = "cppSelfPointee";
+    static const char cppArg0PointeeVar[] = "cppArg0Pointee";
+
+    const AbstractMetaClass *metaClass = context.metaClass();
+    QString baseName = cpythonBaseName(metaClass);
+    writeRichCompareFunctionHeader(s, baseName, context);
+
+    s << "if (";
+    writeTypeCheck(s, context.preciseType(), QLatin1String(PYTHON_ARG));
+    s << ") {\n" << indent;
+    writeArgumentConversion(s, context.preciseType(), QLatin1String(CPP_ARG0),
+                            QLatin1String(PYTHON_ARG), metaClass);
+
+    const auto *te = context.preciseType().typeEntry();
+    Q_ASSERT(te->isSmartPointer());
+    const auto *ste = static_cast<const SmartPointerTypeEntry *>(te);
+
+    s << "const auto *" << selfPointeeVar << " = " << CPP_SELF_VAR
+        << '.' << ste->getter() << "();\n";
+    s << "const auto *" << cppArg0PointeeVar << " = " << CPP_ARG0
+        << '.' << ste->getter() << "();\n";
+
+    // If we have an object without any comparisons, only generate a simple
+    // equality check by pointee address
+    auto availableOps = smartPointeeComparisons(context);
+    const bool comparePointeeAddressOnly = availableOps.isEmpty();
+    if (comparePointeeAddressOnly) {
+        availableOps << AbstractMetaFunction::OperatorEqual
+            << AbstractMetaFunction::OperatorNotEqual;
+    } else {
+        // For value types with operators, we complain about nullptr
+        s << "if (" << selfPointeeVar << " == nullptr || " << cppArg0PointeeVar
+            << " == nullptr) {\n" << indent
+            << "PyErr_SetString(PyExc_NotImplementedError, \"nullptr passed to comparison.\");\n"
+            << returnStatement(m_currentErrorCode)  << '\n' << outdent << "}\n";
+    }
+
+    s << "bool " << CPP_RETURN_VAR << "= false;\n"
+        << "switch (op) {\n";
+    for (auto op : availableOps) {
+        s << "case " << AbstractMetaFunction::pythonRichCompareOpCode(op) << ":\n"
+          << indent << CPP_RETURN_VAR << " = ";
+        if (comparePointeeAddressOnly) {
+            s << selfPointeeVar << ' ' << AbstractMetaFunction::cppComparisonOperator(op)
+              << ' ' << cppArg0PointeeVar << ";\n";
+        } else {
+            // Shortcut for equality: Check pointee address
+            if (op == AbstractMetaFunction::OperatorEqual
+                || op == AbstractMetaFunction::OperatorLessEqual
+                || op == AbstractMetaFunction::OperatorGreaterEqual) {
+                s << selfPointeeVar << " == " << cppArg0PointeeVar << " || ";
+            }
+            // Generate object's comparison
+            s << "*" << selfPointeeVar << ' '
+              << AbstractMetaFunction::cppComparisonOperator(op) << " *"
+              << cppArg0PointeeVar << ";\n";
+        }
+        s << "break;\n" << outdent;
+
+    }
+    if (availableOps.size() < 6) {
+        s << "default:\n" << indent
+            << richCompareComment
+            << "return FallbackRichCompare(self, " << PYTHON_ARG << ", op);\n" << outdent;
+    }
+    s << "}\n" << PYTHON_RETURN_VAR << " = " << CPP_RETURN_VAR
+        << " ? Py_True : Py_False;\n"
+        << "Py_INCREF(" << PYTHON_RETURN_VAR << ");\n";
+
+    s << outdent << "} else {\n" << indent
+      << "goto " << baseName << "_RichComparison_TypeError;\n"
+      << outdent << "}\n";
+
+    writeRichCompareFunctionFooter(s, baseName);
+}
+
 QString CppGenerator::methodDefinitionParameters(const OverloadData &overloadData) const
 {
     const bool usePyArgs = overloadData.pythonFunctionWrapperUsesListOfArguments();
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.h b/sources/shiboken6/generator/shiboken/cppgenerator.h
index 3e3e5a21d..3b2428fb5 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.h
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.h
@@ -376,7 +376,13 @@ private:
                              const QPropertySpec &property,
                              const GeneratorContext &context) const;
 
+    void writeRichCompareFunctionHeader(TextStream &s,
+                                        const QString &baseName,
+                                        const GeneratorContext &context) const;
+    void writeRichCompareFunctionFooter(TextStream &s,
+                                        const QString &baseName) const;
     void writeRichCompareFunction(TextStream &s, const GeneratorContext &context) const;
+    void writeSmartPointerRichCompareFunction(TextStream &s, const GeneratorContext &context) const;
 
     void writeEnumsInitialization(TextStream &s, AbstractMetaEnumList &enums) const;
     void writeEnumInitialization(TextStream &s, const AbstractMetaEnum &metaEnum) const;
diff --git a/sources/shiboken6/tests/libsmart/smart.cpp b/sources/shiboken6/tests/libsmart/smart.cpp
index 81fa30c7e..6908c7881 100644
--- a/sources/shiboken6/tests/libsmart/smart.cpp
+++ b/sources/shiboken6/tests/libsmart/smart.cpp
@@ -137,6 +137,18 @@ int Obj::takeSharedPtrToInteger(SharedPtr<Integer> pInt)
     return pInt->value();
 }
 
+SharedPtr<Integer> Obj::createSharedPtrToInteger(int value)
+{
+    auto *i = new Integer;
+    i->setValue(value);
+    return SharedPtr<Integer>(i);
+}
+
+SharedPtr<Integer> Obj::createNullSharedPtrToInteger()
+{
+    return {};
+}
+
 SharedPtr<const Integer> Obj::giveSharedPtrToConstInteger()
 {
     SharedPtr<const Integer> co(new Integer);
@@ -194,6 +206,13 @@ void Integer::setValue(int v)
     m_int = v;
 }
 
+int Integer::compare(const Integer &rhs) const
+{
+    if (m_int < rhs.m_int)
+        return -1;
+    return m_int > rhs.m_int ? 1 : 0;
+}
+
 void Integer::printInteger() const
 {
     if (shouldPrint())
diff --git a/sources/shiboken6/tests/libsmart/smart_integer.h b/sources/shiboken6/tests/libsmart/smart_integer.h
index 126894120..0f2dfcec9 100644
--- a/sources/shiboken6/tests/libsmart/smart_integer.h
+++ b/sources/shiboken6/tests/libsmart/smart_integer.h
@@ -42,9 +42,41 @@ public:
     int value() const;
     void setValue(int v);
 
+    int compare(const Integer &rhs) const;
+
     int m_int; // public for testing member field access.
 };
 
+inline bool operator==(const Integer &lhs, const Integer &rhs)
+{
+    return lhs.compare(rhs) == 0;
+}
+
+inline bool operator!=(const Integer &lhs, const Integer &rhs)
+{
+    return lhs.compare(rhs) != 0;
+}
+
+inline bool operator<(const Integer &lhs, const Integer &rhs)
+{
+    return lhs.compare(rhs) < 0;
+}
+
+inline bool operator<=(const Integer &lhs, const Integer &rhs)
+{
+    return lhs.compare(rhs) <= 0;
+}
+
+inline bool operator>(const Integer &lhs, const Integer &rhs)
+{
+    return lhs.compare(rhs) > 0;
+}
+
+inline bool operator>=(const Integer &lhs, const Integer &rhs)
+{
+    return lhs.compare(rhs) >= 0;
+}
+
 namespace Smart {
 class LIB_SMART_API Integer2 : public Integer {
 public:
diff --git a/sources/shiboken6/tests/libsmart/smart_obj.h b/sources/shiboken6/tests/libsmart/smart_obj.h
index 8fe45993f..9cd8e5f08 100644
--- a/sources/shiboken6/tests/libsmart/smart_obj.h
+++ b/sources/shiboken6/tests/libsmart/smart_obj.h
@@ -46,7 +46,7 @@ public:
 
     void printObj();
     Integer takeInteger(Integer val);
-    SharedPtr<Obj> giveSharedPtrToObj();
+    static SharedPtr<Obj> giveSharedPtrToObj();
     std::vector<SharedPtr<Obj> > giveSharedPtrToObjList(int size);
     virtual SharedPtr<Integer> giveSharedPtrToInteger(); // virtual for PYSIDE-1188
     SharedPtr<const Integer> giveSharedPtrToConstInteger();
@@ -55,6 +55,9 @@ public:
     int takeSharedPtrToObj(SharedPtr<Obj> pObj);
     int takeSharedPtrToInteger(SharedPtr<Integer> pInt);
 
+    static SharedPtr<Integer> createSharedPtrToInteger(int value);
+    static SharedPtr<Integer> createNullSharedPtrToInteger();
+
     int m_integer;  // public for testing member field access.
     Integer *m_internalInteger;
 };
diff --git a/sources/shiboken6/tests/smartbinding/smart_pointer_test.py b/sources/shiboken6/tests/smartbinding/smart_pointer_test.py
index 10e761149..ce976e1ca 100644
--- a/sources/shiboken6/tests/smartbinding/smart_pointer_test.py
+++ b/sources/shiboken6/tests/smartbinding/smart_pointer_test.py
@@ -239,5 +239,39 @@ class SmartPointerTests(unittest.TestCase):
         r = o.takeSharedPtrToInteger(integer2)
         self.assertEqual(r, integer2.value())
 
+    def testSmartPointerValueComparison(self):
+        """Test a pointee class with comparison operators."""
+        four = Obj.createSharedPtrToInteger(4)
+        four2 = Obj.createSharedPtrToInteger(4)
+        five = Obj.createSharedPtrToInteger(5)
+        self.assertTrue(four == four)
+        self.assertTrue(four == four2)
+        self.assertFalse(four != four)
+        self.assertFalse(four != four2)
+        self.assertFalse(four < four)
+        self.assertTrue(four <= four)
+        self.assertFalse(four > four)
+        self.assertTrue(four >= four)
+        self.assertFalse(four == five)
+        self.assertTrue(four != five)
+        self.assertTrue(four < five)
+        self.assertTrue(four <= five)
+        self.assertFalse(four > five)
+        self.assertFalse(four >= five)
+        self.assertTrue(five > four)
+
+        self.assertRaises(NotImplementedError,
+                          lambda : Obj.createNullSharedPtrToInteger() == four)
+
+    def testSmartPointerObjectComparison(self):
+        """Test a pointee class without comparison operators."""
+        o1 = Obj.giveSharedPtrToObj()
+        o2 = Obj.giveSharedPtrToObj()
+        self.assertTrue(o1 == o1)
+        self.assertFalse(o1 != o1)
+        self.assertFalse(o1 == o2)
+        self.assertTrue(o1 != o2)
+
+
 if __name__ == '__main__':
     unittest.main()