Fix bug#513 - "Hardcoded bool return type for operator overloads"

Reviewer: Renato Araújo <renato.filho@openbossa.org>
          Luciano Wolf <luciano.wolf@openbossa.org>

10 files changed, 245 insertions, 27 deletions

diff --git a/generator/cppgenerator.cpp b/generator/cppgenerator.cpp
index 565718c35..fda0bcf35 100644
--- a/generator/cppgenerator.cpp
+++ b/generator/cppgenerator.cpp
@@ -29,7 +29,7 @@
 #include <QtCore/QTextStream>
 #include <QtCore/QDebug>
 
-// utiliy functions
+// utility functions
 inline CodeSnipList getConversionRule(TypeSystem::Language lang, const AbstractMetaFunction *function)
 {
     CodeSnipList list;
@@ -2733,7 +2733,7 @@ void CppGenerator::writeRichCompareFunction(QTextStream& s, const AbstractMetaCl
     s << baseName << "_richcompare(PyObject* self, PyObject* other, int op)" << endl;
     s << '{' << endl;
     QList<AbstractMetaFunctionList> cmpOverloads = filterGroupedOperatorFunctions(metaClass, AbstractMetaClass::ComparisonOp);
-    s << INDENT << "bool result = false;" << endl;
+    s << INDENT << "PyObject* result = 0;" << endl;
     s << INDENT << metaClass->qualifiedCppName() << "& " CPP_SELF_VAR " = *" << cpythonWrapperCPtr(metaClass) << ';' << endl;
     s << endl;
 
@@ -2786,24 +2786,10 @@ void CppGenerator::writeRichCompareFunction(QTextStream& s, const AbstractMetaCl
                     s << " cppOther = ";
                     writeToCppConversion(s, type, metaClass, "other", ExcludeReference | ExcludeConst);
                     s << ';' << endl;
-                    s << INDENT << "result = (" CPP_SELF_VAR " " << op << " cppOther);" << endl;
-                }
-                s << INDENT << '}';
-            }
 
-            // Compares with implicit conversions
-            if (comparesWithSameType && !metaClass->implicitConversions().isEmpty()) {
-                AbstractMetaType temporaryType;
-                temporaryType.setTypeEntry(metaClass->typeEntry());
-                temporaryType.setConstant(false);
-                temporaryType.setReference(false);
-                temporaryType.setTypeUsagePattern(AbstractMetaType::ValuePattern);
-                s << " else if (" << cpythonIsConvertibleFunction(metaClass->typeEntry());
-                s << "(other)) {" << endl;
-                {
-                    Indentation indent(INDENT);
-                    writeArgumentConversion(s, &temporaryType, "cppOther", "other", metaClass);
-                    s << INDENT << "result = (" CPP_SELF_VAR " " << op << " cppOther);" << endl;
+                    s << INDENT << "result = ";
+                    writeToPythonConversion(s, func->type(), metaClass, CPP_SELF_VAR " " + op + " cppOther");
+                    s << ';' << endl;
                 }
                 s << INDENT << '}';
             }
@@ -2816,19 +2802,19 @@ void CppGenerator::writeRichCompareFunction(QTextStream& s, const AbstractMetaCl
         s << INDENT << "default:" << endl;
         {
             Indentation indent(INDENT);
-            s << INDENT << "PyErr_SetString(PyExc_NotImplementedError, \"operator not implemented.\");" << endl;
-            s << INDENT << "return " << m_currentErrorCode << ';' << endl;
+            s << INDENT << "goto " << baseName << "_RichComparison_TypeError;" << endl;
         }
     }
     s << INDENT << '}' << endl << endl;
 
-    s << INDENT << "if (result)" << endl;
+    s << INDENT << "if (result && !PyErr_Occurred())" << endl;
     {
         Indentation indent(INDENT);
-        s << INDENT << "Py_RETURN_TRUE;" << endl;
+        s << INDENT << "return result;" << endl;
     }
     s << INDENT << baseName << "_RichComparison_TypeError:" << endl;
-    s << INDENT << "Py_RETURN_FALSE;" << endl << endl;
+    s << INDENT << "PyErr_SetString(PyExc_NotImplementedError, \"operator not implemented.\");" << endl;
+    s << INDENT << "return " << m_currentErrorCode << ';' << endl << endl;
     s << '}' << endl << endl;
 }
 
