diff --git a/hphp/runtime/base/array/array_data.cpp b/hphp/runtime/base/array/array_data.cpp index ca9a251df..47c7b356e 100644 --- a/hphp/runtime/base/array/array_data.cpp +++ b/hphp/runtime/base/array/array_data.cpp @@ -253,7 +253,7 @@ void ArrayData::newFullPos(FullPos &fp) { fp.setContainer(this); fp.setNext(strongIterators()); setStrongIterators(&fp); - getFullPos(fp); + fp.m_pos = m_pos; } void ArrayData::freeFullPos(FullPos &fp) { @@ -282,16 +282,10 @@ bool ArrayData::validFullPos(const FullPos& fp) const { return false; } -void ArrayData::getFullPos(FullPos &fp) { - assert(fp.getContainer() == (ArrayData*)this); - fp.m_pos = ArrayData::invalid_index; -} - -bool ArrayData::setFullPos(const FullPos &fp) { - assert(fp.getContainer() == (ArrayData*)this); +bool ArrayData::advanceFullPos(FullPos& fp) { return false; } - + void ArrayData::freeStrongIterators() { for (FullPosRange r(strongIterators()); !r.empty(); r.popFront()) { r.front()->setContainer(nullptr); diff --git a/hphp/runtime/base/array/array_data.h b/hphp/runtime/base/array/array_data.h index a9c48863f..4f0994f22 100644 --- a/hphp/runtime/base/array/array_data.h +++ b/hphp/runtime/base/array/array_data.h @@ -355,18 +355,12 @@ class ArrayData : public Countable { virtual bool validFullPos(const FullPos& fp) const; /** - * Sets the specified mutable iterator to point to whatever element the - * array's internal cursor currently points to. + * Advances the mutable iterator to the next element in the array. Returns + * false if the iterator has moved past the last element, otherwise returns + * true. */ - virtual void getFullPos(FullPos &fp); - - /** - * Sets the array's internal cursor to point to whetever element the - * specified mutable iterator to currently points to. It is the caller's - * responsibility to ensure that fp.getResetFlag() is false. - */ - virtual bool setFullPos(const FullPos &fp); - + virtual bool advanceFullPos(FullPos& fp); + virtual CVarRef currentRef(); virtual CVarRef endRef(); diff --git a/hphp/runtime/base/array/array_iterator.cpp b/hphp/runtime/base/array/array_iterator.cpp index d0f48eb5e..ed3f8ace0 100644 --- a/hphp/runtime/base/array/array_iterator.cpp +++ b/hphp/runtime/base/array/array_iterator.cpp @@ -85,8 +85,7 @@ void ArrayIter::objInit(ObjectData *obj) { } switch (getCollectionType()) { case Collection::VectorType: { - c_Vector* vec = getVector(); - m_version = vec->getVersion(); + m_version = getVector()->getVersion(); m_pos = 0; break; } @@ -150,8 +149,7 @@ ArrayIter::~ArrayIter() { bool ArrayIter::endHelper() { switch (getCollectionType()) { case Collection::VectorType: { - c_Vector* vec = getVector(); - return m_pos >= vec->t_count(); + return m_pos >= getVector()->t_count(); } case Collection::MapType: { return m_pos == 0; @@ -160,8 +158,7 @@ bool ArrayIter::endHelper() { return m_pos == 0; } case Collection::PairType: { - c_Pair* pair = getPair(); - return m_pos >= pair->t_count(); + return m_pos >= getPair()->t_count(); } default: { ObjectData* obj = getIteratorObj(); @@ -343,23 +340,14 @@ bool FullPos::advance() { return false; } if (container == data) { - data = cowCheck(); - if (getResetFlag()) { - setResetFlag(false); - data->reset(); - } else { - data->setFullPos(*this); - data->next(); - } - data->getFullPos(*this); - } else { - data = reregister(); + return cowCheck()->advanceFullPos(*this); } + data = reregister(); assert(data && data == getContainer()); assert(!getResetFlag()); if (!data->validFullPos(*this)) return false; - // To match PHP-like semnatics, we need to set the internal cursor to - // point to the next element. + // To conform to PHP behavior, we need to set the internal + // cursor to point to the next element. data->next(); return true; } @@ -422,14 +410,6 @@ ArrayData* FullPos::cowCheck() { return data; } -ArrayData* FullPos::getData() const { - assert(hasVar()); - if (getVar()->is(KindOfArray)) { - return getVar()->getArrayData(); - } - return nullptr; -} - ArrayData* FullPos::reregister() { ArrayData* container = getContainer(); assert(getArray() != nullptr && container != getArray()); @@ -456,13 +436,12 @@ MutableArrayIter::MutableArrayIter(const Variant *var, Variant *key, assert(getVar()); escalateCheck(); ArrayData* data = cowCheck(); - if (data) { - data->reset(); - data->newFullPos(*this); - setResetFlag(true); - data->next(); - assert(getContainer() == data); - } + if (!data) return; + data->reset(); + data->newFullPos(*this); + setResetFlag(true); + data->next(); + assert(getContainer() == data); } MutableArrayIter::MutableArrayIter(ArrayData *data, Variant *key, @@ -470,16 +449,15 @@ MutableArrayIter::MutableArrayIter(ArrayData *data, Variant *key, m_var = nullptr; m_key = key; m_valp = &val; - if (data) { - setAd(data); - escalateCheck(); - data = cowCheck(); - data->reset(); - data->newFullPos(*this); - setResetFlag(true); - data->next(); - assert(getContainer() == data); - } + if (!data) return; + setAd(data); + escalateCheck(); + data = cowCheck(); + data->reset(); + data->newFullPos(*this); + setResetFlag(true); + data->next(); + assert(getContainer() == data); } MutableArrayIter::~MutableArrayIter() { @@ -494,9 +472,7 @@ MutableArrayIter::~MutableArrayIter() { } bool MutableArrayIter::advance() { - if (!this->FullPos::advance()) { - return false; - } + if (!this->FullPos::advance()) return false; ArrayData* data = getArray(); assert(data); assert(!getResetFlag()); @@ -517,28 +493,26 @@ MArrayIter::MArrayIter(const RefData* ref) { assert(hasVar()); escalateCheck(); ArrayData* data = cowCheck(); - if (data) { - data->reset(); - data->newFullPos(*this); - setResetFlag(true); - data->next(); - assert(getContainer() == data); - } + if (!data) return; + data->reset(); + data->newFullPos(*this); + setResetFlag(true); + data->next(); + assert(getContainer() == data); } MArrayIter::MArrayIter(ArrayData *data) { m_var = nullptr; - if (data) { - assert(!data->isStatic()); - setAd(data); - escalateCheck(); - data = cowCheck(); - data->reset(); - data->newFullPos(*this); - setResetFlag(true); - data->next(); - assert(getContainer() == data); - } + if (!data) return; + assert(!data->isStatic()); + setAd(data); + escalateCheck(); + data = cowCheck(); + data->reset(); + data->newFullPos(*this); + setResetFlag(true); + data->next(); + assert(getContainer() == data); } MArrayIter::~MArrayIter() { @@ -621,7 +595,7 @@ bool Iter::minit(TypedValue* v1) { hasElems = false; } } else if (rtv->m_type == KindOfObject) { - if (rtv->m_data.pobj->getCollectionType() != 0) { + if (rtv->m_data.pobj->isCollection()) { raise_error("Collection elements cannot be taken by reference"); } bool isIterator; diff --git a/hphp/runtime/base/array/array_iterator.h b/hphp/runtime/base/array/array_iterator.h index 6ca83fcc3..d0219dae4 100644 --- a/hphp/runtime/base/array/array_iterator.h +++ b/hphp/runtime/base/array/array_iterator.h @@ -294,8 +294,7 @@ class FullPos { bool prepare(); ArrayData* getArray() const { - ArrayData *data = hasVar() ? getData() : getAd(); - return data; + return hasVar() ? getData() : getAd(); } bool hasVar() const { @@ -339,7 +338,10 @@ class FullPos { } protected: - ArrayData* getData() const; + ArrayData* getData() const { + assert(hasVar()); + return m_var->is(KindOfArray) ? m_var->getArrayData() : nullptr; + } ArrayData* cowCheck(); void escalateCheck(); ArrayData* reregister(); diff --git a/hphp/runtime/base/array/hphp_array.cpp b/hphp/runtime/base/array/hphp_array.cpp index 29c0b6d77..1e6f8ef0c 100644 --- a/hphp/runtime/base/array/hphp_array.cpp +++ b/hphp/runtime/base/array/hphp_array.cpp @@ -1601,7 +1601,7 @@ ArrayData* HphpArray::pop(Variant& value) { } else { value = uninit_null(); } - // To match PHP-like semantics, the pop operation resets the array's + // To conform to PHP behavior, the pop operation resets the array's // internal iterator. a->m_pos = a->nextElm(elms, ElmIndEmpty); return result; @@ -1613,7 +1613,7 @@ ArrayData* HphpArray::dequeue(Variant& value) { if (getCount() > 1) { result = a = copyImpl(); } - // To match PHP-like semantics, we invalidate all strong iterators when an + // To conform to PHP behavior, we invalidate all strong iterators when an // element is removed from the beginning of the array. a->freeStrongIterators(); Elm* elms = a->m_data; @@ -1628,7 +1628,7 @@ ArrayData* HphpArray::dequeue(Variant& value) { } else { value = uninit_null(); } - // To match PHP-like semantics, the dequeue operation resets the array's + // To conform to PHP behavior, the dequeue operation resets the array's // internal iterator a->m_pos = ssize_t(a->nextElm(elms, ElmIndEmpty)); return result; @@ -1640,7 +1640,7 @@ ArrayData* HphpArray::prepend(CVarRef v, bool copy) { if (copy) { result = a = copyImpl(); } - // To match PHP-like semantics, we invalidate all strong iterators when an + // To conform to PHP behavior, we invalidate all strong iterators when an // element is added to the beginning of the array. a->freeStrongIterators(); @@ -1666,7 +1666,7 @@ ArrayData* HphpArray::prepend(CVarRef v, bool copy) { // Renumber. a->compact(true); - // To match PHP-like semantics, the prepend operation resets the array's + // To conform to PHP behavior, the prepend operation resets the array's // internal iterator a->m_pos = ssize_t(a->nextElm(elms, ElmIndEmpty)); return result; @@ -1700,19 +1700,22 @@ bool HphpArray::validFullPos(const FullPos &fp) const { return (fp.m_pos != ssize_t(ElmIndEmpty)); } -void HphpArray::getFullPos(FullPos& fp) { - assert(fp.getContainer() == (ArrayData*)this); - fp.m_pos = m_pos; -} - -bool HphpArray::setFullPos(const FullPos& fp) { - assert(fp.getContainer() == (ArrayData*)this); - assert(!fp.getResetFlag()); - if (fp.m_pos != ssize_t(ElmIndEmpty)) { - m_pos = fp.m_pos; - return true; +bool HphpArray::advanceFullPos(FullPos& fp) { + Elm* elms = m_data; + if (fp.getResetFlag()) { + fp.setResetFlag(false); + fp.m_pos = ElmIndEmpty; + } else if (fp.m_pos == ssize_t(ElmIndEmpty)) { + return false; } - return false; + fp.m_pos = nextElm(elms, fp.m_pos); + if (fp.m_pos == ssize_t(ElmIndEmpty)) { + return false; + } + // To conform to PHP behavior, we need to set the internal + // cursor to point to the next element. + m_pos = nextElm(elms, fp.m_pos); + return true; } CVarRef HphpArray::currentRef() { diff --git a/hphp/runtime/base/array/hphp_array.h b/hphp/runtime/base/array/hphp_array.h index d687f6363..b4f524cec 100644 --- a/hphp/runtime/base/array/hphp_array.h +++ b/hphp/runtime/base/array/hphp_array.h @@ -180,8 +180,7 @@ public: // overrides ArrayData bool validFullPos(const FullPos &fp) const; - void getFullPos(FullPos& fp); - bool setFullPos(const FullPos& fp); + bool advanceFullPos(FullPos& fp); CVarRef currentRef(); CVarRef endRef(); diff --git a/hphp/runtime/vm/name_value_table_wrapper.cpp b/hphp/runtime/vm/name_value_table_wrapper.cpp index 1030f75c3..d0289c56a 100644 --- a/hphp/runtime/vm/name_value_table_wrapper.cpp +++ b/hphp/runtime/vm/name_value_table_wrapper.cpp @@ -282,22 +282,26 @@ bool NameValueTableWrapper::validFullPos(const FullPos & fp) const { return (iter.valid()); } -void NameValueTableWrapper::getFullPos(FullPos &fp) { - assert(fp.getContainer() == this); - fp.m_pos = m_pos; -} - -bool NameValueTableWrapper::setFullPos(const FullPos& fp) { - assert(fp.getContainer() == this); - assert(!fp.getResetFlag()); - if (fp.m_pos != ArrayData::invalid_index) { - NameValueTable::Iterator iter(m_tab, fp.m_pos); - if (iter.valid()) { - m_pos = iter.toInteger(); - return true; +bool NameValueTableWrapper::advanceFullPos(FullPos& fp) { + bool reset = fp.getResetFlag(); + NameValueTable::Iterator iter = reset ? + NameValueTable::Iterator(m_tab) : + NameValueTable::Iterator(m_tab, fp.m_pos); + if (reset) { + fp.setResetFlag(false); + } else { + if (!iter.valid()) { + return false; } + iter.next(); } - return false; + fp.m_pos = iter.toInteger(); + if (!iter.valid()) return false; + // To conform to PHP behavior, we need to set the internal + // cursor to point to the next element. + iter.next(); + m_pos = iter.toInteger(); + return true; } ArrayData* NameValueTableWrapper::escalateForSort() { diff --git a/hphp/runtime/vm/name_value_table_wrapper.h b/hphp/runtime/vm/name_value_table_wrapper.h index acd008e76..d38a9837a 100644 --- a/hphp/runtime/vm/name_value_table_wrapper.h +++ b/hphp/runtime/vm/name_value_table_wrapper.h @@ -133,8 +133,7 @@ public: // ArrayData implementation virtual Variant value(ssize_t &pos) const; virtual Variant each(); virtual bool validFullPos(const FullPos & fp) const; - virtual void getFullPos(FullPos&); - virtual bool setFullPos(const FullPos&); + virtual bool advanceFullPos(FullPos&); virtual ArrayData* escalateForSort(); virtual void ksort(int sort_flags, bool ascending);