[strong] Disallow implicit conversions for add

Implements the strong mode proposal's restrictions on implicit conversions
for the binary + operator. Test suite is also cleaned up/refactored to allow
easier testing of the comparison operators in the future.

BUG=v8:3956
LOG=N

Review URL: https://codereview.chromium.org/1109223004

Cr-Commit-Position: refs/heads/master@{#28159}

src/builtins.h

@@ -170,6 +170,7 @@ enum BuiltinExtraArguments {
   V(STRICT_EQUALS, 1)                      \
   V(COMPARE, 2)                            \
   V(ADD, 1)                                \
+  V(ADD_STRONG, 1)                         \
   V(SUB, 1)                                \
   V(SUB_STRONG, 1)                         \
   V(MUL, 1)                                \
@@ -203,7 +204,9 @@ enum BuiltinExtraArguments {
   V(TO_STRING, 0)                          \
   V(TO_NAME, 0)                            \
   V(STRING_ADD_LEFT, 1)                    \
+  V(STRING_ADD_LEFT_STRONG, 1)             \
   V(STRING_ADD_RIGHT, 1)                   \
+  V(STRING_ADD_RIGHT_STRONG, 1)            \
   V(APPLY_PREPARE, 1)                      \
   V(REFLECT_APPLY_PREPARE, 1)              \
   V(REFLECT_CONSTRUCT_PREPARE, 2)          \

src/code-stubs-hydrogen.cc

@@ -1187,14 +1187,15 @@ HValue* CodeStubGraphBuilder<BinaryOpICStub>::BuildCodeInitializedStub() {
                     state.op(), left, right,
                     Type::String(zone()), right_type,
                     result_type, state.fixed_right_arg(),
-                    allocation_mode));
+                    allocation_mode, state.language_mode()));
       }
       if_leftisstring.Else();
       {
         Push(BuildBinaryOperation(
                     state.op(), left, right,
                     left_type, right_type, result_type,
-                    state.fixed_right_arg(), allocation_mode));
+                    state.fixed_right_arg(), allocation_mode,
+                    state.language_mode()));
       }
       if_leftisstring.End();
       result = Pop();
@@ -1207,14 +1208,15 @@ HValue* CodeStubGraphBuilder<BinaryOpICStub>::BuildCodeInitializedStub() {
                     state.op(), left, right,
                     left_type, Type::String(zone()),
                     result_type, state.fixed_right_arg(),
-                    allocation_mode));
+                    allocation_mode, state.language_mode()));
       }
       if_rightisstring.Else();
       {
         Push(BuildBinaryOperation(
                     state.op(), left, right,
                     left_type, right_type, result_type,
-                    state.fixed_right_arg(), allocation_mode));
+                    state.fixed_right_arg(), allocation_mode,
+                    state.language_mode()));
       }
       if_rightisstring.End();
       result = Pop();
@@ -1223,7 +1225,7 @@ HValue* CodeStubGraphBuilder<BinaryOpICStub>::BuildCodeInitializedStub() {
     result = BuildBinaryOperation(
             state.op(), left, right,
             left_type, right_type, result_type,
-            state.fixed_right_arg(), allocation_mode);
+            state.fixed_right_arg(), allocation_mode, state.language_mode());
   }
 
   // If we encounter a generic argument, the number conversion is
@@ -1257,7 +1259,8 @@ HValue* CodeStubGraphBuilder<BinaryOpWithAllocationSiteStub>::BuildCodeStub() {
 
   return BuildBinaryOperation(state.op(), left, right,
                               left_type, right_type, result_type,
-                              state.fixed_right_arg(), allocation_mode);
+                              state.fixed_right_arg(), allocation_mode,
+                              state.language_mode());
 }
 
 

src/compiler/js-typed-lowering.cc

@@ -165,6 +165,8 @@ class JSBinopReduction final {
     return ChangeToPureOperator(op, false, type);
   }
 
+  bool IsStrong() { return is_strong(OpParameter<LanguageMode>(node_)); }
+
   bool OneInputIs(Type* t) { return left_type()->Is(t) || right_type()->Is(t); }
 
   bool BothInputsAre(Type* t) {
@@ -320,7 +322,7 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
     // JSAdd(x:number, y:number) => NumberAdd(x, y)
     return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number());
   }