diff --git a/libshiboken/conversions.h b/libshiboken/conversions.h
index 8b312dfe1..11423ef6b 100644
--- a/libshiboken/conversions.h
+++ b/libshiboken/conversions.h
@@ -203,7 +203,7 @@ struct ValueTypeConverter
 template <typename T>
 struct ObjectTypeConverter
 {
-    static inline bool checkType(PyObject* pyObj) { return PyObject_TypeCheck(pyObj, SbkType<T>()); }
+    static inline bool checkType(PyObject* pyObj) { return pyObj == Py_None || PyObject_TypeCheck(pyObj, SbkType<T>()); }
     /// Py_None objects are the only objects convertible to an object type (in the form of a NULL pointer).
     static inline bool isConvertible(PyObject* pyObj) { return pyObj == Py_None || PyObject_TypeCheck(pyObj, SbkType<T>()); }
     /// Convenience overload that calls "toPython(const T*)" method.
@@ -452,7 +452,8 @@ struct EnumConverter
 template <typename CString>
 struct Converter_CString
 {
-    static inline bool checkType(PyObject* pyObj) { return PyString_Check(pyObj); }
+    // Note: 0 is also a const char* in C++, so None is accepted in checkType
+    static inline bool checkType(PyObject* pyObj) { return pyObj == Py_None || PyString_Check(pyObj); }
     static inline bool isConvertible(PyObject* pyObj) { return pyObj == Py_None || PyString_Check(pyObj); }
     static inline PyObject* toPython(void* cppobj) { return toPython(reinterpret_cast<CString>(cppobj)); }
     static inline PyObject* toPython(CString cppobj)
diff --git a/tests/libsample/CMakeLists.txt b/tests/libsample/CMakeLists.txt
index 158c0e5e8..3452c5761 100644
--- a/tests/libsample/CMakeLists.txt
+++ b/tests/libsample/CMakeLists.txt
@@ -34,6 +34,7 @@ sometime.cpp
 str.cpp
 strlist.cpp
 virtualmethods.cpp
+expression.cpp
 )
 
 include_directories(${CMAKE_CURRENT_SOURCE_DIR})
diff --git a/tests/libsample/expression.cpp b/tests/libsample/expression.cpp
new file mode 100644
index 000000000..d93d47f01
--- /dev/null
+++ b/tests/libsample/expression.cpp
@@ -0,0 +1,127 @@
+/*
+ * This file is part of the Shiboken Python Binding Generator project.
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * Contact: PySide team <contact@pyside.org>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+
+#include "expression.h"
+#include <sstream>
+
+Expression::Expression() : m_value(0), m_operation(None), m_operand1(0), m_operand2(0)
+{
+}
+
+Expression::Expression(int number) : m_value(number), m_operation(None), m_operand1(0), m_operand2(0)
+{
+}
+
+Expression::Expression(const Expression& other)
+{
+    m_operand1 = other.m_operand1 ? new Expression(*other.m_operand1) : 0;
+    m_operand2 = other.m_operand2 ? new Expression(*other.m_operand2) : 0;
+    m_value = other.m_value;
+    m_operation = other.m_operation;
+}
+
+Expression& Expression::operator=(const Expression& other)
+{
+    delete m_operand1;
+    delete m_operand2;
+    m_operand1 = other.m_operand1 ? new Expression(*other.m_operand1) : 0;
+    m_operand2 = other.m_operand2 ? new Expression(*other.m_operand2) : 0;
+    m_operation = other.m_operation;
+    m_value = other.m_value;
+    return *this;
+}
+
+Expression::~Expression()
+{
+    delete m_operand1;
+    delete m_operand2;
+}
+
+Expression Expression::operator+(const Expression& other)
+{
+    Expression expr;
+    expr.m_operation = Add;
+    expr.m_operand1 = new Expression(*this);
+    expr.m_operand2 = new Expression(other);
+    return expr;
+}
+
+Expression Expression::operator-(const Expression& other)
+{
+    Expression expr;
+    expr.m_operation = Add;
+    expr.m_operand1 = new Expression(*this);
+    expr.m_operand2 = new Expression(other);
+    return expr;
+}
+
+Expression Expression::operator<(const Expression& other)
+{
+    Expression expr;
+    expr.m_operation = LessThan;
+    expr.m_operand1 = new Expression(*this);
+    expr.m_operand2 = new Expression(other);
+    return expr;
+}
+
+Expression Expression::operator>(const Expression& other)
+{
+    Expression expr;
+    expr.m_operation = GreaterThan;
+    expr.m_operand1 = new Expression(*this);
+    expr.m_operand2 = new Expression(other);
+    return expr;
+}
+
+std::string Expression::toString() const
+{
+    if (m_operation == None) {
+        std::ostringstream s;
+        s << m_value;
+        return s.str();
+    }
+
+    std::string result;
+    result += '(';
+    result += m_operand1->toString();
+    char op;
+    switch (m_operation) {
+        case Add:
+            op = '+';
+            break;
+        case Sub:
+            op = '-';
+            break;
+        case LessThan:
+            op = '<';
+            break;
+        case GreaterThan:
+            op = '<';
+            break;
+    }
+    result += op;
+    result += m_operand2->toString();
+    result += ')';
+    return result;
+}
+
diff --git a/tests/libsample/expression.h b/tests/libsample/expression.h
new file mode 100644
index 000000000..608d5b5e4
--- /dev/null
+++ b/tests/libsample/expression.h
@@ -0,0 +1,58 @@
+/*
+ * This file is part of the Shiboken Python Binding Generator project.
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * Contact: PySide team <contact@pyside.org>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+
+#ifndef EXPRESSION_H
+#define EXPRESSION_H
+
+#include "libsamplemacros.h"
+#include <string>
+
+class LIBSAMPLE_API Expression
+{
+public:
+    enum Operation {
+        None, Add, Sub, LessThan, GreaterThan
+    };
+
+    Expression(int number);
+    Expression(const Expression& other);
+    Expression& operator=(const Expression& other);
+
+    ~Expression();
+
+    Expression operator>(const Expression& other);
+    Expression operator<(const Expression& other);
+    Expression operator+(const Expression& other);
+    Expression operator-(const Expression& other);
+
+    std::string toString() const;
+private:
+    int m_value;
+    Operation m_operation;
+    Expression* m_operand1;
+    Expression* m_operand2;
+
+    Expression();
+};
+
+#endif // EXPRESSION_H
diff --git a/tests/samplebinding/CMakeLists.txt b/tests/samplebinding/CMakeLists.txt
index 5e3498cbf..d3e27591f 100644
--- a/tests/samplebinding/CMakeLists.txt
+++ b/tests/samplebinding/CMakeLists.txt
@@ -22,6 +22,7 @@ ${CMAKE_CURRENT_BINARY_DIR}/sample/derived_wrapper.cpp
 ${CMAKE_CURRENT_BINARY_DIR}/sample/derived_someinnerclass_wrapper.cpp
 ${CMAKE_CURRENT_BINARY_DIR}/sample/echo_wrapper.cpp
 ${CMAKE_CURRENT_BINARY_DIR}/sample/event_wrapper.cpp
+${CMAKE_CURRENT_BINARY_DIR}/sample/expression_wrapper.cpp
 ${CMAKE_CURRENT_BINARY_DIR}/sample/handleholder_wrapper.cpp
 ${CMAKE_CURRENT_BINARY_DIR}/sample/implicitconv_wrapper.cpp
 ${CMAKE_CURRENT_BINARY_DIR}/sample/implicitbase_wrapper.cpp
diff --git a/tests/samplebinding/global.h b/tests/samplebinding/global.h
index f85ee69b7..d027b0f57 100644
--- a/tests/samplebinding/global.h
+++ b/tests/samplebinding/global.h
@@ -44,3 +44,4 @@
 #include "virtualmethods.h"
 #include "voidholder.h"
 #include "valueandvirtual.h"
+#include "expression.h"
diff --git a/tests/samplebinding/reference_test.py b/tests/samplebinding/reference_test.py
index e754449d2..abff658c4 100644
--- a/tests/samplebinding/reference_test.py
+++ b/tests/samplebinding/reference_test.py
@@ -64,7 +64,7 @@ class ReferenceTest(unittest.TestCase):
     def testCantSegFaultWhenReceiveNone(self):
         '''do not segfault when receiving None as argument.'''
         s = Str()
-        self.assert_(not None == s)
+        self.assertTrue(None == s)
 
     def testMethodThatReceivesConstReference(self):
         '''Test a method that receives a const reference to an object as argument.'''
diff --git a/tests/samplebinding/richcompare_test.py b/tests/samplebinding/richcompare_test.py
new file mode 100644
index 000000000..dfd20a378
--- /dev/null
+++ b/tests/samplebinding/richcompare_test.py
@@ -0,0 +1,41 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+#
+# This file is part of the Shiboken Python Bindings Generator project.
+#
+# Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+#
+# Contact: PySide team <contact@pyside.org>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public License
+# version 2.1 as published by the Free Software Foundation. Please
+# review the following information to ensure the GNU Lesser General
+# Public License version 2.1 requirements will be met:
+# http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+# #
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+# 02110-1301 USA
+
+import unittest
+
+from sample import *
+
+class TestRichCompare(unittest.TestCase):
+
+    def testIt(self):
+        a = Expression(2)
+        b = Expression(3)
+        c = a + b
+        d = a + c < b + a
+        self.assertEqual(d.toString(), "((2+(2+3))<(3+2))")
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/samplebinding/typesystem_sample.xml b/tests/samplebinding/typesystem_sample.xml
index f0a78e1ef..ba58c5f22 100644
--- a/tests/samplebinding/typesystem_sample.xml
+++ b/tests/samplebinding/typesystem_sample.xml
@@ -1255,6 +1255,8 @@
     <!-- type used in abstract method -->
     <object-type name="HideType" generate="no" />
 
+    <value-type name="Expression" />
+
     <rejection class="ListUser" function-name="createList()"/>
     <rejection class="ListUser" function-name="callCreateList()"/>
     <rejection class="ListUser" function-name="createComplexList(Complex, Complex)"/>