Functions for basic php arithmetic operators in tv_arith.h @override-unit-failures

Deletes operator overloads on variant for the same and points
bytecode.cpp at them.  Also fixes one known accidental deviation from
zend.  These functions take Cells by value because it seemed to make
sense---I'll likely convert the various functions in the new
tv_conversions.h (and tv_comparisons.h) to do the same but in another
diff.

hphp/compiler/expression/binary_op_expression.cpp

@@ -31,6 +31,7 @@
 #include "hphp/compiler/expression/simple_function_call.h"
 #include "hphp/compiler/expression/simple_variable.h"
 #include "hphp/compiler/statement/loop_statement.h"
+#include "hphp/runtime/base/tv_arith.h"
 
 using namespace HPHP;
 
@@ -549,21 +550,26 @@ ExpressionPtr BinaryOpExpression::foldConst(AnalysisResultConstPtr ar) {
         case T_IS_GREATER_OR_EQUAL:
           result = cellGreaterOrEqual(v1.asCell(), v2.asCell()); break;
         case '+':
-          result = plus(v1, v2); break;
+          *result.asCell() = cellAdd(*v1.asCell(), *v2.asCell());
+          break;
         case '-':
-          result = minus(v1, v2); break;
+          *result.asCell() = cellSub(*v1.asCell(), *v2.asCell());
+          break;
         case '*':
-          result = multiply(v1, v2); break;
+          *result.asCell() = cellMul(*v1.asCell(), *v2.asCell());
+          break;
         case '/':
           if ((v2.isIntVal() && v2.toInt64() == 0) || v2.toDouble() == 0.0) {
             return ExpressionPtr();
           }
-          result = divide(v1, v2); break;
+          *result.asCell() = cellDiv(*v1.asCell(), *v2.asCell());
+          break;
         case '%':
           if ((v2.isIntVal() && v2.toInt64() == 0) || v2.toDouble() == 0.0) {
             return ExpressionPtr();
           }
-          result = modulo(v1.toInt64(), v2.toInt64()); break;
+          *result.asCell() = cellMod(*v1.asCell(), *v2.asCell());
+          break;
         case T_SL:
           result = shift_left(v1.toInt64(), v2.toInt64()); break;
         case T_SR:

hphp/runtime/base/builtin_functions.h

@@ -94,20 +94,6 @@ bool empty(CVarRef v, CStrRef, bool = false) = delete;
 inline Variant bitwise_or (CVarRef v1, CVarRef v2) { return v1 | v2;}
 inline Variant bitwise_and(CVarRef v1, CVarRef v2) { return v1 & v2;}
 inline Variant bitwise_xor(CVarRef v1, CVarRef v2) { return v1 ^ v2;}
-inline Variant multiply(CVarRef v1, CVarRef v2)    { return v1 * v2;}
-inline Variant plus(CVarRef v1, CVarRef v2)        { return v1 + v2;}
-inline Variant minus(CVarRef v1, CVarRef v2)       { return v1 - v2;}
-inline Variant divide(CVarRef v1, CVarRef v2)      { return v1 / v2; }
-inline Variant modulo(int64_t v1, int64_t v2) {
-  if (UNLIKELY(v2 == 0)) {
-    raise_warning(Strings::DIVISION_BY_ZERO);
-    return false;
-  }
-  if (UNLIKELY(uint64_t(v2+1) <= 2u)) {
-    return 0;
-  }
-  return v1 % v2;
-}
 inline int64_t shift_left(int64_t v1, int64_t v2)        { return v1 << v2; }
 inline int64_t shift_right(int64_t v1, int64_t v2)       { return v1 >> v2; }
 

hphp/runtime/base/execution_context.h

@@ -481,6 +481,7 @@ private:
   template <unsigned mdepth>
   void setHelperPost(unsigned ndiscard, Variant& tvRef,
                      Variant& tvRef2);
+  template<class Op> void implCellBinOp(PC&, Op op);
   template<class Op> void implCellBinOpBool(PC&, Op op);
   bool cellInstanceOf(TypedValue* c, const HPHP::NamedEntity* s);
   bool initIterator(PC& pc, PC& origPc, Iter* it,

hphp/runtime/base/hphp_value.h

@@ -139,6 +139,12 @@ private:
 typedef TypedValue Cell;
 typedef TypedValue Var;
 
+/*
+ * A TypedNum is a TypedValue that is either KindOfDouble or
+ * KindOfInt64.
+ */
+typedef TypedValue TypedNum;
+
 //////////////////////////////////////////////////////////////////////
 
 /*

hphp/runtime/base/tv_arith.cpp

@@ -0,0 +1,184 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010-2013 Facebook, Inc. (http://www.facebook.com)     |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#include "hphp/runtime/base/tv_arith.h"
+
+#include <type_traits>
+
+#include "hphp/runtime/base/tv_conversions.h"
+
+namespace HPHP {
+
+//////////////////////////////////////////////////////////////////////
+
+namespace {
+
+// Helper for converting String, Array, Bool, Null or Obj to Dbl|Int.
+// Other types must be handled outside of this.
+TypedNum numericConvHelper(Cell cell) {
+  assert(cellIsPlausible(&cell));
+
+  switch (cell.m_type) {
+  case KindOfString:
+  case KindOfStaticString: return stringToNumeric(cell.m_data.pstr);
+  case KindOfBoolean:      return make_tv<KindOfInt64>(cell.m_data.num);
+  case KindOfUninit:
+  case KindOfNull:         return make_tv<KindOfInt64>(0);
+  case KindOfObject:       return make_tv<KindOfInt64>(
+                             cell.m_data.pobj->o_toInt64());
+  case KindOfArray:        throw BadArrayOperandException();
+  default:                 break;
+  }
+  not_reached();
+}
+
+template<class Op>
+Cell cellArith(Op o, Cell c1, Cell c2) {
+  for (;;) {
+    if (c1.m_type == KindOfInt64) {
+      for (;;) {
+        if (c2.m_type == KindOfInt64)  return o(c1.m_data.num, c2.m_data.num);
+        if (c2.m_type == KindOfDouble) return o(c1.m_data.num, c2.m_data.dbl);
+        c2 = numericConvHelper(c2);
+        assert(c1.m_type == KindOfInt64 || c1.m_type == KindOfDouble);
+      }
+    }
+
+    if (c1.m_type == KindOfDouble) {
+      for (;;) {
+        if (c2.m_type == KindOfDouble) return o(c1.m_data.dbl, c2.m_data.dbl);
+        if (c2.m_type == KindOfInt64)  return o(c1.m_data.dbl, c2.m_data.num);
+        c2 = numericConvHelper(c2);
+        assert(c1.m_type == KindOfInt64 || c1.m_type == KindOfDouble);
+      }
+    }
+
+    if (c1.m_type == KindOfArray && c2.m_type == KindOfArray) {
+      return make_tv<KindOfArray>(o(c1.m_data.parr, c2.m_data.parr));
+    }
+
+    c1 = numericConvHelper(c1);
+    assert(c1.m_type == KindOfInt64 || c1.m_type == KindOfDouble);
+  }
+}
+
+Cell num(int64_t n) { return make_tv<KindOfInt64>(n); }
+Cell dbl(double d)  { return make_tv<KindOfDouble>(d); }
+
+struct Add {
+  Cell operator()(double  a, int64_t b) const { return dbl(a + b); }
+  Cell operator()(double  a, double  b) const { return dbl(a + b); }
+  Cell operator()(int64_t a, double  b) const { return dbl(a + b); }
+  Cell operator()(int64_t a, int64_t b) const { return num(a + b); }
+
+  ArrayData* operator()(ArrayData* a1, ArrayData* a2) const {
+    auto const newArr = a1->plus(a2, true /* copy */);
+    newArr->incRefCount();
+    return newArr;
+  }
+};
+
+struct Sub {
+  Cell operator()(double  a, int64_t b) const { return dbl(a - b); }
+  Cell operator()(double  a, double  b) const { return dbl(a - b); }
+  Cell operator()(int64_t a, double  b) const { return dbl(a - b); }
+  Cell operator()(int64_t a, int64_t b) const { return num(a - b); }
+
+  ArrayData* operator()(ArrayData* a1, ArrayData* a2) const {
+    throw BadArrayOperandException();
+  }
+};
+
+struct Mul {
+  Cell operator()(double  a, int64_t b) const { return dbl(a * b); }
+  Cell operator()(double  a, double  b) const { return dbl(a * b); }
+  Cell operator()(int64_t a, double  b) const { return dbl(a * b); }
+  Cell operator()(int64_t a, int64_t b) const { return num(a * b); }
+
+  ArrayData* operator()(ArrayData* a1, ArrayData* a2) const {
+    throw BadArrayOperandException();
+  }
+};
+
+struct Div {
+  Cell operator()(int64_t t, int64_t u) const {
+    /*
+     * TODO(#2547526): dividing INT64_MIN by -1 crashes.
+     */
+    if (u == 0) {
+      raise_warning(Strings::DIVISION_BY_ZERO);
+      return make_tv<KindOfBoolean>(false);
+    }
+    if (t % u == 0) return make_tv<KindOfInt64>(t / u);
+    return make_tv<KindOfDouble>(static_cast<double>(t) / u);
+  }
+
+  template<class T, class U>
+  typename std::enable_if<
+    std::is_floating_point<T>::value || std::is_floating_point<U>::value,
+    Cell
+  >::type operator()(T t, U u) const {
+    static_assert(
+      !(std::is_integral<T>::value && std::is_integral<U>::value), ""
+    );
+    if (u == 0) {
+      raise_warning(Strings::DIVISION_BY_ZERO);
+      return make_tv<KindOfBoolean>(false);
+    }
+    return make_tv<KindOfDouble>(t / u);
+  }
+
+  ArrayData* operator()(ArrayData* a1, ArrayData* a2) const {
+    throw BadArrayOperandException();
+  }
+};
+
+}
+
+//////////////////////////////////////////////////////////////////////
+
+Cell cellAdd(Cell c1, Cell c2) {
+  return cellArith(Add(), c1, c2);
+}
+
+TypedNum cellSub(Cell c1, Cell c2) {
+  return cellArith(Sub(), c1, c2);
+}
+
+TypedNum cellMul(Cell c1, Cell c2) {
+  return cellArith(Mul(), c1, c2);
+}
+
+Cell cellDiv(Cell c1, Cell c2) {
+  return cellArith(Div(), c1, c2);
+}
+
+Cell cellMod(Cell c1, Cell c2) {
+  auto const i1 = cellToInt(&c1);
+  auto const i2 = cellToInt(&c2);
+  if (i2 == 0) {
+    raise_warning(Strings::DIVISION_BY_ZERO);
+    return make_tv<KindOfBoolean>(false);
+  }
+
+  // This is to avoid SIGFPE in the case of INT64_MIN % -1.
+  if (i2 == -1) return make_tv<KindOfInt64>(0);
+
+  return make_tv<KindOfInt64>(i1 % i2);
+}
+
+//////////////////////////////////////////////////////////////////////
+
+}

