Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix bug in v4 strict equality.
Change-Id: I184065e0b7c8c6536f2081b9bf03e98992a4c9fe
Reviewed-by: Roberto Raggi <roberto.raggi@nokia.com>
  • Loading branch information
Michael Brasser authored and Qt by Nokia committed Mar 15, 2012
1 parent 49b451d commit 20ffaaf
Show file tree
Hide file tree
Showing 9 changed files with 201 additions and 13 deletions.
45 changes: 45 additions & 0 deletions src/qml/qml/v4/qv4bindings.cpp
Expand Up @@ -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; }

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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];
Expand Down
16 changes: 15 additions & 1 deletion src/qml/qml/v4/qv4compiler.cpp
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
15 changes: 15 additions & 0 deletions src/qml/qml/v4/qv4instruction.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions src/qml/qml/v4/qv4instruction_p.h
Expand Up @@ -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) \
Expand Down Expand Up @@ -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) \
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
28 changes: 16 additions & 12 deletions src/qml/qml/v4/qv4irbuilder.cpp
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/qml/qml/v4/qv4program_p.h
Expand Up @@ -85,6 +85,7 @@ struct QV4Program {

enum QQmlRegisterType {
UndefinedType,
NullType,
QObjectStarType,
QRealType,
IntType,
Expand Down
48 changes: 48 additions & 0 deletions 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
}

48 changes: 48 additions & 0 deletions 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
}

2 changes: 2 additions & 0 deletions tests/auto/qml/v4/tst_v4.cpp
Expand Up @@ -124,6 +124,8 @@ void tst_v4::qtscript_data()
{
QTest::addColumn<QString>("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";
Expand Down

0 comments on commit 20ffaaf

Please sign in to comment.