Fix bug#633 - "bool of null QDate (possibly other empty QString/null QObj types?) returns True for empty instance; probably should be False"

Reviewer: Marcelo Lira <marcelo.lira@openbossa.org>
          Renato Araújo <renato.filho@openbossa.org>

7 files changed, 96 insertions, 7 deletions

diff --git a/generator/cppgenerator.cpp b/generator/cppgenerator.cpp
index f62e14f59..3eed2ffbc 100644
--- a/generator/cppgenerator.cpp
+++ b/generator/cppgenerator.cpp
@@ -190,6 +190,20 @@ void CppGenerator::writeToPythonFunction(QTextStream& s, const AbstractMetaClass
     s << "}" << endl;
 }
 
+bool CppGenerator::hasBoolCast(const AbstractMetaClass* metaClass) const
+{
+    if (!useIsNullAsNbNonZero())
+        return false;
+    // TODO: This could be configurable someday
+    const AbstractMetaFunction* func = metaClass->findFunction("isNull");
+    if (!func || !func->type() || !func->type()->typeEntry()->isPrimitive() || !func->isPublic())
+        return false;
+    const PrimitiveTypeEntry* pte = static_cast<const PrimitiveTypeEntry*>(func->type()->typeEntry());
+    while (pte->aliasedTypeEntry())
+        pte = pte->aliasedTypeEntry();
+    return func && func->isConstant() && pte->name() == "bool" && func->arguments().isEmpty();
+}
+
 /*!
     Function used to write the class generated binding code on the buffer
     \param s the output buffer
@@ -326,7 +340,8 @@ void CppGenerator::generateClass(QTextStream &s, const AbstractMetaClass *metaCl
     QTextStream smd(&singleMethodDefinitions);
 
     bool hasComparisonOperator = metaClass->hasComparisonOperatorOverload();
-    bool typeAsNumber = metaClass->hasArithmeticOperatorOverload() || metaClass->hasLogicalOperatorOverload() || metaClass->hasBitwiseOperatorOverload();
+    bool hasBoolCast = this->hasBoolCast(metaClass);
+    bool typeAsNumber = metaClass->hasArithmeticOperatorOverload() || metaClass->hasLogicalOperatorOverload() || metaClass->hasBitwiseOperatorOverload() || hasBoolCast;
 
     s << endl << "// Target ---------------------------------------------------------" << endl << endl;
     s << "extern \"C\" {" << endl;
@@ -397,6 +412,23 @@ void CppGenerator::generateClass(QTextStream &s, const AbstractMetaClass *metaCl
         s << endl;
     }
 
+    if (hasBoolCast) {
+        s << "static int " << cpythonBaseName(metaClass) << "___nb_bool(PyObject* pyObj)\n{\n";
+        s << INDENT << "if (!Shiboken::Object::isValid(pyObj))" << endl;
+        {
+            Indentation indent(INDENT);
+            s << INDENT << "return -1;" << endl;
+        }
+        s << INDENT << "const ::" << metaClass->qualifiedCppName() << "* cppSelf = ";
+        s << "Shiboken::Converter< ::" << metaClass->qualifiedCppName() << "*>::toCpp(pyObj);" << endl;
+        s << INDENT << "int result;" << endl;
+        s << INDENT << "Py_BEGIN_ALLOW_THREADS" << endl;
+        s << INDENT << "result = !cppSelf->isNull();" << endl;
+        s << INDENT << "Py_END_ALLOW_THREADS" << endl;
+        s << INDENT << "return result;" << endl;
+        s << '}' << endl << endl;
+    }
+
     if (typeAsNumber) {
         QList<AbstractMetaFunctionList> opOverloads = filterGroupedOperatorFunctions(
                 metaClass,
@@ -2380,7 +2412,8 @@ void CppGenerator::writeClassDefinition(QTextStream& s, const AbstractMetaClass*
 
     if (metaClass->hasArithmeticOperatorOverload()
         || metaClass->hasLogicalOperatorOverload()
-        || metaClass->hasBitwiseOperatorOverload()) {
+        || metaClass->hasBitwiseOperatorOverload()
+        || hasBoolCast(metaClass)) {
         tp_as_number = QString("&%1_as_number").arg(cpythonBaseName(metaClass));
     }
 
@@ -2622,6 +2655,8 @@ void CppGenerator::writeTypeAsNumberDefinition(QTextStream& s, const AbstractMet
         nb[opName] = cpythonFunctionName(rfunc);
     }
 
+    nb["bool"] = hasBoolCast(metaClass) ? cpythonBaseName(metaClass) + "___nb_bool" : "0";
+
     s << "static PyNumberMethods " << cpythonBaseName(metaClass);
     s << "_as_number = {" << endl;
     s << INDENT << "/*nb_add*/                  (binaryfunc)" << nb["__add__"] << ',' << endl;