-  if (r.NeitherInputCanBe(Type::StringOrReceiver())) {
+  if (r.NeitherInputCanBe(Type::StringOrReceiver()) && !r.IsStrong()) {
     // JSAdd(x:-string, y:-string) => NumberAdd(ToNumber(x), ToNumber(y))
     Node* frame_state = NodeProperties::GetFrameStateInput(node, 1);
     r.ConvertInputsToNumber(frame_state);
@@ -331,7 +333,8 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
   //   a) The inserted ToString operation screws up valueOf vs. toString order.
   //   b) Deoptimization at ToString doesn't have corresponding bailout id.
   //   c) Our current StringAddStub is actually non-pure and requires context.
-  if (r.OneInputIs(Type::String())) {
+  if ((r.OneInputIs(Type::String()) && !r.IsStrong()) ||
+      r.BothInputsAre(Type::String())) {
     // JSAdd(x:string, y:string) => StringAdd(x, y)
     // JSAdd(x:string, y) => StringAdd(x, ToString(y))
     // JSAdd(x, y:string) => StringAdd(ToString(x), y)
@@ -346,7 +349,7 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
 Reduction JSTypedLowering::ReduceNumberBinop(Node* node,
                                              const Operator* numberOp) {
   JSBinopReduction r(this, node);
-  if (is_strong(OpParameter<LanguageMode>(node))) {
+  if (r.IsStrong()) {
     if (r.BothInputsAre(Type::Number())) {
       return r.ChangeToPureOperator(numberOp, Type::Number());
     }
@@ -360,7 +363,7 @@ Reduction JSTypedLowering::ReduceNumberBinop(Node* node,
 
 Reduction JSTypedLowering::ReduceInt32Binop(Node* node, const Operator* intOp) {
   JSBinopReduction r(this, node);
-  if (is_strong(OpParameter<LanguageMode>(node))) {
+  if (r.IsStrong()) {
     if (r.BothInputsAre(Type::Number())) {
       r.ConvertInputsToUI32(kSigned, kSigned);
       return r.ChangeToPureOperator(intOp, Type::Integral32());
@@ -378,9 +381,7 @@ Reduction JSTypedLowering::ReduceUI32Shift(Node* node,
                                            Signedness left_signedness,
                                            const Operator* shift_op) {
   JSBinopReduction r(this, node);
-  Type* reduce_type = is_strong(
-                        OpParameter<LanguageMode>(node)) ? Type::Number() :
-                                                           Type::Primitive();
+  Type* reduce_type = r.IsStrong() ? Type::Number() : Type::Primitive();
   if (r.BothInputsAre(reduce_type)) {
     r.ConvertInputsForShift(left_signedness);
     return r.ChangeToPureOperator(shift_op, Type::Integral32());

src/hydrogen.cc

@@ -10611,23 +10611,27 @@ HValue* HGraphBuilder::BuildBinaryOperation(
     }
 
     // Convert left argument as necessary.
-    if (left_type->Is(Type::Number())) {
+    if (left_type->Is(Type::Number()) && !is_strong(language_mode)) {
       DCHECK(right_type->Is(Type::String()));
       left = BuildNumberToString(left, left_type);
     } else if (!left_type->Is(Type::String())) {
       DCHECK(right_type->Is(Type::String()));
-      HValue* function = AddLoadJSBuiltin(Builtins::STRING_ADD_RIGHT);
+      HValue* function = AddLoadJSBuiltin(is_strong(language_mode) ?
+                                            Builtins::STRING_ADD_RIGHT_STRONG :
+                                            Builtins::STRING_ADD_RIGHT);
       Add<HPushArguments>(left, right);
       return AddUncasted<HInvokeFunction>(function, 2);
     }
 
     // Convert right argument as necessary.
-    if (right_type->Is(Type::Number())) {
+    if (right_type->Is(Type::Number()) && !is_strong(language_mode)) {
       DCHECK(left_type->Is(Type::String()));
       right = BuildNumberToString(right, right_type);
     } else if (!right_type->Is(Type::String())) {
       DCHECK(left_type->Is(Type::String()));
-      HValue* function = AddLoadJSBuiltin(Builtins::STRING_ADD_LEFT);
+      HValue* function = AddLoadJSBuiltin(is_strong(language_mode) ?
+                                            Builtins::STRING_ADD_LEFT_STRONG :
+                                            Builtins::STRING_ADD_LEFT);
       Add<HPushArguments>(left, right);
       return AddUncasted<HInvokeFunction>(function, 2);
     }

src/hydrogen.h

@@ -1437,7 +1437,7 @@ class HGraphBuilder {
                                Type* result_type,
                                Maybe<int> fixed_right_arg,
                                HAllocationMode allocation_mode,
-                               LanguageMode language_mode = SLOPPY);
+                               LanguageMode language_mode);
 
   HLoadNamedField* AddLoadFixedArrayLength(HValue *object,
                                            HValue *dependency = NULL);

src/ic/ic.cc

@@ -2821,7 +2821,7 @@ Builtins::JavaScript BinaryOpIC::TokenToJSBuiltin(Token::Value op,
   if (is_strong(language_mode)) {
     switch (op) {
       default: UNREACHABLE();
-      case Token::ADD: return Builtins::ADD;
+      case Token::ADD: return Builtins::ADD_STRONG;
       case Token::SUB: return Builtins::SUB_STRONG;
       case Token::MUL: return Builtins::MUL_STRONG;
       case Token::DIV: return Builtins::DIV_STRONG;

src/runtime.js

@@ -160,6 +160,15 @@ function ADD(x) {
 }
 
 
+// Strong mode ADD throws if an implicit conversion would be performed
+function ADD_STRONG(x) {
+  if (IS_NUMBER(this) && IS_NUMBER(x)) return %NumberAdd(this, x);
+  if (IS_STRING(this) && IS_STRING(x)) return %_StringAdd(this, x);
+
+  throw %MakeTypeError('strong_implicit_cast');
+}
+
+
 // Left operand (this) is already a string.
 function STRING_ADD_LEFT(y) {
   if (!IS_STRING(y)) {
@@ -175,6 +184,15 @@ function STRING_ADD_LEFT(y) {
 }
 
 
+// Left operand (this) is already a string.
+function STRING_ADD_LEFT_STRONG(y) {
+  if (IS_STRING(y)) {
+    return %_StringAdd(this, y);
+  }
+  throw %MakeTypeError('strong_implicit_cast');
+}
+
+
 // Right operand (y) is already a string.
 function STRING_ADD_RIGHT(y) {
   var x = this;
@@ -191,6 +209,15 @@ function STRING_ADD_RIGHT(y) {
 }
 
 
+// Right operand (y) is already a string.
+function STRING_ADD_RIGHT_STRONG(y) {
+  if (IS_STRING(this)) {
+    return %_StringAdd(this, y);
+  }
+  throw %MakeTypeError('strong_implicit_cast');
+}
+
+
 // ECMA-262, section 11.6.2, page 50.
 function SUB(y) {
   var x = IS_NUMBER(this) ? this : %NonNumberToNumber(this);
@@ -199,7 +226,7 @@ function SUB(y) {
 }
 
 
-// ECMA-262, section 11.6.2, page 50.
+// Strong mode SUB throws if an implicit conversion would be performed
 function SUB_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberSub(this, y);
@@ -216,7 +243,7 @@ function MUL(y) {
 }
 
 
-// ECMA-262, section 11.5.1, page 48.
+// Strong mode MUL throws if an implicit conversion would be performed
 function MUL_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberMul(this, y);
@@ -233,7 +260,7 @@ function DIV(y) {
 }
 
 
-// ECMA-262, section 11.5.2, page 49.
+// Strong mode DIV throws if an implicit conversion would be performed
 function DIV_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberDiv(this, y);
@@ -250,7 +277,7 @@ function MOD(y) {
 }
 
 
-// ECMA-262, section 11.5.3, page 49.
+// Strong mode MOD throws if an implicit conversion would be performed
 function MOD_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberMod(this, y);
@@ -272,7 +299,7 @@ function BIT_OR(y) {
 }
 
 
-//ECMA-262, section 11.10, page 57.
+// Strong mode BIT_OR throws if an implicit conversion would be performed
 function BIT_OR_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberOr(this, y);
@@ -303,7 +330,7 @@ function BIT_AND(y) {
 }
 
 
-//ECMA-262, section 11.10, page 57.
+// Strong mode BIT_AND throws if an implicit conversion would be performed
 function BIT_AND_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberAnd(this, y);
@@ -320,7 +347,7 @@ function BIT_XOR(y) {
 }
 
 
-//ECMA-262, section 11.10, page 57.
+// Strong mode BIT_XOR throws if an implicit conversion would be performed
 function BIT_XOR_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberXor(this, y);
@@ -337,7 +364,7 @@ function SHL(y) {
 }
 
 
-//ECMA-262, section 11.7.1, page 51.
+// Strong mode SHL throws if an implicit conversion would be performed
 function SHL_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberShl(this, y);
@@ -368,7 +395,7 @@ function SAR(y) {
 }
 
 
-//ECMA-262, section 11.7.2, page 51.
+// Strong mode SAR throws if an implicit conversion would be performed
 function SAR_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberSar(this, y);
@@ -385,7 +412,7 @@ function SHR(y) {
 }
 
 
-//ECMA-262, section 11.7.3, page 52.
+// Strong mode SHR throws if an implicit conversion would be performed
 function SHR_STRONG(y) {
   if (IS_NUMBER(this) && IS_NUMBER(y)) {
     return %NumberShr(this, y);

test/mjsunit/strong/implicit-conversions.js

@@ -7,7 +7,7 @@
 "use strict";
 
 // TODO(conradw): Implement other strong operators
-let strongBinops = [
+let strongNumberBinops = [
   "-",
   "*",
   "/",
@@ -20,22 +20,37 @@ let strongBinops = [
   ">>>",
 ];
 
+let strongStringOrNumberBinops = [
+  "+"
+];
+
+let strongBinops = strongNumberBinops.concat(strongStringOrNumberBinops);
+
 let strongUnops = [
   "~",
   "+",
   "-"
 ];
 
-let nonNumberValues = [
+let nonStringOrNumberValues = [
   "{}",
-  "'foo'",
+  "false",
   "(function(){})",
   "[]",
-  "'0'",
-  "'NaN'",
   "(class Foo {})"
 ];
 
+let stringValues = [
+  "''",
+  "'               '",
+  "'foo'",
+  "'f\\u006F\\u006F'",
+  "'0'",
+  "'NaN'"
+];
+
+let nonNumberValues = nonStringOrNumberValues.concat(stringValues);
+
 let numberValues = [
   "0",
   "(-0)",
@@ -57,6 +72,16 @@ let numberValues = [
   "(-Infinity)"
 ];
 
+function add_strong(x, y) {
+  "use strong";
+  return x + y;
+}
+
+function add_num_strong(x, y) {
+  "use strong";
+  return x + y;
+}
+
 function sub_strong(x, y) {
   "use strong";
   return x - y;
@@ -107,6 +132,11 @@ function sar_strong(x, y) {
   return x >>> y;
 }
 
+function typed_add_strong(x, y) {
+  "use strong";
+  return (+x) + (+y);
+}
+
 function typed_sub_strong(x, y) {
   "use strong";
   return (+x) - (+y);
@@ -157,12 +187,17 @@ function typed_sar_strong(x, y) {
   return (+x) >>> (+y);
 }
 
-let strongFuncs = [sub_strong, mul_strong, div_strong, mod_strong, or_strong,
-                   and_strong, xor_strong, shl_strong, shr_strong, sar_strong,
-                   typed_sub_strong, typed_mul_strong, typed_div_strong,
-                   typed_mod_strong, typed_or_strong,  typed_and_strong,
-                   typed_xor_strong, typed_shl_strong, typed_shr_strong,
-                   typed_sar_strong];
+let strongNumberFuncs = [add_num_strong, sub_strong, mul_strong, div_strong,
+                         mod_strong, or_strong, and_strong, xor_strong,
+                         shl_strong, shr_strong, sar_strong, typed_add_strong,
+                         typed_sub_strong, typed_mul_strong, typed_div_strong,
+                         typed_mod_strong, typed_or_strong,  typed_and_strong,
+                         typed_xor_strong, typed_shl_strong, typed_shr_strong,
+                         typed_sar_strong];
+
+let strongStringOrNumberFuncs = [add_strong];
+
+let strongFuncs = strongNumberFuncs.concat(strongStringOrNumberFuncs);
 
 function inline_sub_strong(x, y) {
   "use strong";
@@ -197,29 +232,40 @@ function assertStrongThrowBehaviour(expr) {
   assertThrows("'use strong'; let v = " + expr + ";", TypeError);
 }
 
-for (let op of strongBinops) {
-  for (let v1 of numberValues) {
+function checkArgumentCombinations(op, leftList, rightList, willThrow) {
+  for (let v1 of leftList) {
     let assignExpr = "foo " + op + "= " + v1 + ";";
-    for (let v2 of numberValues) {
-      assertDoesNotThrow("'use strong'; let foo = " + v2 + "; " + assignExpr);
-      assertStrongNonThrowBehaviour("(" + v1 + op + v2 + ")");
-    }
-    for (let v2 of nonNumberValues) {
-      assertThrows("'use strong'; let foo = " + v2 + "; " + assignExpr,
-        TypeError);
-      assertStrongThrowBehaviour("(" + v1 + op + v2 + ")");
-    }
-  }
-  for (let v1 of nonNumberValues) {
-    let assignExpr = "foo " + op + "= " + v1 + ";";
-    for (let v2 of numberValues.concat(nonNumberValues)) {
-      assertThrows("'use strong'; let foo = " + v2 + "; " + assignExpr,
-        TypeError);
-      assertStrongThrowBehaviour("(" + v1 + op + v2 + ")");
+    for (let v2 of rightList) {
+      let compoundAssignment = "'use strong'; let foo = " + v2 + "; " +
+                               assignExpr;
+      if(willThrow) {
+        assertThrows(compoundAssignment, TypeError);
+        assertStrongThrowBehaviour("(" + v1 + op + v2 + ")");
+      } else {
+        assertDoesNotThrow(compoundAssignment);
+        assertStrongNonThrowBehaviour("(" + v1 + op + v2 + ")");
+      }
     }
   }
 }
 
+for (let op of strongBinops) {
+  checkArgumentCombinations(op, numberValues, numberValues, false);
+  checkArgumentCombinations(op, numberValues, nonNumberValues, true);
+}
+
+for (let op of strongNumberBinops) {
+  checkArgumentCombinations(op, nonNumberValues,
+                            numberValues.concat(nonNumberValues), true);
+}
+
+for (let op of strongStringOrNumberBinops) {
+  checkArgumentCombinations(op, nonNumberValues,
+                            numberValues.concat(nonStringOrNumberValues), true);
+  checkArgumentCombinations(op, nonStringOrNumberValues, stringValues, true);
+  checkArgumentCombinations(op, stringValues, stringValues, false);
+}
+
 for (let op of strongUnops) {
   for (let value of numberValues) {
     assertStrongNonThrowBehaviour("(" + op + value + ")");
@@ -229,35 +275,48 @@ for (let op of strongUnops) {
   }
 }
 
-for (let func of strongFuncs) {
-  let a = func(4, 5);
-  let b = func(4, 5);
-  assertTrue(a === b);
+for (let func of strongNumberFuncs) {
+  // Check IC None*None->None throws
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %OptimizeFunctionOnNextCall(func);
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %DeoptimizeFunction(func);
+  func(4, 5);
+  func(4, 5);
+  // Check IC Smi*Smi->Smi throws
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %OptimizeFunctionOnNextCall(func);
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %DeoptimizeFunction(func);
+  func(NaN, NaN);
+  func(NaN, NaN);
+  // Check IC Number*Number->Number throws
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %OptimizeFunctionOnNextCall(func);
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %DeoptimizeFunction(func);
+}
+
+for (let func of strongStringOrNumberFuncs) {
+  // Check IC None*None->None throws
+  assertThrows(function(){func(2, "foo");}, TypeError);
   %OptimizeFunctionOnNextCall(func);
-  let c = func(4, 5);
-  assertOptimized(func);
-  assertTrue(b === c);
+  assertThrows(function(){func(2, "foo");}, TypeError);
   %DeoptimizeFunction(func);
-  let d = func(4, 5);
-  assertTrue(c === d);
+  func("foo", "bar");
+  func("foo", "bar");
+  // Check IC String*String->String throws
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %OptimizeFunctionOnNextCall(func);
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %DeoptimizeFunction(func);
+  func(NaN, NaN);
+  func(NaN, NaN);
+  // Check IC Generic*Generic->Generic throws
+  assertThrows(function(){func(2, "foo");}, TypeError);
+  %OptimizeFunctionOnNextCall(func);
+  assertThrows(function(){func(2, "foo");}, TypeError);
   %DeoptimizeFunction(func);
-  %ClearFunctionTypeFeedback(func);
-}
-
-for (let func of strongFuncs) {
-  try {
-    let a = func(2, 3);
-    let b = func(2, 3);
-    assertTrue(a === b);
-    %OptimizeFunctionOnNextCall(func);
-    let c = func(2, "foo");
-    assertUnreachable();
-  } catch (e) {
-    assertInstanceof(e, TypeError);
-    assertUnoptimized(func);
-    assertThrows(function(){func(2, "foo");}, TypeError);
-    assertDoesNotThrow(function(){func(2, 3);});
-  }
 }
 
 assertThrows(function(){inline_outer(1, {})}, TypeError);