Fix AND expression in v4
authorLuis Gabriel Lima <luis.gabriel@openbossa.org>
Thu, 8 Mar 2012 02:24:18 +0000 (23:24 -0300)
committerQt by Nokia <qt-info@nokia.com>
Wed, 14 Mar 2012 09:26:52 +0000 (10:26 +0100)
The type of the and expressions, e.g. (a && b), were being assigned
to the type of the right hand expression (b). As reported in
QTBUG-24660, this approach could lead to some unexpected behaviors.

Now, when the left and right hand expressions are of different types,
the responsability to deal with the and expression is delegated to v8.

Task-number: QTBUG-24660
Change-Id: Ic42ebb035e62e2f197c337b2106d00453a99f04c
Reviewed-by: Roberto Raggi <roberto.raggi@nokia.com>
src/qml/qml/v4/qv4irbuilder.cpp
tests/auto/qml/v4/data/logicalAnd.2.qml [new file with mode: 0644]
tests/auto/qml/v4/data/logicalAnd.3.qml [new file with mode: 0644]
tests/auto/qml/v4/data/logicalAnd.4.qml [new file with mode: 0644]
tests/auto/qml/v4/data/logicalAnd.5.qml [new file with mode: 0644]
tests/auto/qml/v4/data/logicalAnd.6.qml [new file with mode: 0644]
tests/auto/qml/v4/data/logicalAnd.7.qml [new file with mode: 0644]
tests/auto/qml/v4/data/logicalAnd.qml [new file with mode: 0644]
tests/auto/qml/v4/data/nestedLogicalAnd.qml [new file with mode: 0644]
tests/auto/qml/v4/tst_v4.cpp

index 8f74224..0617825 100644 (file)
@@ -911,12 +911,15 @@ bool QV4IRBuilder::visit(AST::BinaryExpression *ast)
             IR::BasicBlock *iffalse = _function->newBasicBlock();
             IR::BasicBlock *endif = _function->newBasicBlock();
 
-            condition(ast->left, iftrue, iffalse);
+            ExprResult left = expression(ast->left);
+            IR::Temp *cond = _block->TEMP(IR::BoolType);
+            _block->MOVE(cond, left);
+            _block->CJUMP(cond, iftrue, iffalse);
 
             IR::Temp *r = _block->TEMP(IR::InvalidType);
 
             _block = iffalse;
-            _block->MOVE(r, _block->CONST(IR::BoolType, 0)); // ### use the right null value
+            _block->MOVE(r, cond);
             _block->JUMP(endif);
 
             _block = iftrue;
@@ -924,9 +927,12 @@ bool QV4IRBuilder::visit(AST::BinaryExpression *ast)
             _block->MOVE(r, right);
             _block->JUMP(endif);
 
+            if (left.type() != right.type())
+                discard();
+
             _block = endif;
 
-            r->type = right.type(); // ### not exactly, it can be IR::BoolType.
+            r->type = right.type();
             _expr.code = r;
         }
     } break;
diff --git a/tests/auto/qml/v4/data/logicalAnd.2.qml b/tests/auto/qml/v4/data/logicalAnd.2.qml
new file mode 100644 (file)
index 0000000..cc3d75b
--- /dev/null
@@ -0,0 +1,6 @@
+import Qt.v4 1.0
+
+Result {
+    property string s: "foo" && "bar"
+    result: s == "bar"
+}
diff --git a/tests/auto/qml/v4/data/logicalAnd.3.qml b/tests/auto/qml/v4/data/logicalAnd.3.qml
new file mode 100644 (file)
index 0000000..6527f05
--- /dev/null
@@ -0,0 +1,8 @@
+import Qt.v4 1.0
+
+Result {
+    property string s: ""
+    property bool flag: true
+
+    result: (s && flag) == ""
+}
diff --git a/tests/auto/qml/v4/data/logicalAnd.4.qml b/tests/auto/qml/v4/data/logicalAnd.4.qml
new file mode 100644 (file)
index 0000000..fbcee91
--- /dev/null
@@ -0,0 +1,8 @@
+import Qt.v4 1.0
+
+Result {
+    property string s: "foo"
+    property bool flag: true
+
+    result: (!flag && s) == false
+}
diff --git a/tests/auto/qml/v4/data/logicalAnd.5.qml b/tests/auto/qml/v4/data/logicalAnd.5.qml
new file mode 100644 (file)
index 0000000..f069846
--- /dev/null
@@ -0,0 +1,7 @@
+import Qt.v4 1.0
+
+Result {
+    property bool flag: true
+
+    result: (null && flag) == null
+}
diff --git a/tests/auto/qml/v4/data/logicalAnd.6.qml b/tests/auto/qml/v4/data/logicalAnd.6.qml
new file mode 100644 (file)
index 0000000..e98b5c9
--- /dev/null
@@ -0,0 +1,9 @@
+import Qt.v4 1.0
+
+Result {
+    property string s: ""
+    property bool flag: true
+    property string subresult: s && flag
+
+    result: subresult === ""
+}
diff --git a/tests/auto/qml/v4/data/logicalAnd.7.qml b/tests/auto/qml/v4/data/logicalAnd.7.qml
new file mode 100644 (file)
index 0000000..0f19d3f
--- /dev/null
@@ -0,0 +1,9 @@
+import Qt.v4 1.0
+
+Result {
+    property real nan: Number.NaN
+    property bool flag: true
+    property real subresult: nan && flag
+
+    result: isNaN(subresult)
+}
diff --git a/tests/auto/qml/v4/data/logicalAnd.qml b/tests/auto/qml/v4/data/logicalAnd.qml
new file mode 100644 (file)
index 0000000..40fc361
--- /dev/null
@@ -0,0 +1,6 @@
+import Qt.v4 1.0
+
+Result {
+    property int a: 10
+    result: a == 10 && a == 2
+}
diff --git a/tests/auto/qml/v4/data/nestedLogicalAnd.qml b/tests/auto/qml/v4/data/nestedLogicalAnd.qml
new file mode 100644 (file)
index 0000000..1358fce
--- /dev/null
@@ -0,0 +1,14 @@
+import Qt.v4 1.0
+
+Result {
+    property bool val1: false
+    property bool val2: true
+    property bool val3: false
+
+    property bool b1: (true && true && false)
+    property bool b2: (true && (false && true))
+    property bool b3: ((true && true) && true)
+    property bool b4: (val1 && (val2 && val3)) ? true : false
+
+    result: !b1 && !b2 && b3 && !b4
+}
index 91a3226..ed65cd2 100644 (file)
@@ -64,6 +64,8 @@ private slots:
     void unnecessaryReeval();
     void logicalOr();
     void nestedLogicalOr();