@@ -2634,7 +2669,7 @@ void CppGenerator::writeTypeAsNumberDefinition(QTextStream& s, const AbstractMet
     s << INDENT << "/*nb_negative*/             (unaryfunc)" << nb["__neg__"] << ',' << endl;
     s << INDENT << "/*nb_positive*/             (unaryfunc)" << nb["__pos__"] << ',' << endl;
     s << INDENT << "/*nb_absolute*/             0," << endl;
-    s << INDENT << "/*nb_nonzero*/              0," << endl;
+    s << INDENT << "/*nb_nonzero*/              " << nb["bool"] << ',' << endl;
     s << INDENT << "/*nb_invert*/               (unaryfunc)" << nb["__invert__"] << ',' << endl;
     s << INDENT << "/*nb_lshift*/               (binaryfunc)" << nb["__lshift__"] << ',' << endl;
     s << INDENT << "/*nb_rshift*/               (binaryfunc)" << nb["__rshift__"] << ',' << endl;
diff --git a/generator/cppgenerator.h b/generator/cppgenerator.h
index f2bc30e05..279ff8724 100644
--- a/generator/cppgenerator.h
+++ b/generator/cppgenerator.h
@@ -194,6 +194,7 @@ private:
 
     void writeRegisterType(QTextStream& s, const AbstractMetaClass* metaClass);
     void writeRegisterType(QTextStream& s, const AbstractMetaEnum* metaEnum);
+    bool hasBoolCast(const AbstractMetaClass* metaClass) const;
 
     // Maps special function names to function parameters and return types
     // used by CPython API in the sequence protocol.
diff --git a/generator/shibokengenerator.cpp b/generator/shibokengenerator.cpp
index ebf63ab8c..06e526775 100644
--- a/generator/shibokengenerator.cpp
+++ b/generator/shibokengenerator.cpp
@@ -35,6 +35,7 @@
 #define RETURN_VALUE_HEURISTIC "enable-return-value-heuristic"
 #define ENABLE_PYSIDE_EXTENSIONS "enable-pyside-extensions"
 #define DISABLE_VERBOSE_ERROR_MESSAGES "disable-verbose-error-messages"
+#define USE_ISNULL_AS_NB_NONZERO "use-isnull-as-nb_nonzero"
 
 //static void dumpFunction(AbstractMetaFunctionList lst);
 static QString baseConversionString(QString typeName);
@@ -1627,6 +1628,7 @@ QMap<QString, QString> ShibokenGenerator::options() const
     opts.insert(RETURN_VALUE_HEURISTIC, "Enable heuristics to detect parent relationship on return values (USE WITH CAUTION!)");
     opts.insert(ENABLE_PYSIDE_EXTENSIONS, "Enable PySide extensions, such as support for signal/slots, use this if you are creating a binding for a Qt-based library.");
     opts.insert(DISABLE_VERBOSE_ERROR_MESSAGES, "Disable verbose error messages. Turn the python code hard to debug but safe few kB on the generated bindings.");
+    opts.insert(USE_ISNULL_AS_NB_NONZERO, "If a class have an isNull()const method, it will be used to compute the value of boolean casts");
     return opts;
 }
 
@@ -1636,6 +1638,7 @@ bool ShibokenGenerator::doSetup(const QMap<QString, QString>& args)
     m_usePySideExtensions = args.contains(ENABLE_PYSIDE_EXTENSIONS);
     m_userReturnValueHeuristic = args.contains(RETURN_VALUE_HEURISTIC);
     m_verboseErrorMessagesDisabled = args.contains(DISABLE_VERBOSE_ERROR_MESSAGES);
