Fix evaluation of lhs in binary expressions

The entire left hand side of a binary expression must be evaluated
before the right hand side. In case of for example

    x !== (x = 1)

it is important to generate the code for looking up "x" from the
lhs before dealing with the rhs, because this overall expression
is supposed to throw a reference error.

Change-Id: I03aee3257ff7b7a60aa789dba9f0445c387a1ad5
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/qv4codegen.cpp b/qv4codegen.cpp
index 07d874f..b93e740 100644 (file)
--- a/qv4codegen.cpp
+++ b/qv4codegen.cpp
@@ -1120,32 +1120,33 @@ bool Codegen::visit(BinaryExpression *ast)
         return false;
     }
 
-    Result left = expression(ast->left);
+    IR::Expr* left = *expression(ast->left);
     if (_function->isStrict) {
         if (IR::Name *n = left->asName())
             if (*n->id == QLatin1String("eval") || *n->id == QLatin1String("arguments"))
             throwSyntaxError(ast->left->lastSourceLocation(), QCoreApplication::translate("qv4codegen", "Variable name may not be eval or arguments in strict mode"));
     }
-    Result right = expression(ast->right);
 
     switch (ast->op) {
     case QSOperator::Or:
     case QSOperator::And:
         break;
 
-    case QSOperator::Assign:
+    case QSOperator::Assign: {
+        IR::Expr* right = *expression(ast->right);
         if (! (left->asTemp() || left->asName() || left->asSubscript() || left->asMember()))
             throwSyntaxError(ast->operatorToken, QCoreApplication::translate("qv4codegen", "left-hand side of assignment operator is not an lvalue"));
 
         if (_expr.accept(nx)) {
-            move(*left, *right);
+            move(left, right);
         } else {
             const unsigned t = _block->newTemp();
-            move(_block->TEMP(t), *right);
-            move(*left, _block->TEMP(t));
-            _expr.code = *left;
+            move(_block->TEMP(t), right);
+            move(left, _block->TEMP(t));
+            _expr.code = left;
         }
         break;
+    }
 
     case QSOperator::InplaceAnd:
     case QSOperator::InplaceSub:
@@ -1158,16 +1159,17 @@ bool Codegen::visit(BinaryExpression *ast)
     case QSOperator::InplaceRightShift:
     case QSOperator::InplaceURightShift:
     case QSOperator::InplaceXor: {
+        IR::Expr* right = *expression(ast->right);
         if (! (left->asTemp() || left->asName() || left->asSubscript() || left->asMember()))
             throwSyntaxError(ast->operatorToken, QCoreApplication::translate("qv4codegen", "left-hand side of inplace operator is not an lvalue"));
 
         if (_expr.accept(nx)) {
-            move(*left, *right, baseOp(ast->op));
+            move(left, right, baseOp(ast->op));
         } else {
             const unsigned t = _block->newTemp();
-            move(_block->TEMP(t), *right);
-            move(*left, _block->TEMP(t), baseOp(ast->op));
-            _expr.code = *left;
+            move(_block->TEMP(t), right);
+            move(left, _block->TEMP(t), baseOp(ast->op));
+            _expr.code = left;
         }
         break;
     }
@@ -1181,11 +1183,19 @@ bool Codegen::visit(BinaryExpression *ast)
     case QSOperator::Le:
     case QSOperator::Lt:
     case QSOperator::StrictEqual:
-    case QSOperator::StrictNotEqual:
+    case QSOperator::StrictNotEqual: {
+        if (!left->asTemp() && !left->asConst()) {
+            const unsigned t = _block->newTemp();
+            move(_block->TEMP(t), left);
+            left = _block->TEMP(t);
+        }
+
+        IR::Expr* right = *expression(ast->right);
+
         if (_expr.accept(cx)) {
-            cjump(binop(IR::binaryOperator(ast->op), *left, *right), _expr.iftrue, _expr.iffalse);
+            cjump(binop(IR::binaryOperator(ast->op), left, right), _expr.iftrue, _expr.iffalse);
         } else {
-            IR::Expr *e = binop(IR::binaryOperator(ast->op), *left, *right);
+            IR::Expr *e = binop(IR::binaryOperator(ast->op), left, right);
             if (e->asConst() || e->asString())
                 _expr.code = e;
             else {
@@ -1195,6 +1205,7 @@ bool Codegen::visit(BinaryExpression *ast)
             }
         }
         break;
+    }
 
     case QSOperator::Add:
     case QSOperator::BitAnd:
@@ -1207,7 +1218,15 @@ bool Codegen::visit(BinaryExpression *ast)
     case QSOperator::RShift:
     case QSOperator::Sub:
     case QSOperator::URShift: {
-        IR::Expr *e = binop(IR::binaryOperator(ast->op), *left, *right);
+        if (!left->asTemp() && !left->asConst()) {
+            const unsigned t = _block->newTemp();
+            move(_block->TEMP(t), left);
+            left = _block->TEMP(t);
+        }
+
+        IR::Expr* right = *expression(ast->right);
+
+        IR::Expr *e = binop(IR::binaryOperator(ast->op), left, right);
         if (e->asConst() || e->asString())
             _expr.code = e;
         else {

diff --git a/tests/TestExpectations b/tests/TestExpectations
index 9efb8a8..61ab662 100644 (file)
--- a/tests/TestExpectations
+++ b/tests/TestExpectations
@@ -15,12 +15,6 @@ S15.1.3.2_A2.5_T1
 10.4.3-1-82-s failing
 11.1.4_4-5-1 failing
 11.1.4_5-6-1 failing
-S11.10.1_A2.4_T1 failing
-S11.10.1_A2.4_T3 failing
-S11.10.2_A2.4_T1 failing
-S11.10.2_A2.4_T3 failing
-S11.10.3_A2.4_T1 failing
-S11.10.3_A2.4_T3 failing
 S11.11.1_A2.1_T1 failing
 S11.11.1_A3_T2 failing
 S11.11.1_A3_T3 failing
@@ -73,52 +67,16 @@ S11.3.2_A4_T4 failing
 11.4.1-5-2 failing
 S11.4.1_A1 failing
 S11.4.1_A2.1 failing
-S11.5.1_A2.4_T1 failing
-S11.5.1_A2.4_T3 failing
 11.4.4-2-1-s failing
 11.4.4-2-2-s failing
 11.4.5-2-1-s failing
 11.4.5-2-2-s failing
-S11.5.2_A2.4_T1 failing
-S11.5.2_A2.4_T3 failing
-S11.5.3_A2.4_T1 failing
-S11.5.3_A2.4_T3 failing
 S11.5.3_A3_T1.4 failing
 S11.5.3_A3_T2.3 failing
 S11.5.3_A3_T2.9 failing
 S11.5.3_A4_T2 failing
-S11.6.1_A2.4_T1 failing
-S11.6.1_A2.4_T3 failing
-S11.6.2_A2.4_T1 failing
-S11.6.2_A2.4_T3 failing
-S11.7.1_A2.4_T1 failing
-S11.7.1_A2.4_T3 failing
-S11.7.2_A2.4_T1 failing
-S11.7.2_A2.4_T3 failing
-S11.8.2_A2.4_T1 failing
-S11.8.2_A2.4_T3 failing
-S11.8.3_A2.4_T1 failing
-S11.8.3_A2.4_T3 failing
-S11.7.3_A2.4_T1 failing
-S11.7.3_A2.4_T3 failing
-S11.8.1_A2.4_T1 failing
-S11.8.1_A2.4_T3 failing
-S11.8.4_A2.4_T1 failing
-S11.8.4_A2.4_T3 failing
-S11.8.6_A2.4_T1 failing
-S11.8.6_A2.4_T3 failing
 S11.8.6_A3 failing
 S11.8.6_A5_T2 failing
-S11.8.7_A2.4_T1 failing
-S11.8.7_A2.4_T3 failing
-S11.9.1_A2.4_T1 failing
-S11.9.1_A2.4_T3 failing
-S11.9.2_A2.4_T1 failing
-S11.9.2_A2.4_T3 failing
-S11.9.4_A2.4_T1 failing
-S11.9.4_A2.4_T3 failing
-S11.9.5_A2.4_T1 failing
-S11.9.5_A2.4_T3 failing
 12.10-0-3 failing
 S12.10_A1.11_T1 failing
 S12.10_A1.11_T2 failing
@@ -229,4 +187,4 @@ S12.9_A1_T6 failing
 S12.9_A1_T7 failing
 S12.9_A1_T8 failing
 S12.9_A1_T9 failing
-S15.2.4.4_A14 failing
+S15.2.4.4_A14 failing
\ No newline at end of file