[strong] Disallow implicit conversions for add

test/mjsunit/strong/implicit-conversions.js

Index: test/mjsunit/strong/implicit-conversions.js
diff --git a/test/mjsunit/strong/implicit-conversions.js b/test/mjsunit/strong/implicit-conversions.js
index 6aa2b1dc2005e85224dc0065cfb62aee0a606369..49af153e520f56689a23b908994e49e9e4b2b69a 100644
--- a/test/mjsunit/strong/implicit-conversions.js
+++ b/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) {

[arv (Not doing code reviews), 2015/04/30 16:07:32]

Any reason for this over add_strong?

[conradw, 2015/05/04 09:28:22]

It's a kind of unfortunate way to make sure that the type feedback is starting
over from scratch (the two functions are used in different tests).

There's no way to clear type feedback on the fly for binop ICs using a natives
function. %ClearFunctionTypeFeedback wasn't doing what I thought it was.

+  "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);

[arv (Not doing code reviews), 2015/04/30 16:07:32]

Shouldn't this use a non smi?

[conradw, 2015/05/04 09:28:22]

I'm not particularly worried in this case, because of the second argument it's
going to be a miss either way.

+  %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);