+    m_useIsNullAsNbNonZero = args.contains(USE_ISNULL_AS_NB_NONZERO);
     return true;
 }
 
@@ -1654,6 +1657,11 @@ bool ShibokenGenerator::usePySideExtensions() const
     return m_usePySideExtensions;
 }
 
+bool ShibokenGenerator::useIsNullAsNbNonZero() const
+{
+    return m_useIsNullAsNbNonZero;
+}
+
 QString ShibokenGenerator::cppApiVariableName(const QString& moduleName) const
 {
     QString result = moduleName.isEmpty() ? ShibokenGenerator::packageName() : moduleName;
diff --git a/generator/shibokengenerator.h b/generator/shibokengenerator.h
index ade04662d..218bc4610 100644
--- a/generator/shibokengenerator.h
+++ b/generator/shibokengenerator.h
@@ -310,6 +310,8 @@ public:
     bool useReturnValueHeuristic() const;
     /// Returns true if the user enabled PySide extensions.
     bool usePySideExtensions() const;
+    /// Return true if the generator should use the result of isNull()const to compute boolean casts.
+    bool useIsNullAsNbNonZero() const;
     QString cppApiVariableName(const QString& moduleName = QString()) const;
     QString getTypeIndexVariableName(const TypeEntry* metaType);
     /// Returns true if the user don't want verbose error messages on the generated bindings.
@@ -369,6 +371,7 @@ private:
     bool m_userReturnValueHeuristic;
     bool m_usePySideExtensions;
     bool m_verboseErrorMessagesDisabled;
+    bool m_useIsNullAsNbNonZero;
 };
 
 
diff --git a/tests/libsample/pen.h b/tests/libsample/pen.h
index 9c97c2e36..a0c13ef50 100644
--- a/tests/libsample/pen.h
+++ b/tests/libsample/pen.h
@@ -29,9 +29,13 @@
 class LIBSAMPLE_API Color
 {
 public:
-    Color() {}
-    Color(SampleNamespace::InValue arg) {}
-    Color(unsigned int arg) {}
+    Color() : m_null(true) {}
+    Color(SampleNamespace::InValue arg) : m_null(false) {}
+    Color(unsigned int arg) : m_null(false) {}
+
+    bool isNull() const { return m_null; }
+private:
+    bool m_null;
 };
 
 class LIBSAMPLE_API Pen
diff --git a/tests/samplebinding/CMakeLists.txt b/tests/samplebinding/CMakeLists.txt
index c81209c21..abeade93f 100644
--- a/tests/samplebinding/CMakeLists.txt
+++ b/tests/samplebinding/CMakeLists.txt
@@ -88,7 +88,7 @@ ${CMAKE_CURRENT_BINARY_DIR}/sample/valueandvirtual_wrapper.cpp
 )
 
 add_custom_command(OUTPUT ${sample_SRC}
-COMMAND ${GENERATORRUNNER_BINARY} --generatorSet=${generators_BINARY_DIR}/shiboken_generator${CMAKE_RELEASE_POSTFIX}${CMAKE_DEBUG_POSTFIX} --enable-parent-ctor-heuristic
+COMMAND ${GENERATORRUNNER_BINARY} --generatorSet=${generators_BINARY_DIR}/shiboken_generator${CMAKE_RELEASE_POSTFIX}${CMAKE_DEBUG_POSTFIX} --enable-parent-ctor-heuristic --use-isnull-as-nb_nonzero
         ${CMAKE_CURRENT_SOURCE_DIR}/global.h
         --include-paths=${libsample_SOURCE_DIR}
         --typesystem-paths=${CMAKE_CURRENT_SOURCE_DIR}
diff --git a/tests/samplebinding/nonzero_test.py b/tests/samplebinding/nonzero_test.py
new file mode 100644
index 000000000..eb319f030
--- /dev/null
+++ b/tests/samplebinding/nonzero_test.py
@@ -0,0 +1,38 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+#
+# This file is part of the Shiboken Python Bindings Generator project.
+#
+# Copyright (C) 2011 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 TestNonZeroOperator(unittest.TestCase):
+    def testIt(self):
+        c = Color()
+        self.assertFalse(c)
+        c = Color(2)
+        self.assertTrue(c)
+
+if __name__ == "__main__":
+    unittest.main()