+    void logicalAnd();
+    void nestedLogicalAnd();
     void conditionalExpr();
     void qtscript();
     void qtscript_data();
@@ -208,6 +210,115 @@ void tst_v4::nestedLogicalOr()
     delete o;
 }
 
+void tst_v4::logicalAnd()
+{
+    {
+        QQmlComponent component(&engine, testFileUrl("logicalAnd.qml"));
+
+        QObject *o = component.create();
+        QVERIFY(o != 0);
+
+        ResultObject *ro = qobject_cast<ResultObject *>(o);
+        QVERIFY(ro != 0);
+
+        QCOMPARE(ro->result(), 0);
+        delete o;
+    }
+
+    {
+        QQmlComponent component(&engine, testFileUrl("logicalAnd.2.qml"));
+
+        QObject *o = component.create();
+        QVERIFY(o != 0);
+
+        ResultObject *ro = qobject_cast<ResultObject *>(o);
+        QVERIFY(ro != 0);
+
+        QCOMPARE(ro->result(), 1);
+        delete o;
+    }
+
+    {
+        QQmlComponent component(&engine, testFileUrl("logicalAnd.3.qml"));
+
+        QObject *o = component.create();
+        QVERIFY(o != 0);
+
+        ResultObject *ro = qobject_cast<ResultObject *>(o);
+        QVERIFY(ro != 0);
+
+        QCOMPARE(ro->result(), 1);
+        delete o;
+    }
+
+    {
+        // QTBUG-24660
+        QQmlComponent component(&engine, testFileUrl("logicalAnd.4.qml"));
+
+        QObject *o = component.create();
+        QVERIFY(o != 0);
+
+        ResultObject *ro = qobject_cast<ResultObject *>(o);
+        QVERIFY(ro != 0);
+
+        QCOMPARE(ro->result(), 1);
+        delete o;
+    }
+
+    {
+        QQmlComponent component(&engine, testFileUrl("logicalAnd.5.qml"));
+
+        QObject *o = component.create();
+        QVERIFY(o != 0);
+
+        ResultObject *ro = qobject_cast<ResultObject *>(o);
+        QVERIFY(ro != 0);
+
+        QCOMPARE(ro->result(), 1);
+        delete o;
+    }
+
+    {
+        QQmlComponent component(&engine, testFileUrl("logicalAnd.6.qml"));
+
+        QObject *o = component.create();
+        QVERIFY(o != 0);
+
+        ResultObject *ro = qobject_cast<ResultObject *>(o);
+        QVERIFY(ro != 0);
+
+        QCOMPARE(ro->result(), 1);
+        delete o;
+    }
+
+    {
+        QQmlComponent component(&engine, testFileUrl("logicalAnd.7.qml"));
+
+        QObject *o = component.create();
+        QVERIFY(o != 0);
+
+        ResultObject *ro = qobject_cast<ResultObject *>(o);
+        QVERIFY(ro != 0);
+
+        QCOMPARE(ro->result(), 1);
+        delete o;
+    }
+}
+
+void tst_v4::nestedLogicalAnd()
+{
+    QQmlComponent component(&engine, testFileUrl("nestedLogicalAnd.qml"));
+
+    QObject *o = component.create();
+    QVERIFY(o != 0);
+
+    ResultObject *ro = qobject_cast<ResultObject *>(o);
+    QVERIFY(ro != 0);
+
+    QCOMPARE(ro->result(), 1);
+    delete o;
+}
+
 void tst_v4::conditionalExpr()
 {
     {