From 20ffaaf2977255147ad8a45762d5efb6b5635829 Mon Sep 17 00:00:00 2001 From: Michael Brasser Date: Wed, 14 Mar 2012 11:35:13 +1000 Subject: [PATCH] Fix bug in v4 strict equality. Change-Id: I184065e0b7c8c6536f2081b9bf03e98992a4c9fe Reviewed-by: Roberto Raggi --- src/qml/qml/v4/qv4bindings.cpp | 45 +++++++++++++++++++++++ src/qml/qml/v4/qv4compiler.cpp | 16 ++++++++- src/qml/qml/v4/qv4instruction.cpp | 15 ++++++++ src/qml/qml/v4/qv4instruction_p.h | 11 ++++++ src/qml/qml/v4/qv4irbuilder.cpp | 28 ++++++++------- src/qml/qml/v4/qv4program_p.h | 1 + tests/auto/qml/v4/data/equals.qml | 48 +++++++++++++++++++++++++ tests/auto/qml/v4/data/strictEquals.qml | 48 +++++++++++++++++++++++++ tests/auto/qml/v4/tst_v4.cpp | 2 ++ 9 files changed, 201 insertions(+), 13 deletions(-) create mode 100644 tests/auto/qml/v4/data/equals.qml create mode 100644 tests/auto/qml/v4/data/strictEquals.qml diff --git a/src/qml/qml/v4/qv4bindings.cpp b/src/qml/qml/v4/qv4bindings.cpp index 89e831ca3f..5d367f2ae6 100644 --- a/src/qml/qml/v4/qv4bindings.cpp +++ b/src/qml/qml/v4/qv4bindings.cpp @@ -66,6 +66,7 @@ struct Register { typedef QQmlRegisterType Type; void setUndefined() { dataType = UndefinedType; } + void setNull() { dataType = NullType; } void setNaN() { setqreal(qSNaN()); } bool isUndefined() const { return dataType == UndefinedType; } @@ -1239,6 +1240,10 @@ void QV4Bindings::run(int instrIndex, quint32 &executedBlocks, } QML_V4_END_INSTR(MathPIReal, unaryop) + QML_V4_BEGIN_INSTR(LoadNull, null_value) + registers[instr->null_value.reg].setNull(); + QML_V4_END_INSTR(LoadNull, null_value) + QML_V4_BEGIN_INSTR(LoadReal, real_value) registers[instr->real_value.reg].setqreal(instr->real_value.value); QML_V4_END_INSTR(LoadReal, real_value) @@ -1521,6 +1526,46 @@ void QV4Bindings::run(int instrIndex, quint32 &executedBlocks, } QML_V4_END_INSTR(StrictNotEqualString, binaryop) + QML_V4_BEGIN_INSTR(EqualObject, binaryop) + { + const Register &left = registers[instr->binaryop.left]; + const Register &right = registers[instr->binaryop.right]; + QObject *leftobj = (left.gettype() == NullType) ? 0 : left.getQObject(); + QObject *rightobj = (right.gettype() == NullType) ? 0 : right.getQObject(); + registers[instr->binaryop.output].setbool(leftobj == rightobj); + } + QML_V4_END_INSTR(EqualObject, binaryop) + + QML_V4_BEGIN_INSTR(NotEqualObject, binaryop) + { + const Register &left = registers[instr->binaryop.left]; + const Register &right = registers[instr->binaryop.right]; + QObject *leftobj = (left.gettype() == NullType) ? 0 : left.getQObject(); + QObject *rightobj = (right.gettype() == NullType) ? 0 : right.getQObject(); + registers[instr->binaryop.output].setbool(leftobj != rightobj); + } + QML_V4_END_INSTR(NotEqualObject, binaryop) + + QML_V4_BEGIN_INSTR(StrictEqualObject, binaryop) + { + const Register &left = registers[instr->binaryop.left]; + const Register &right = registers[instr->binaryop.right]; + QObject *leftobj = (left.gettype() == NullType) ? 0 : left.getQObject(); + QObject *rightobj = (right.gettype() == NullType) ? 0 : right.getQObject(); + registers[instr->binaryop.output].setbool(leftobj == rightobj); + } + QML_V4_END_INSTR(StrictEqualObject, binaryop) + + QML_V4_BEGIN_INSTR(StrictNotEqualObject, binaryop) + { + const Register &left = registers[instr->binaryop.left]; + const Register &right = registers[instr->binaryop.right]; + QObject *leftobj = (left.gettype() == NullType) ? 0 : left.getQObject(); + QObject *rightobj = (right.gettype() == NullType) ? 0 : right.getQObject(); + registers[instr->binaryop.output].setbool(leftobj != rightobj); + } + QML_V4_END_INSTR(StrictNotEqualObject, binaryop) + QML_V4_BEGIN_INSTR(MathMaxReal, binaryop) { const Register &left = registers[instr->binaryop.left]; diff --git a/src/qml/qml/v4/qv4compiler.cpp b/src/qml/qml/v4/qv4compiler.cpp index b22708d7db..09b0f3861b 100644 --- a/src/qml/qml/v4/qv4compiler.cpp +++ b/src/qml/qml/v4/qv4compiler.cpp @@ -234,6 +234,12 @@ void QV4CompilerPrivate::visitConst(IR::Const *e) gen(i); } break; + case IR::NullType: { + Instr::LoadNull i; + i.reg = currentReg; + gen(i); + } break; + default: if (qmlVerboseCompiler()) qWarning() << Q_FUNC_INFO << "unexpected type"; @@ -682,21 +688,29 @@ quint8 QV4CompilerPrivate::instructionOpcode(IR::Binop *e) return V4Instr::LeReal; case IR::OpEqual: + if (e->left->type == IR::ObjectType || e->right->type == IR::ObjectType) + return V4Instr::EqualObject; if (e->left->type == IR::StringType) return V4Instr::EqualString; return V4Instr::EqualReal; case IR::OpNotEqual: + if (e->left->type == IR::ObjectType || e->right->type == IR::ObjectType) + return V4Instr::NotEqualObject; if (e->left->type == IR::StringType) return V4Instr::NotEqualString; return V4Instr::NotEqualReal; case IR::OpStrictEqual: + if (e->left->type == IR::ObjectType || e->right->type == IR::ObjectType) + return V4Instr::StrictEqualObject; if (e->left->type == IR::StringType) return V4Instr::StrictEqualString; return V4Instr::StrictEqualReal; case IR::OpStrictNotEqual: + if (e->left->type == IR::ObjectType || e->right->type == IR::ObjectType) + return V4Instr::StrictNotEqualObject; if (e->left->type == IR::StringType) return V4Instr::StrictNotEqualString; return V4Instr::StrictNotEqualReal; @@ -774,7 +788,7 @@ void QV4CompilerPrivate::visitBinop(IR::Binop *e) case IR::OpNotEqual: case IR::OpStrictEqual: case IR::OpStrictNotEqual: - if (e->left->type != IR::StringType) { + if (e->left->type >= IR::FirstNumberType) { convertToReal(e->left, left); convertToReal(e->right, right); } diff --git a/src/qml/qml/v4/qv4instruction.cpp b/src/qml/qml/v4/qv4instruction.cpp index e26014765d..1ed8bd245e 100644 --- a/src/qml/qml/v4/qv4instruction.cpp +++ b/src/qml/qml/v4/qv4instruction.cpp @@ -213,6 +213,9 @@ void Bytecode::dump(const V4Instr *i, int address) const case V4Instr::MathPIReal: INSTR_DUMP << "\t" << "MathPIReal" << "\t\t" << "Input_Reg(" << i->unaryop.src << ") -> Output_Reg(" << i->unaryop.output << ")"; break; + case V4Instr::LoadNull: + INSTR_DUMP << "\t" << "LoadNull" << "\t\t" << "Constant(null) -> Output_Reg(" << i->null_value.reg << ")"; + break; case V4Instr::LoadReal: INSTR_DUMP << "\t" << "LoadReal" << "\t\t" << "Constant(" << i->real_value.value << ") -> Output_Reg(" << i->real_value.reg << ")"; break; @@ -315,6 +318,18 @@ void Bytecode::dump(const V4Instr *i, int address) const case V4Instr::StrictNotEqualString: INSTR_DUMP << "\t" << "StrictNotEqualString" << "\t" << "Input_Reg(" << i->binaryop.left << ") Input_Reg(" << i->binaryop.right << ") -> Output_Reg(" << i->binaryop.output << ")"; break; + case V4Instr::EqualObject: + INSTR_DUMP << "\t" << "EqualObject" << "\t\t" << "Input_Reg(" << i->binaryop.left << ") Input_Reg(" << i->binaryop.right << ") -> Output_Reg(" << i->binaryop.output << ")"; + break; + case V4Instr::NotEqualObject: + INSTR_DUMP << "\t" << "NotEqualObject" << "\t\t" << "Input_Reg(" << i->binaryop.left << ") Input_Reg(" << i->binaryop.right << ") -> Output_Reg(" << i->binaryop.output << ")"; + break; + case V4Instr::StrictEqualObject: + INSTR_DUMP << "\t" << "StrictEqualObject" << "\t" << "Input_Reg(" << i->binaryop.left << ") Input_Reg(" << i->binaryop.right << ") -> Output_Reg(" << i->binaryop.output << ")"; + break; + case V4Instr::StrictNotEqualObject: + INSTR_DUMP << "\t" << "StrictNotEqualObject" << "\t" << "Input_Reg(" << i->binaryop.left << ") Input_Reg(" << i->binaryop.right << ") -> Output_Reg(" << i->binaryop.output << ")"; + break; case V4Instr::MathMaxReal: INSTR_DUMP << "\t" << "MathMaxReal" << "\t" << "Input_Reg(" << i->binaryop.left << ") Input_Reg(" << i->binaryop.right << ") -> Output_Reg(" << i->binaryop.output << ")"; break; diff --git a/src/qml/qml/v4/qv4instruction_p.h b/src/qml/qml/v4/qv4instruction_p.h index d33051840d..d6c790e46f 100644 --- a/src/qml/qml/v4/qv4instruction_p.h +++ b/src/qml/qml/v4/qv4instruction_p.h @@ -106,6 +106,7 @@ QT_BEGIN_NAMESPACE F(MathFloorReal, unaryop) \ F(MathCeilReal, unaryop) \ F(MathPIReal, unaryop) \ + F(LoadNull, null_value) \ F(LoadReal, real_value) \ F(LoadInt, int_value) \ F(LoadBool, bool_value) \ @@ -140,6 +141,10 @@ QT_BEGIN_NAMESPACE F(NotEqualString, binaryop) \ F(StrictEqualString, binaryop) \ F(StrictNotEqualString, binaryop) \ + F(EqualObject, binaryop) \ + F(NotEqualObject, binaryop) \ + F(StrictEqualObject, binaryop) \ + F(StrictNotEqualObject, binaryop) \ F(MathMaxReal, binaryop) \ F(MathMinReal, binaryop) \ F(NewString, construct) \ @@ -271,6 +276,11 @@ union V4Instr { qint8 reg; }; + struct instr_null_value { + QML_V4_INSTR_HEADER + qint8 reg; + }; + struct instr_real_value { QML_V4_INSTR_HEADER qint8 reg; @@ -358,6 +368,7 @@ union V4Instr { instr_fetch fetch; instr_copy copy; instr_construct construct; + instr_null_value null_value; instr_real_value real_value; instr_int_value int_value; instr_bool_value bool_value; diff --git a/src/qml/qml/v4/qv4irbuilder.cpp b/src/qml/qml/v4/qv4irbuilder.cpp index 06178259f2..481b23eb4a 100644 --- a/src/qml/qml/v4/qv4irbuilder.cpp +++ b/src/qml/qml/v4/qv4irbuilder.cpp @@ -881,13 +881,15 @@ void QV4IRBuilder::binop(AST::BinaryExpression *ast, ExprResult left, ExprResult implicitCvt(left, t); implicitCvt(right, t); } + } else if ((left.type() != IR::ObjectType && left.type() != IR::NullType) || + (right.type() != IR::ObjectType && right.type() != IR::NullType)) + return; - if (_expr.hint == ExprResult::cx) { - _expr.format = ExprResult::cx; - _block->CJUMP(_block->BINOP(IR::binaryOperator(ast->op), left, right), _expr.iftrue, _expr.iffalse); - } else { - _expr.code = _block->BINOP(IR::binaryOperator(ast->op), left, right); - } + if (_expr.hint == ExprResult::cx) { + _expr.format = ExprResult::cx; + _block->CJUMP(_block->BINOP(IR::binaryOperator(ast->op), left, right), _expr.iftrue, _expr.iffalse); + } else { + _expr.code = _block->BINOP(IR::binaryOperator(ast->op), left, right); } } @@ -999,9 +1001,6 @@ bool QV4IRBuilder::visit(AST::BinaryExpression *ast) implicitCvt(left, IR::RealType); implicitCvt(right, IR::RealType); binop(ast, left, right); - } else if (left.type() == IR::BoolType || right.type() == IR::BoolType) { - implicitCvt(left, IR::BoolType); - implicitCvt(right, IR::BoolType); } else if (left.isValid() && right.isValid()) { binop(ast, left, right); } @@ -1013,16 +1012,21 @@ bool QV4IRBuilder::visit(AST::BinaryExpression *ast) ExprResult right = expression(ast->right); if (left.type() == right.type()) { binop(ast, left, right); - } else if (left.type() >= IR::BoolType && right.type() >= IR::BoolType) { + } else if (left.type() > IR::BoolType && right.type() > IR::BoolType) { // left and right have numeric type (int or real) binop(ast, left, right); + } else if ((left.type() == IR::ObjectType && right.type() == IR::NullType) || + (right.type() == IR::ObjectType && left.type() == IR::NullType)) { + // comparing a qobject with null + binop(ast, left, right); } else if (left.isValid() && right.isValid()) { + // left and right have different types const bool isEq = ast->op == QSOperator::StrictEqual; if (_expr.hint == ExprResult::cx) { _expr.format = ExprResult::cx; - _block->JUMP(isEq ? _expr.iftrue : _expr.iffalse); + _block->JUMP(isEq ? _expr.iffalse : _expr.iftrue); } else { - _expr.code = _block->CONST(IR::BoolType, isEq ? 1 : 0); + _expr.code = _block->CONST(IR::BoolType, isEq ? 0 : 1); } } } break; diff --git a/src/qml/qml/v4/qv4program_p.h b/src/qml/qml/v4/qv4program_p.h index d23cc6192f..60e7403786 100644 --- a/src/qml/qml/v4/qv4program_p.h +++ b/src/qml/qml/v4/qv4program_p.h @@ -85,6 +85,7 @@ struct QV4Program { enum QQmlRegisterType { UndefinedType, + NullType, QObjectStarType, QRealType, IntType, diff --git a/tests/auto/qml/v4/data/equals.qml b/tests/auto/qml/v4/data/equals.qml new file mode 100644 index 0000000000..c32603cc7e --- /dev/null +++ b/tests/auto/qml/v4/data/equals.qml @@ -0,0 +1,48 @@ +import QtQuick 2.0 + +QtObject { + property QtObject myprop1: null + property QtObject myprop2: QtObject {} + property real zero: 0 + property bool falseProp: false + + property bool test1: myprop1 == false + property bool test2: myprop1 == null + property bool test3: 5 == myprop1 + property bool test4: null == myprop1 + property bool test5: myprop1 != false + property bool test6: myprop1 != null + property bool test7: 5 != myprop1 + property bool test8: null != myprop1 + + property bool test9: myprop2 == false + property bool test10: myprop2 == null + property bool test11: 5 == myprop2 + property bool test12: null == myprop2 + property bool test13: myprop2 != false + property bool test14: myprop2 != null + property bool test15: 5 != myprop2 + property bool test16: null != myprop2 + + property bool test17: myprop1 == myprop1 + property bool test18: myprop1 != myprop1 + property bool test19: myprop1 == myprop2 + property bool test20: myprop1 != myprop2 + property bool test21: myprop2 == myprop2 + property bool test22: myprop2 != myprop2 + + property bool test23: myprop1 == "hello" + property bool test24: myprop1 != "hello" + property bool test25: myprop2 == "hello" + property bool test26: myprop2 != "hello" + + property bool test27: falseProp == zero + property bool test28: falseProp != zero + property bool test29: falseProp == 1 + property bool test30: falseProp != 1 + property bool test31: true == zero + property bool test32: true != zero + property bool test33: true == 1 + property bool test34: true != 1 +} + diff --git a/tests/auto/qml/v4/data/strictEquals.qml b/tests/auto/qml/v4/data/strictEquals.qml new file mode 100644 index 0000000000..3f4d0d8b3f --- /dev/null +++ b/tests/auto/qml/v4/data/strictEquals.qml @@ -0,0 +1,48 @@ +import QtQuick 2.0 + +QtObject { + property QtObject myprop1: null + property QtObject myprop2: QtObject {} + property real zero: 0 + property bool falseProp: false + + property bool test1: myprop1 === false + property bool test2: myprop1 === null + property bool test3: 5 === myprop1 + property bool test4: null === myprop1 + property bool test5: myprop1 !== false + property bool test6: myprop1 !== null + property bool test7: 5 !== myprop1 + property bool test8: null !== myprop1 + + property bool test9: myprop2 === false + property bool test10: myprop2 === null + property bool test11: 5 === myprop2 + property bool test12: null === myprop2 + property bool test13: myprop2 !== false + property bool test14: myprop2 !== null + property bool test15: 5 !== myprop2 + property bool test16: null !== myprop2 + + property bool test17: myprop1 === myprop1 + property bool test18: myprop1 !== myprop1 + property bool test19: myprop1 === myprop2 + property bool test20: myprop1 !== myprop2 + property bool test21: myprop2 === myprop2 + property bool test22: myprop2 !== myprop2 + + property bool test23: myprop1 === "hello" + property bool test24: myprop1 !== "hello" + property bool test25: myprop2 === "hello" + property bool test26: myprop2 !== "hello" + + property bool test27: falseProp === zero + property bool test28: falseProp !== zero + property bool test29: falseProp === 1 + property bool test30: falseProp !== 1 + property bool test31: true === zero + property bool test32: true !== zero + property bool test33: true === 1 + property bool test34: true !== 1 +} + diff --git a/tests/auto/qml/v4/tst_v4.cpp b/tests/auto/qml/v4/tst_v4.cpp index ed65cd2a82..1c89617157 100644 --- a/tests/auto/qml/v4/tst_v4.cpp +++ b/tests/auto/qml/v4/tst_v4.cpp @@ -124,6 +124,8 @@ void tst_v4::qtscript_data() { QTest::addColumn("file"); + QTest::newRow("equals") << "equals.qml"; + QTest::newRow("strict equals") << "strictEquals.qml"; QTest::newRow("qreal -> int rounding") << "qrealToIntRounding.qml"; QTest::newRow("exception on fetch") << "fetchException.qml"; QTest::newRow("logical or") << "logicalOr.qml";