diff --git a/hphp/runtime/base/tv_arith.cpp b/hphp/runtime/base/tv_arith.cpp index 6ac20d0f5..97a68fd78 100644 --- a/hphp/runtime/base/tv_arith.cpp +++ b/hphp/runtime/base/tv_arith.cpp @@ -287,6 +287,108 @@ void cellBitOpEq(Op op, Cell& c1, Cell c2) { cellSet(result, c1); } +// Op must implement the interface described for cellIncDecOp. +template +void stringIncDecOp(Op op, Cell& cell) { + assert(IS_STRING_TYPE(cell.m_type)); + + auto const sd = cell.m_data.pstr; + if (sd->empty()) { + decRefStr(sd); + cellCopy(op.emptyString(), cell); + return; + } + + int64_t ival; + double dval; + auto const dt = sd->isNumericWithVal(ival, dval, true /* allow_errors */); + switch (dt) { + case KindOfInt64: + decRefStr(sd); + op(ival); + cellCopy(num(ival), cell); + break; + case KindOfDouble: + decRefStr(sd); + op(dval); + cellCopy(dbl(dval), cell); + break; + default: + assert(dt == KindOfNull); + op.nonNumericString(cell); + break; + } +} + +/* + * Inc or Dec for a string, depending on Op. Op must implement + * + * - a function call operator for numeric types + * - a nullCase(Cell&) function that returns the result for null types + * - an emptyString() function that performs the operation for empty strings + * - and a nonNumericString(Cell&) function used for non-numeric strings + * + * PHP's Inc and Dec behave differently in all these cases, so this + * abstracts out the common parts from those differences. + */ +template +void cellIncDecOp(Op op, Cell& cell) { + assert(cellIsPlausible(&cell)); + + switch (cell.m_type) { + case KindOfInt64: + op(cell.m_data.num); + break; + case KindOfDouble: + op(cell.m_data.dbl); + break; + + case KindOfString: + case KindOfStaticString: + stringIncDecOp(op, cell); + break; + + case KindOfUninit: + case KindOfNull: + op.nullCase(cell); + break; + + case KindOfBoolean: + case KindOfObject: + case KindOfArray: + break; + default: + not_reached(); + } +} + +const StaticString s_1("1"); + +struct Inc { + template void operator()(T& t) const { ++t; } + void nullCase(Cell& cell) const { cellCopy(num(1), cell); } + + Cell emptyString() const { + return make_tv(s_1.get()); + } + + void nonNumericString(Cell& cell) const { + auto const sd = cell.m_data.pstr; + auto const newSd = NEW(StringData)(sd, CopyString); + newSd->inc(); + newSd->incRefCount(); + decRefStr(sd); + cellCopy(make_tv(newSd), cell); + } +}; + +struct Dec { + template void operator()(T& t) const { --t; } + void nullCase(Cell&) const {} + Cell emptyString() const { return num(-1); } + void nonNumericString(Cell&) const {} +}; + } ////////////////////////////////////////////////////////////////////// @@ -379,6 +481,14 @@ void cellBitXorEq(Cell& c1, Cell c2) { cellBitOpEq(cellBitXor, c1, c2); } +void cellInc(Cell& cell) { + cellIncDecOp(Inc(), cell); +} + +void cellDec(Cell& cell) { + cellIncDecOp(Dec(), cell); +} + ////////////////////////////////////////////////////////////////////// } diff --git a/hphp/runtime/base/tv_arith.h b/hphp/runtime/base/tv_arith.h index 2c6d51a80..625ff36dc 100644 --- a/hphp/runtime/base/tv_arith.h +++ b/hphp/runtime/base/tv_arith.h @@ -112,6 +112,15 @@ void cellBitAndEq(Cell& c1, Cell); void cellBitOrEq(Cell& c1, Cell); void cellBitXorEq(Cell& c1, Cell); +/* + * PHP operator ++ and --. + * + * Mutates the argument in place, with the effects of php's + * pre-increment or pre-decrement operators. + */ +void cellInc(Cell&); +void cellDec(Cell&); + ////////////////////////////////////////////////////////////////////// } diff --git a/hphp/runtime/base/type_variant.cpp b/hphp/runtime/base/type_variant.cpp index 41d5fc5a6..3cf643335 100644 --- a/hphp/runtime/base/type_variant.cpp +++ b/hphp/runtime/base/type_variant.cpp @@ -541,85 +541,6 @@ Variant Variant::bitNot() const { throw InvalidOperandException("only numerics and strings can be negated"); } -/////////////////////////////////////////////////////////////////////////////// -// increment/decrement - -Variant &Variant::operator++() { - switch (getType()) { - case KindOfUninit: - case KindOfNull: set(int64_t(1)); break; - case KindOfInt64: set(toInt64() + 1); break; - case KindOfDouble: set(toDouble() + 1); break; - case KindOfStaticString: - case KindOfString: - { - if (getStringData()->empty()) { - set(s_1); - } else { - int64_t lval; double dval; - DataType ret = convertToNumeric(&lval, &dval); - switch (ret) { - case KindOfInt64: set(lval + 1); break; - case KindOfDouble: set(dval + 1); break; - case KindOfUninit: - case KindOfNull: - split(); - getStringData()->inc(); break; - default: - assert(false); - break; - } - } - } - break; - default: - break; - } - return *this; -} - -Variant Variant::operator++(int) { - Variant ret(*this); - operator++(); - return ret; -} - -Variant &Variant::operator--() { - switch (getType()) { - case KindOfInt64: set(toInt64() - 1); break; - case KindOfDouble: set(toDouble() - 1); break; - case KindOfStaticString: - case KindOfString: - { - if (getStringData()->empty()) { - set(int64_t(-1LL)); - } else { - int64_t lval; double dval; - DataType ret = convertToNumeric(&lval, &dval); - switch (ret) { - case KindOfInt64: set(lval - 1); break; - case KindOfDouble: set(dval - 1); break; - case KindOfUninit: - case KindOfNull: /* do nothing */ break; - default: - assert(false); - break; - } - } - } - break; - default: - break; - } - return *this; -} - -Variant Variant::operator--(int) { - Variant ret(*this); - operator--(); - return ret; -} - /////////////////////////////////////////////////////////////////////////////// // iterator functions diff --git a/hphp/runtime/base/type_variant.h b/hphp/runtime/base/type_variant.h index 56f912d3e..01498a98f 100644 --- a/hphp/runtime/base/type_variant.h +++ b/hphp/runtime/base/type_variant.h @@ -585,10 +585,10 @@ class Variant : private TypedValue { Variant &operator <<=(int64_t n) = delete; Variant &operator >>=(int64_t n) = delete; - Variant &operator ++ (); - Variant operator ++ (int); - Variant &operator -- (); - Variant operator -- (int); + Variant &operator ++ () = delete; + Variant operator ++ (int) = delete; + Variant &operator -- () = delete; + Variant operator -- (int) = delete; // Return the result of applying the php bitwise not operator to // this value. diff --git a/hphp/runtime/vm/member_operations.h b/hphp/runtime/vm/member_operations.h index bbf67443e..fe7d602e0 100644 --- a/hphp/runtime/vm/member_operations.h +++ b/hphp/runtime/vm/member_operations.h @@ -997,45 +997,11 @@ inline TypedValue* SetOpNewElem(TypedValue& tvScratch, TypedValue& tvRef, } template -inline void IncDecBody(unsigned char op, TypedValue* fr, - TypedValue* to) { - if (fr->m_type == KindOfInt64) { - switch ((IncDecOp)op) { - case PreInc: { - ++(fr->m_data.num); - if (setResult) { - cellDup(*fr, *to); - } - break; - } - case PostInc: { - if (setResult) { - cellDup(*fr, *to); - } - ++(fr->m_data.num); - break; - } - case PreDec: { - --(fr->m_data.num); - if (setResult) { - cellDup(*fr, *to); - } - break; - } - case PostDec: { - if (setResult) { - cellDup(*fr, *to); - } - --(fr->m_data.num); - break; - } - default: assert(false); - } - return; - } +NEVER_INLINE +void incDecBodySlow(unsigned char op, TypedValue* fr, TypedValue* to) { if (fr->m_type == KindOfUninit) { ActRec* fp = g_vmContext->m_fp; - size_t pind = ((uintptr_t(fp) - uintptr_t(fr)) / sizeof(TypedValue)) - 1; + size_t pind = reinterpret_cast(fp) - fr - 1; if (pind < size_t(fp->m_func->numNamedLocals())) { // Only raise a warning if fr points to a local variable raise_notice(Strings::UNDEFINED_VARIABLE, @@ -1044,38 +1010,77 @@ inline void IncDecBody(unsigned char op, TypedValue* fr, // Convert uninit null to null so that we don't write out an uninit null // to the eval stack for PostInc and PostDec. fr->m_type = KindOfNull; + } else { + fr = tvToCell(fr); } - switch ((IncDecOp)op) { - case PreInc: { - ++(tvAsVariant(fr)); + + assert(cellIsPlausible(fr)); + + switch (static_cast(op)) { + case PreInc: + cellInc(*fr); if (setResult) { - tvReadCell(fr, to); + cellDup(*fr, *to); } break; - } - case PostInc: { + case PostInc: if (setResult) { - tvReadCell(fr, to); + cellDup(*fr, *to); } - ++(tvAsVariant(fr)); + cellInc(*fr); break; - } - case PreDec: { - --(tvAsVariant(fr)); + case PreDec: + cellDec(*fr); if (setResult) { - tvReadCell(fr, to); + cellDup(*fr, *to); } break; - } - case PostDec: { + case PostDec: if (setResult) { - tvReadCell(fr, to); + cellDup(*fr, *to); } - --(tvAsVariant(fr)); + cellDec(*fr); + break; + case IncDec_invalid: + not_reached(); + } +} + +template +inline void IncDecBody(unsigned char op, TypedValue* fr, TypedValue* to) { + if (UNLIKELY(fr->m_type != KindOfInt64)) { + return incDecBodySlow(op, fr, to); + } + + switch (static_cast(op)) { + case PreInc: + ++fr->m_data.num; + if (setResult) { + cellCopy(*fr, *to); + } + return; + case PostInc: + if (setResult) { + cellCopy(*fr, *to); + } + ++fr->m_data.num; + return; + case PreDec: + --fr->m_data.num; + if (setResult) { + cellCopy(*fr, *to); + } + return; + case PostDec: + if (setResult) { + cellCopy(*fr, *to); + } + --fr->m_data.num; + return; + case IncDec_invalid: break; } - default: assert(false); - } + not_reached(); } template diff --git a/hphp/test/ext/test_cpp_base.cpp b/hphp/test/ext/test_cpp_base.cpp index d534a2789..d8fb93f63 100644 --- a/hphp/test/ext/test_cpp_base.cpp +++ b/hphp/test/ext/test_cpp_base.cpp @@ -344,12 +344,6 @@ bool TestCppBase::TestArray() { arr.lvalAt(name) = String("value"); VS(arr, CREATE_MAP1("name", "value")); } - { - Array arr; - arr.lvalAt(s_A) = 10; - arr.lvalAt(s_A)++; - VS(arr[s_A], 11); - } { Array arr; @@ -741,21 +735,6 @@ bool TestCppBase::TestVariant() { v1 = 20; VS(v2[1], 20); } - { - Variant v1 = 10; - Variant v2 = strongBind(v1); - v2++; - VS(v2, 11); - VS(v1, 11); - } - { - Variant arr = CREATE_VECTOR2(1, 2); - Variant v; - for (MutableArrayIter iter = arr.begin(nullptr, v); iter.advance();) { - v++; - } - VS(arr, CREATE_VECTOR2(2, 3)); - } // array escalation {