hphp/runtime/base/tv_arith.h

@@ -0,0 +1,64 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010-2013 Facebook, Inc. (http://www.facebook.com)     |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#ifndef incl_HPHP_RUNTIME_BASE_TV_ARITH_H_
+#define incl_HPHP_RUNTIME_BASE_TV_ARITH_H_
+
+#include "hphp/runtime/base/complex_types.h"
+
+namespace HPHP {
+
+//////////////////////////////////////////////////////////////////////
+
+/*
+ * Functions that implement php arithmetic.
+ *
+ * These functions return Cells by value.  In cases where they may
+ * return reference counted types, the value is already incRef'd when
+ * returned.
+ */
+
+//////////////////////////////////////////////////////////////////////
+
+/*
+ * Operator +.
+ *
+ * Returns a TypedNum, unless both arguments are KindOfArray, in which
+ * case it returns an Cell that contains an Array.
+ */
+Cell cellAdd(Cell, Cell);
+
+/*
+ * Operators - and *.
+ *
+ * These arithmetic operators on any php value only return numbers.
+ */
+TypedNum cellSub(Cell, Cell);
+TypedNum cellMul(Cell, Cell);
+
+/*
+ * Operators / and %.
+ *
+ * The operators return numbers unless the second argument converts to
+ * zero, in which case they return boolean false.
+ */
+Cell cellDiv(Cell, Cell);
+Cell cellMod(Cell, Cell);
+
+//////////////////////////////////////////////////////////////////////
+
+}
+
+#endif

hphp/runtime/base/tv_comparisons.cpp

@@ -17,6 +17,7 @@
 
 #include <type_traits>
 
+#include "hphp/runtime/base/tv_conversions.h"
 #include "hphp/runtime/base/comparisons.h"
 #include "hphp/runtime/base/type_conversions.h"
 

hphp/runtime/base/tv_conversions-inl.h

@@ -14,13 +14,14 @@
    +----------------------------------------------------------------------+
 */
 
+#include "hphp/runtime/base/type_conversions.h" // toInt64(double)
+
 namespace HPHP {
 
 //////////////////////////////////////////////////////////////////////
 
 inline bool cellToBool(const Cell* cell) {
-  assert(tvIsPlausible(cell));
-  assert(cell->m_type != KindOfRef);
+  assert(cellIsPlausible(cell));
 
   switch (cell->m_type) {
   case KindOfUninit:
@@ -38,6 +39,33 @@ inline bool cellToBool(const Cell* cell) {
   not_reached();
 }
 
+inline int64_t cellToInt(const Cell* cell) {
+  assert(cellIsPlausible(cell));
+
+  switch (cell->m_type) {
+  case KindOfInt64:        return cell->m_data.num;
+  case KindOfDouble:       return toInt64(cell->m_data.dbl);
+  case KindOfString:
+  case KindOfStaticString: return cell->m_data.pstr->toInt64(10);
+  case KindOfArray:        return cell->m_data.parr->empty() ? 0 : 1;
+  case KindOfObject:       return cell->m_data.pobj->o_toInt64();
+  case KindOfBoolean:      return cell->m_data.num;
+  case KindOfUninit:
+  case KindOfNull:         return 0;
+  default:                 break;
+  }
+  not_reached();
+}
+
+inline TypedNum stringToNumeric(const StringData* sd) {
+  int64_t ival;
+  double dval;
+  auto const dt = sd->isNumericWithVal(ival, dval, true /* allow_errors */);
+  return dt == KindOfInt64 ? make_tv<KindOfInt64>(ival) :
+         dt == KindOfDouble ? make_tv<KindOfDouble>(dval) :
+         make_tv<KindOfInt64>(0);
+}
+
 //////////////////////////////////////////////////////////////////////
 
 }

hphp/runtime/base/tv_conversions.h

@@ -0,0 +1,44 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010-2013 Facebook, Inc. (http://www.facebook.com)     |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#ifndef incl_HPHP_RUNTIME_BASE_TV_CONVERSIONS_H_
+#define incl_HPHP_RUNTIME_BASE_TV_CONVERSIONS_H_
+
+#include "hphp/runtime/base/complex_types.h"
+
+namespace HPHP {
+
+//////////////////////////////////////////////////////////////////////
+
+/*
+ * Convert a cell to various types, without changing the Cell.
+ */
+bool cellToBool(const Cell*);
+int64_t cellToInt(const Cell*);
+
+/*
+ * Convert a string to a TypedNum following php semantics, allowing
+ * strings that have only a partial number in them.  (I.e. the string
+ * may have junk after the number.)
+ */
+TypedNum stringToNumeric(const StringData*);
+
+//////////////////////////////////////////////////////////////////////
+
+}
+
+#include "hphp/runtime/base/tv_conversions-inl.h"
+
+#endif

hphp/runtime/base/tv_helpers.h

@@ -459,11 +459,6 @@ inline bool tvIsString(const TypedValue* tv) {
 
 void tvUnboxIfNeeded(TypedValue* tv);
 
-/*
- * Convert a cell to a boolean, without changing the Cell.
- */
-bool cellToBool(const Cell*);
-
 /*
  * TypedValue conversions that update the tv in place (decrefing and
  * old value, if necessary).
@@ -492,6 +487,4 @@ extern const RawDestructor g_destructors[kDestrTableSize];
 ///////////////////////////////////////////////////////////////////////////////
 }
 
-#include "hphp/runtime/base/tv_helpers-inl.h"
-
 #endif // incl_HPHP_TV_HELPERS_H_

hphp/runtime/base/type_variant.cpp

@@ -29,6 +29,7 @@
 #include "hphp/runtime/vm/instance.h"
 #include "hphp/system/systemlib.h"
 #include "hphp/runtime/ext/ext_collections.h"
+#include "hphp/runtime/base/tv_arith.h"
 #include "hphp/util/util.h"
 #include "hphp/util/logger.h"
 
@@ -636,43 +637,6 @@ Variant Variant::operator+() const {
 ///////////////////////////////////////////////////////////////////////////////
 // add or array append
 
-Variant operator+(const Variant & lhs, const Variant & rhs) {
-  // Frequent case: two straight integers
-  if (lhs.m_type == KindOfInt64 && rhs.m_type == KindOfInt64) {
-    return lhs.m_data.num + rhs.m_data.num;
-  }
-  // Frequent case: two straight doubles
-  if (lhs.m_type == KindOfDouble && rhs.m_type == KindOfDouble) {
-    return lhs.m_data.dbl + rhs.m_data.dbl;
-  }
-  // Less frequent cases involving references etc.
-  if (lhs.isIntVal() && rhs.isIntVal()) {
-    return lhs.toInt64() + rhs.toInt64();
-  }
-  if (lhs.isDouble() || rhs.isDouble()) {
-    return lhs.toDouble() + rhs.toDouble();
-  }
-  if (lhs.isString()) {
-    int64_t lval; double dval;
-    if (lhs.convertToNumeric(&lval, &dval) == KindOfDouble) {
-      return dval + rhs.toDouble();
-    }
-  }
-  if (rhs.isString()) {
-    int64_t lval; double dval;
-    if (rhs.convertToNumeric(&lval, &dval) == KindOfDouble) {
-      return lhs.toDouble() + dval;
-    }
-  }
-  int na = lhs.is(KindOfArray) + rhs.is(KindOfArray);
-  if (na == 2) {
-    return lhs.toCArrRef() + rhs.toCArrRef();
-  } else if (na) {
-    throw BadArrayMergeException();
-  }
-  return lhs.toInt64() + rhs.toInt64();
-}
-
 Variant &Variant::operator+=(CVarRef var) {
   if (m_type == KindOfInt64 && var.m_type == KindOfInt64) {
     m_data.num += var.m_data.num;
@@ -790,33 +754,6 @@ Variant Variant::operator-() const {
   return *this;
 }
 
-Variant Variant::operator-(CVarRef var) const {
-  if (is(KindOfArray) || var.is(KindOfArray)) {
-    throw BadArrayOperandException();
-  }
-  if (isDouble() || var.isDouble()) {
-    return toDouble() - var.toDouble();
-  }
-  if (isIntVal() && var.isIntVal()) {
-    return toInt64() - var.toInt64();
-  }
-  if (isString()) {
-    int64_t lval; double dval;
-    DataType ret = convertToNumeric(&lval, &dval);
-    if (ret == KindOfDouble) {
-      return dval - var.toDouble();
-    }
-  }
-  if (var.isString()) {
-    int64_t lval; double dval;
-    DataType ret = var.convertToNumeric(&lval, &dval);
-    if (ret == KindOfDouble) {
-      return toDouble() - dval;
-    }
-  }
-  return toInt64() - var.toInt64();
-}
-
 Variant &Variant::operator-=(CVarRef var) {
   if (is(KindOfArray) || var.is(KindOfArray)) {
     throw BadArrayOperandException();
@@ -882,33 +819,6 @@ Variant &Variant::operator-=(double n) {
 ///////////////////////////////////////////////////////////////////////////////
 // multiply
 
-Variant Variant::operator*(CVarRef var) const {
-  if (is(KindOfArray) || var.is(KindOfArray)) {
-    throw BadArrayOperandException();
-  }
-  if (isDouble() || var.isDouble()) {
-    return toDouble() * var.toDouble();
-  }
-  if (isIntVal() && var.isIntVal()) {
-    return toInt64() * var.toInt64();
-  }
-  if (isString()) {
-    int64_t lval; double dval;
-    DataType ret = convertToNumeric(&lval, &dval);
-    if (ret == KindOfDouble) {
-      return dval * var.toDouble();
-    }
-  }
-  if (var.isString()) {
-    int64_t lval; double dval;
-    DataType ret = var.convertToNumeric(&lval, &dval);
-    if (ret == KindOfDouble) {
-      return toDouble() * dval;
-    }
-  }
-  return toInt64() * var.toInt64();
-}
-
 Variant &Variant::operator*=(CVarRef var) {
   if (is(KindOfArray) || var.is(KindOfArray)) {
     throw BadArrayOperandException();
@@ -974,64 +884,6 @@ Variant &Variant::operator*=(double n) {
 ///////////////////////////////////////////////////////////////////////////////
 // divide
 
-Variant Variant::operator/(CVarRef var) const {
-  if (is(KindOfArray) || var.is(KindOfArray)) {
-    throw BadArrayOperandException();
-  }
-  int64_t lval; double dval; bool int1 = true;
-  int64_t lval2; double dval2; bool int2 = true;
-
-  if (isDouble()) {
-    dval = toDouble();
-    int1 = false;
-  } else if (isIntVal()) {
-    lval = toInt64();
-  } else if (isString()) {
-    DataType ret = convertToNumeric(&lval, &dval);
-    if (ret == KindOfDouble) {
-      int1 = false;
-    } else if (ret != KindOfInt64) {
-      lval = 0;
-    }
-  } else {
-    assert(false);
-  }
-  if (var.isDouble()) {
-    dval2 = var.toDouble();
-    int2 = false;
-  } else if (var.isIntVal()) {
-    lval2 = var.toInt64();
-  } else if (var.isString()) {
-    DataType ret = var.convertToNumeric(&lval2, &dval2);
-    if (ret == KindOfDouble) {
-      int2 = false;
-    } else if (ret != KindOfInt64) {
-      lval2 = 0;
-    }
-  } else {
-    assert(false);
-  }
-
-  if ((int2 && lval2 == 0) || (!int2 && dval2 == 0)) {
-    raise_warning(Strings::DIVISION_BY_ZERO);
-    return false;
-  }
-
-  if (int1 && int2) {
-    if (lval % lval2 == 0) {
-      return lval / lval2;
-    } else {
-      return (double)lval / lval2;
-    }
-  } else if (int1 && !int2) {
-    return lval / dval2;
-  } else if (!int1 && int2) {
-    return dval / lval2;
-  } else {
-    return dval / dval2;
-  }
-}
-
 Variant &Variant::operator/=(CVarRef var) {
   if (is(KindOfArray) || var.is(KindOfArray)) {
     throw BadArrayOperandException();
@@ -1136,16 +988,6 @@ Variant &Variant::operator/=(double n) {
 ///////////////////////////////////////////////////////////////////////////////
 // modulus
 
-int64_t Variant::operator%(CVarRef var) const {
-  int64_t lval = toInt64();
-  int64_t lval2 = var.toInt64();
-  if (lval2 == 0) {
-    raise_warning(Strings::DIVISION_BY_ZERO);
-    return false;
-  }
-  return lval % lval2;
-}
-
 Variant &Variant::operator%=(CVarRef var) {
   int64_t lval = toInt64();
   int64_t lval2 = var.toInt64();

hphp/runtime/base/type_variant.h

@@ -541,7 +541,6 @@ class Variant : private TypedValue {
 
   Variant  operator +  () const;
   Variant unary_plus() const { return Variant(*this).operator+();}
-  friend Variant  operator +  (const Variant & lhs, const Variant & rhs);
   Variant &operator += (CVarRef v);
   Variant &operator += (int     n) { return operator+=((int64_t)n);}
   Variant &operator += (int64_t   n);
@@ -549,25 +548,25 @@ class Variant : private TypedValue {
 
   Variant negate() const { return Variant(*this).operator-();}
   Variant  operator -  () const;
-  Variant  operator -  (CVarRef v) const;
+  Variant  operator -  (CVarRef v) const = delete;
   Variant &operator -= (CVarRef v);
   Variant &operator -= (int     n) { return operator-=((int64_t)n);}
   Variant &operator -= (int64_t   n);
   Variant &operator -= (double  n);
 
-  Variant  operator *  (CVarRef v) const;
+  Variant  operator *  (CVarRef v) const = delete;
   Variant &operator *= (CVarRef v);
   Variant &operator *= (int     n) { return operator*=((int64_t)n);}
   Variant &operator *= (int64_t   n);
   Variant &operator *= (double  n);
 
-  Variant  operator /  (CVarRef v) const;
+  Variant  operator /  (CVarRef v) const = delete;
   Variant &operator /= (CVarRef v);
   Variant &operator /= (int     n) { return operator/=((int64_t)n);}
   Variant &operator /= (int64_t   n);
   Variant &operator /= (double  n);
 
-  int64_t    operator %  (CVarRef v) const;
+  int64_t    operator %  (CVarRef v) const = delete;
   Variant &operator %= (CVarRef v);
   Variant &operator %= (int     n) { return operator%=((int64_t)n);}
   Variant &operator %= (int64_t   n);
@@ -1153,6 +1152,8 @@ private:
   DataType convertToNumeric(int64_t *lval, double *dval) const;
 };
 
+Variant operator+(const Variant & lhs, const Variant & rhs) = delete;
+
 class RefResultValue {
 public:
   CVarRef get() const { return m_var; }

hphp/runtime/vm/bytecode.cpp

@@ -22,6 +22,9 @@
 
 #include "folly/String.h"
 
+#include "hphp/runtime/base/tv_comparisons.h"
+#include "hphp/runtime/base/tv_conversions.h"
+#include "hphp/runtime/base/tv_arith.h"
 #include "hphp/compiler/builtin_symbols.h"
 #include "hphp/runtime/vm/event_hook.h"
 #include "hphp/runtime/vm/jit/translator.h"
@@ -3666,91 +3669,43 @@ inline void OPTBLD_INLINE VMExecutionContext::iopConcat(PC& pc) {
   m_stack.popC();
 }
 
-#define MATHOP(OP, VOP) do {                                                  \
-  NEXT();                                                                     \
-  Cell* c1 = m_stack.topC();                                                  \
-  Cell* c2 = m_stack.indC(1);                                                 \
-  if (c2->m_type == KindOfInt64 && c1->m_type == KindOfInt64) {               \
-    int64_t a = c2->m_data.num;                                                 \
-    int64_t b = c1->m_data.num;                                                 \
-    MATHOP_DIVCHECK(0)                                                        \
-    c2->m_data.num = a OP b;                                                  \
-    m_stack.popX();                                                           \
-  }                                                                           \
-  MATHOP_DOUBLE(OP)                                                           \
-  else {                                                                      \
-    tvCellAsVariant(c2) = VOP(tvCellAsVariant(c2), tvCellAsCVarRef(c1));      \
-    m_stack.popC();                                                           \
-  }                                                                           \
-} while (0)
-
-#define MATHOP_DOUBLE(OP)                                                     \
-  else if (c2->m_type == KindOfDouble                                         \
-             && c1->m_type == KindOfDouble) {                                 \
-    double a = c2->m_data.dbl;                                                \
-    double b = c1->m_data.dbl;                                                \
-    MATHOP_DIVCHECK(0.0)                                                      \
-    c2->m_data.dbl = a OP b;                                                  \
-    m_stack.popX();                                                           \
-  }
-#define MATHOP_DIVCHECK(x)
-inline void OPTBLD_INLINE VMExecutionContext::iopAdd(PC& pc) {
-  MATHOP(+, plus);
-}
-
-inline void OPTBLD_INLINE VMExecutionContext::iopSub(PC& pc) {
-  MATHOP(-, minus);
-}
-
-inline void OPTBLD_INLINE VMExecutionContext::iopMul(PC& pc) {
-  MATHOP(*, multiply);
-}
-#undef MATHOP_DIVCHECK
-
-#define MATHOP_DIVCHECK(x)                                                    \
-    if (b == x) {                                                             \
-      raise_warning(Strings::DIVISION_BY_ZERO);                               \
-      c2->m_data.num = 0;                                                     \
-      c2->m_type = KindOfBoolean;                                             \
-    } else
-inline void OPTBLD_INLINE VMExecutionContext::iopDiv(PC& pc) {
-  NEXT();
-  Cell* c1 = m_stack.topC();  // denominator
-  Cell* c2 = m_stack.indC(1); // numerator
-  // Special handling for evenly divisible ints
-  if (c2->m_type == KindOfInt64 && c1->m_type == KindOfInt64
-      && c1->m_data.num != 0 && c2->m_data.num % c1->m_data.num == 0) {
-    int64_t b = c1->m_data.num;
-    MATHOP_DIVCHECK(0)
-    c2->m_data.num /= b;
-    m_stack.popX();
-  }
-  MATHOP_DOUBLE(/)
-  else {
-    tvCellAsVariant(c2) = divide(tvCellAsVariant(c2), tvCellAsCVarRef(c1));
-    m_stack.popC();
-  }
-}
-#undef MATHOP_DOUBLE
-
-// XXX:
-Variant moduloVar(CVarRef v1, CVarRef v2) {
-  return modulo(v1.toInt64(), v2.toInt64());
-}
-
-#define MATHOP_DOUBLE(OP)
-inline void OPTBLD_INLINE VMExecutionContext::iopMod(PC& pc) {
-  MATHOP(%, moduloVar);
-}
-#undef MATHOP_DOUBLE
-#undef MATHOP_DIVCHECK
-
 inline void OPTBLD_INLINE VMExecutionContext::iopNot(PC& pc) {
   NEXT();
   Cell* c1 = m_stack.topC();
   tvCellAsVariant(c1) = !tvCellAsVariant(c1).toBoolean();
 }
 
+template<class Op>
+void OPTBLD_INLINE VMExecutionContext::implCellBinOp(PC& pc, Op op) {
+  NEXT();
+  auto const c1 = m_stack.topC();
+  auto const c2 = m_stack.indC(1);
+  auto const result = op(*c2, *c1);
+  tvRefcountedDecRefCell(c2);
+  *c2 = result;
+  m_stack.popC();
+}
+
+inline void OPTBLD_INLINE VMExecutionContext::iopAdd(PC& pc) {
+  implCellBinOp(pc, cellAdd);
+}
+
+inline void OPTBLD_INLINE VMExecutionContext::iopSub(PC& pc) {
+  implCellBinOp(pc, cellSub);
+}
+
+inline void OPTBLD_INLINE VMExecutionContext::iopMul(PC& pc) {
+  implCellBinOp(pc, cellMul);
+}
+
+inline void OPTBLD_INLINE VMExecutionContext::iopDiv(PC& pc) {
+  implCellBinOp(pc, cellDiv);
+}
+
+inline void OPTBLD_INLINE VMExecutionContext::iopMod(PC& pc) {
+  implCellBinOp(pc, cellMod);
+}
+
 template<class Op>
 void OPTBLD_INLINE VMExecutionContext::implCellBinOpBool(PC& pc, Op op) {
   NEXT();
@@ -3810,6 +3765,24 @@ inline void OPTBLD_INLINE VMExecutionContext::iopGte(PC& pc) {
   implCellBinOpBool(pc, cellGreaterOrEqual);
 }
 
+#define MATHOP(OP, VOP) do {                                            \
+  NEXT();                                                               \
+  Cell* c1 = m_stack.topC();                                            \
+  Cell* c2 = m_stack.indC(1);                                           \
+  if (c2->m_type == KindOfInt64 && c1->m_type == KindOfInt64) {         \
+    int64_t a = c2->m_data.num;                                         \
+    int64_t b = c1->m_data.num;                                         \
+    MATHOP_DIVCHECK(0)                                                  \
+    c2->m_data.num = a OP b;                                            \
+    m_stack.popX();                                                     \
+  }                                                                     \
+  MATHOP_DOUBLE(OP)                                                     \
+  else {                                                                \
+    tvCellAsVariant(c2) = VOP(tvCellAsVariant(c2), tvCellAsCVarRef(c1)); \
+    m_stack.popC();                                                     \
+  }                                                                     \
+} while (0)
+
 #define MATHOP_DOUBLE(OP)
 #define MATHOP_DIVCHECK(x)
 inline void OPTBLD_INLINE VMExecutionContext::iopBitAnd(PC& pc) {

hphp/test/quick/string_int_arith.php

@@ -0,0 +1,29 @@
+<?php
+
+function zero() { return 0; }
+function foo() { return "0x10"; }
+function twelve() { return 12; }
+
+function main() {
+  var_dump(zero() + foo());
+  var_dump(zero() - foo());
+  var_dump(zero() / foo());
+  var_dump(zero() * foo());
+
+  var_dump(foo() + zero());
+  var_dump(foo() - zero());
+  var_dump(foo() / zero());
+  var_dump(foo() * zero());
+
+  var_dump(twelve() + foo());
+  var_dump(twelve() - foo());
+  var_dump(twelve() / foo());
+  var_dump(twelve() * foo());
+
+  var_dump(foo() + twelve());
+  var_dump(foo() - twelve());
+  var_dump(foo() / twelve());
+  var_dump(foo() * twelve());
+}
+
+main();