From c51583cfb8fbc831f452e7ddd4485bc8f0c2db7b Mon Sep 17 00:00:00 2001 From: Jordan DeLong Date: Sun, 23 Jun 2013 18:32:24 -0700 Subject: [PATCH] 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 | 16 +- hphp/runtime/base/builtin_functions.h | 14 -- hphp/runtime/base/execution_context.h | 1 + hphp/runtime/base/hphp_value.h | 6 + hphp/runtime/base/tv_arith.cpp | 184 +++++++++++++++++++++ hphp/runtime/base/{tv_helpers-inl.h => tv_arith.h} | 59 ++++--- hphp/runtime/base/tv_comparisons.cpp | 1 + .../{tv_helpers-inl.h => tv_conversions-inl.h} | 32 +++- .../base/{tv_helpers-inl.h => tv_conversions.h} | 39 ++--- hphp/runtime/base/tv_helpers.h | 7 - hphp/runtime/base/type_variant.cpp | 160 +----------------- hphp/runtime/base/type_variant.h | 11 +- hphp/runtime/vm/bytecode.cpp | 111 +++++-------- hphp/test/quick/string_int_arith.php | 29 ++++ 14 files changed, 371 insertions(+), 299 deletions(-) create mode 100644 hphp/runtime/base/tv_arith.cpp copy hphp/runtime/base/{tv_helpers-inl.h => tv_arith.h} (54%) copy hphp/runtime/base/{tv_helpers-inl.h => tv_conversions-inl.h} (63%) rename hphp/runtime/base/{tv_helpers-inl.h => tv_conversions.h} (62%) create mode 100644 hphp/test/quick/string_int_arith.php diff --git a/hphp/compiler/expression/binary_op_expression.cpp b/hphp/compiler/expression/binary_op_expression.cpp index 3d796c286db..189870c5c92 100644 --- a/hphp/compiler/expression/binary_op_expression.cpp +++ b/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: diff --git a/hphp/runtime/base/builtin_functions.h b/hphp/runtime/base/builtin_functions.h index 9e4a51fb054..0029984b818 100644 --- a/hphp/runtime/base/builtin_functions.h +++ b/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; } diff --git a/hphp/runtime/base/execution_context.h b/hphp/runtime/base/execution_context.h index a3ff8df3356..b47ac9036ab 100644 --- a/hphp/runtime/base/execution_context.h +++ b/hphp/runtime/base/execution_context.h @@ -481,6 +481,7 @@ private: template void setHelperPost(unsigned ndiscard, Variant& tvRef, Variant& tvRef2); + template void implCellBinOp(PC&, Op op); template void implCellBinOpBool(PC&, Op op); bool cellInstanceOf(TypedValue* c, const HPHP::NamedEntity* s); bool initIterator(PC& pc, PC& origPc, Iter* it, diff --git a/hphp/runtime/base/hphp_value.h b/hphp/runtime/base/hphp_value.h index f1be2b7ebc4..fe37b4db357 100644 --- a/hphp/runtime/base/hphp_value.h +++ b/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; + ////////////////////////////////////////////////////////////////////// /* diff --git a/hphp/runtime/base/tv_arith.cpp b/hphp/runtime/base/tv_arith.cpp new file mode 100644 index 00000000000..f882c6dc992 --- /dev/null +++ b/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 + +#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(cell.m_data.num); + case KindOfUninit: + case KindOfNull: return make_tv(0); + case KindOfObject: return make_tv( + cell.m_data.pobj->o_toInt64()); + case KindOfArray: throw BadArrayOperandException(); + default: break; + } + not_reached(); +} + +template +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(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(n); } +Cell dbl(double d) { return make_tv(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(false); + } + if (t % u == 0) return make_tv(t / u); + return make_tv(static_cast(t) / u); + } + + template + typename std::enable_if< + std::is_floating_point::value || std::is_floating_point::value, + Cell + >::type operator()(T t, U u) const { + static_assert( + !(std::is_integral::value && std::is_integral::value), "" + ); + if (u == 0) { + raise_warning(Strings::DIVISION_BY_ZERO); + return make_tv(false); + } + return make_tv(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(false); + } + + // This is to avoid SIGFPE in the case of INT64_MIN % -1. + if (i2 == -1) return make_tv(0); + + return make_tv(i1 % i2); +} + +////////////////////////////////////////////////////////////////////// + +} diff --git a/hphp/runtime/base/tv_helpers-inl.h b/hphp/runtime/base/tv_arith.h similarity index 54% copy from hphp/runtime/base/tv_helpers-inl.h copy to hphp/runtime/base/tv_arith.h index 3a5e9ed73b9..730f87aa1c0 100644 --- a/hphp/runtime/base/tv_helpers-inl.h +++ b/hphp/runtime/base/tv_arith.h @@ -13,31 +13,52 @@ | 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 { ////////////////////////////////////////////////////////////////////// -inline bool cellToBool(const Cell* cell) { - assert(tvIsPlausible(cell)); - assert(cell->m_type != KindOfRef); - - switch (cell->m_type) { - case KindOfUninit: - case KindOfNull: return false; - case KindOfInt64: return cell->m_data.num != 0; - case KindOfBoolean: return cell->m_data.num; - case KindOfDouble: return cell->m_data.dbl != 0; - case KindOfStaticString: - case KindOfString: return cell->m_data.pstr->toBoolean(); - case KindOfArray: return !cell->m_data.parr->empty(); - case KindOfObject: // TODO: should handle o_toBoolean? - return true; - default: break; - } - not_reached(); -} +/* + * 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 diff --git a/hphp/runtime/base/tv_comparisons.cpp b/hphp/runtime/base/tv_comparisons.cpp index 9d16c749691..586b4e3fb70 100644 --- a/hphp/runtime/base/tv_comparisons.cpp +++ b/hphp/runtime/base/tv_comparisons.cpp @@ -17,6 +17,7 @@ #include +#include "hphp/runtime/base/tv_conversions.h" #include "hphp/runtime/base/comparisons.h" #include "hphp/runtime/base/type_conversions.h" diff --git a/hphp/runtime/base/tv_helpers-inl.h b/hphp/runtime/base/tv_conversions-inl.h similarity index 63% copy from hphp/runtime/base/tv_helpers-inl.h copy to hphp/runtime/base/tv_conversions-inl.h index 3a5e9ed73b9..aaa8571735c 100644 --- a/hphp/runtime/base/tv_helpers-inl.h +++ b/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(ival) : + dt == KindOfDouble ? make_tv(dval) : + make_tv(0); +} + ////////////////////////////////////////////////////////////////////// } diff --git a/hphp/runtime/base/tv_helpers-inl.h b/hphp/runtime/base/tv_conversions.h similarity index 62% rename from hphp/runtime/base/tv_helpers-inl.h rename to hphp/runtime/base/tv_conversions.h index 3a5e9ed73b9..ce9228d97e3 100644 --- a/hphp/runtime/base/tv_helpers-inl.h +++ b/hphp/runtime/base/tv_conversions.h @@ -13,31 +13,32 @@ | 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 { ////////////////////////////////////////////////////////////////////// -inline bool cellToBool(const Cell* cell) { - assert(tvIsPlausible(cell)); - assert(cell->m_type != KindOfRef); - - switch (cell->m_type) { - case KindOfUninit: - case KindOfNull: return false; - case KindOfInt64: return cell->m_data.num != 0; - case KindOfBoolean: return cell->m_data.num; - case KindOfDouble: return cell->m_data.dbl != 0; - case KindOfStaticString: - case KindOfString: return cell->m_data.pstr->toBoolean(); - case KindOfArray: return !cell->m_data.parr->empty(); - case KindOfObject: // TODO: should handle o_toBoolean? - return true; - default: break; - } - not_reached(); -} +/* + * 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 diff --git a/hphp/runtime/base/tv_helpers.h b/hphp/runtime/base/tv_helpers.h index 7f17412b207..2968e89ee65 100644 --- a/hphp/runtime/base/tv_helpers.h +++ b/hphp/runtime/base/tv_helpers.h @@ -460,11 +460,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_ diff --git a/hphp/runtime/base/type_variant.cpp b/hphp/runtime/base/type_variant.cpp index 3c0ce815b5e..705acfaa017 100644 --- a/hphp/runtime/base/type_variant.cpp +++ b/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(); diff --git a/hphp/runtime/base/type_variant.h b/hphp/runtime/base/type_variant.h index 5b54ade2d54..3667083e0f6 100644 --- a/hphp/runtime/base/type_variant.h +++ b/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; } diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index 6e217b8abfa..0718844e1c9 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/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,89 +3669,41 @@ 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) +inline void OPTBLD_INLINE VMExecutionContext::iopNot(PC& pc) { + NEXT(); + Cell* c1 = m_stack.topC(); + tvCellAsVariant(c1) = !tvCellAsVariant(c1).toBoolean(); +} + +template +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(); +} -#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); + implCellBinOp(pc, cellAdd); } inline void OPTBLD_INLINE VMExecutionContext::iopSub(PC& pc) { - MATHOP(-, minus); + implCellBinOp(pc, cellSub); } inline void OPTBLD_INLINE VMExecutionContext::iopMul(PC& pc) { - MATHOP(*, multiply); + implCellBinOp(pc, cellMul); } -#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()); + implCellBinOp(pc, cellDiv); } -#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(); + implCellBinOp(pc, cellMod); } template @@ -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) { diff --git a/hphp/test/quick/string_int_arith.php b/hphp/test/quick/string_int_arith.php new file mode 100644 index 00000000000..c430ae6b322 --- /dev/null +++ b/hphp/test/quick/string_int_arith.php @@ -0,0 +1,29 @@ +