PolicyArray: implement NOT_IMPLEMENTED methods

This implements the "last" unimplemented methods in ArrayShell. The sorting-related methods need no implementation because PolicyArray scales to HphpArray before sorting.

hphp/runtime/base/array/array_data.cpp

@@ -123,8 +123,8 @@ void ArrayData::release() {
     that->release();
     return;
   }
-  if (isArrayShell()) {
-    auto that = static_cast<ArrayShell*>(this);
+  if (isPolicyArray()) {
+    auto that = static_cast<PolicyArray*>(this);
     that->release();
     return;
   }

hphp/runtime/base/array/array_data.h

@@ -42,7 +42,7 @@ class ArrayData : public Countable {
     kHphpArray,
     kSharedMap,
     kNameValueTableWrapper,
-    kArrayShell,
+    kPolicyArray,
   };
 
 public:
@@ -166,7 +166,7 @@ public:
   /*
    * Specific derived class type querying operators.
    */
-  bool isArrayShell() const { return m_kind == ArrayKind::kArrayShell; }
+  bool isPolicyArray() const { return m_kind == ArrayKind::kPolicyArray; }
   bool isHphpArray() const { return m_kind == ArrayKind::kHphpArray; }
   bool isSharedMap() const { return m_kind == ArrayKind::kSharedMap; }
   bool isNameValueTableWrapper() const {

hphp/runtime/base/array/array_init.cpp

@@ -27,8 +27,8 @@ ArrayInit::ArrayInit(ssize_t n) {
   if (!n) {
     m_data = HphpArray::GetStaticEmptyArray();
   } else if (false) {
-    // Force compilation of ArrayShell
-    m_data = NEW(ArrayShell)(n);
+    // Force compilation of PolicyArray
+    m_data = NEW(PolicyArray)(n);
   } else {
     m_data = ArrayData::Make(n);
   }

hphp/runtime/base/array/policy_array.cpp

@@ -182,10 +182,10 @@ void SimpleArrayStore::prepend(const Variant& v, uint length,
 
 ////////////////////////////////////////////////////////////////////////////////
 
-IMPLEMENT_SMART_ALLOCATION(ArrayShell)
+IMPLEMENT_SMART_ALLOCATION(PolicyArray)
 
-ArrayShell::ArrayShell(uint capacity)
-    : ArrayData(ArrayKind::kArrayShell)
+PolicyArray::PolicyArray(uint capacity)
+    : ArrayData(ArrayKind::kPolicyArray)
     , Store(m_allocMode, capacity) {
   m_size = 0;
   m_pos = invalid_index;
@@ -194,9 +194,9 @@ ArrayShell::ArrayShell(uint capacity)
   APILOG << "(" << capacity << ");";
 }
 
-ArrayShell::ArrayShell(const ArrayShell& rhs, uint capacity,
+PolicyArray::PolicyArray(const PolicyArray& rhs, uint capacity,
                        AllocationMode am)
-    : ArrayData(ArrayKind::kArrayShell, am)
+    : ArrayData(ArrayKind::kPolicyArray, am)
     , Store(rhs, rhs.m_size, capacity, am, &rhs) {
   m_size = rhs.m_size;
   m_pos = rhs.m_pos;
@@ -205,30 +205,30 @@ ArrayShell::ArrayShell(const ArrayShell& rhs, uint capacity,
   APILOG << "(" << &rhs << ", " << capacity << ", " << uint(am) << ");";
 }
 
-ArrayShell::~ArrayShell() {
+PolicyArray::~PolicyArray() {
   APILOG << "()";
   destroy(m_size, m_allocMode);
 }
 
-Variant ArrayShell::getKey(ssize_t pos) const {
+Variant PolicyArray::getKey(ssize_t pos) const {
   APILOG << "(" << pos << ")";
   assert(size_t(pos) < m_size);
   return key(toPos(pos));
 }
 
-Variant ArrayShell::getValue(ssize_t pos) const {
+Variant PolicyArray::getValue(ssize_t pos) const {
   APILOG << "(" << pos << ")";
   assert(size_t(pos) < m_size);
   return getValueRef(pos);
 }
 
-const Variant& ArrayShell::getValueRef(ssize_t pos) const {
+const Variant& PolicyArray::getValueRef(ssize_t pos) const {
   APILOG << "(" << pos << ")";
   assert(size_t(pos) < m_size);
   return val(toPos(pos));
 }
 
-bool ArrayShell::isVectorData() const {
+bool PolicyArray::isVectorData() const {
   APILOG << "()";
   for (ssize_t i = 0; i < m_size; ++i) {
     if (Store::find(i, m_size) != toPos(i)) return false;
@@ -236,7 +236,7 @@ bool ArrayShell::isVectorData() const {
   return true;
 }
 
-Variant ArrayShell::reset() {
+Variant PolicyArray::reset() {
   APILOG << "()";
   if (m_size) {
     auto const first = firstIndex(m_size);
@@ -247,11 +247,17 @@ Variant ArrayShell::reset() {
   return false;
 }
 
-Variant ArrayShell::prev() {
-  NOT_IMPLEMENTED();
+Variant PolicyArray::prev() {
+  APILOG << "()";
+  if (m_pos == invalid_index
+      || (m_pos = iter_rewind(m_pos)) == invalid_index) {
+    return false;
+  }
+  assert(size_t(m_pos) < m_size);
+  return val(toPos(m_pos));
 }
 
-Variant ArrayShell::current() const {
+Variant PolicyArray::current() const {
   APILOG << "()";
   if (m_pos == invalid_index) {
     return false;
@@ -260,7 +266,7 @@ Variant ArrayShell::current() const {
   return val(toPos(m_pos));
 }
 
-Variant ArrayShell::next() {
+Variant PolicyArray::next() {
   APILOG << "()";
   if (m_pos == invalid_index
       || (m_pos = iter_advance(m_pos)) == invalid_index) {
@@ -270,7 +276,7 @@ Variant ArrayShell::next() {
   return val(toPos(m_pos));
 }
 
-Variant ArrayShell::end() {
+Variant PolicyArray::end() {
   if (m_size) {
     auto const last = lastIndex(m_size);;
     m_pos = toUint32(last);
@@ -280,7 +286,7 @@ Variant ArrayShell::end() {
   return false;
 }
 
-Variant ArrayShell::key() const {
+Variant PolicyArray::key() const {
   APILOG << ")";
   if (m_pos == invalid_index) {
     return uninit_null();
@@ -289,15 +295,19 @@ Variant ArrayShell::key() const {
   return key(toPos(m_pos));
 }
 
-Variant ArrayShell::value(int32_t &pos) const {
-  NOT_IMPLEMENTED();
+Variant PolicyArray::value(int32_t &pos) const {
+  if (pos == ArrayData::invalid_index) {
+    return false;
+  }
+  assert(uint32_t(pos) < m_size);
+  return Variant(Store::val(toPos(pos)));
 }
 
 static const StaticString s_value("value");
 static const StaticString s_key("key");
 static_assert(ArrayData::invalid_index == size_t(-1), "ehm");
 
-Variant ArrayShell::each() {
+Variant PolicyArray::each() {
   APILOG << "()";
   if (m_pos == invalid_index) return false;
   assert(m_size);
@@ -314,7 +324,7 @@ Variant ArrayShell::each() {
 }
 
 template <class K>
-TypedValue* ArrayShell::nvGetImpl(K k) const {
+TypedValue* PolicyArray::nvGetImpl(K k) const {
   APILOG << "(" << keystr(k) << ")";
   auto const pos = find(k, m_size);
   return LIKELY(pos != PosType::invalid)
@@ -322,20 +332,20 @@ TypedValue* ArrayShell::nvGetImpl(K k) const {
     : nullptr;
 }
 
-void ArrayShell::nvGetKey(TypedValue* out, ssize_t pos) {
+void PolicyArray::nvGetKey(TypedValue* out, ssize_t pos) {
   APILOG << "(" << out << ", " << pos << ")";
   assert(size_t(pos) < m_size);
   new(out) Variant(key(toPos(pos)));
 }
 
-TypedValue* ArrayShell::nvGetValueRef(ssize_t pos) {
+TypedValue* PolicyArray::nvGetValueRef(ssize_t pos) {
   APILOG << "(" << pos << ")";
   assert(size_t(pos) < m_size);
   return reinterpret_cast<TypedValue*>(&lval(toPos(pos)));
 }
 
 template <class K>
-TypedValue* ArrayShell::nvGetCellImpl(K k) const {
+TypedValue* PolicyArray::nvGetCellImpl(K k) const {
   APILOG << "(" << keystr(k) << ")";
   auto const pos = find(k, m_size);
   return LIKELY(pos != PosType::invalid)
@@ -343,8 +353,14 @@ TypedValue* ArrayShell::nvGetCellImpl(K k) const {
     : nvGetNotFound(k);
 }
 
+// template <class K>
+// ssize_t PolicyArray::getIndexImpl(K k) const {
+//   APILOG << "(" << keystr(k) << ")";
+//   return toInt64(find(k, m_size));
+// }
+
 template <class K>
-const Variant& ArrayShell::getImpl(K k, bool error) const {
+const Variant& PolicyArray::getImpl(K k, bool error) const {
   APILOG << "(" << keystr(k) << ", " << error << ")";
   auto const pos = find(k, m_size);
   if (pos != PosType::invalid) {
@@ -354,13 +370,13 @@ const Variant& ArrayShell::getImpl(K k, bool error) const {
 }
 
 template <class K>
-ArrayData *ArrayShell::lvalImpl(K k, Variant*& ret,
+ArrayData *PolicyArray::lvalImpl(K k, Variant*& ret,
                                   bool copy, bool checkExist) {
   APILOG << "(" << keystr(k) << ", " << ret << ", "
          << copy << ", " << checkExist << ")";
 
   if (copy) {
-    return ArrayShell::copy()->lvalImpl(k, ret, false, checkExist);
+    return PolicyArray::copy()->lvalImpl(k, ret, false, checkExist);
   }
 
   PosType pos = PosType::invalid;
@@ -399,9 +415,9 @@ ArrayData *ArrayShell::lvalImpl(K k, Variant*& ret,
   return this;
 }
 
-ArrayData *ArrayShell::lvalNew(Variant *&ret, bool copy) {
+ArrayData *PolicyArray::lvalNew(Variant *&ret, bool copy) {
   if (copy) {
-    return ArrayShell::copy()->lvalNew(ret, false);
+    return PolicyArray::copy()->lvalNew(ret, false);
   }
 
   // Andrei: TODO - append() currently never fails, probably it
@@ -419,20 +435,29 @@ ArrayData *ArrayShell::lvalNew(Variant *&ret, bool copy) {
   return this;
 }
 
-ArrayData *ArrayShell::createLvalPtr(StringData* k, Variant *&ret, bool copy) {
-  NOT_IMPLEMENTED();
+ArrayData *PolicyArray::createLvalPtr(StringData* k, Variant *&ret, bool copy) {
+  APILOG << "(" << keystr(k) << ", " << ret << ", " << copy << ")";
+  return addLval(k, ret, copy);
 }
 
-ArrayData *ArrayShell::getLvalPtr(StringData* k, Variant *&ret, bool copy) {
-  NOT_IMPLEMENTED();
+ArrayData *PolicyArray::getLvalPtr(StringData* k, Variant *&ret, bool copy) {
+  APILOG << "(" << keystr(k) << ", " << ret << ", " << copy << ")";
+  if (copy) {
+    return PolicyArray::copy()->getLvalPtr(k, ret, false);
+  }
+  const auto pos = find(k, m_size);
+  ret = pos != PosType::invalid
+    ? &Store::lval(pos)
+    : nullptr;
+  return this;
 }
 
 template <class K>
-ArrayShell* ArrayShell::setImpl(K k, const Variant& v, bool copy) {
+PolicyArray* PolicyArray::setImpl(K k, const Variant& v, bool copy) {
   APILOG << "(" << keystr(k) << ", " << valstr(v) << ", " << copy
          << ")";
-  ArrayShell* result = this;
-  if (copy) result = ArrayShell::copy();
+  PolicyArray* result = this;
+  if (copy) result = PolicyArray::copy();
   if (result->update(k, v, result->m_size, result->m_allocMode)) {
     // Added a new element, must update size and possibly m_pos
     if (m_pos == invalid_index) m_pos = result->m_size;
@@ -442,11 +467,11 @@ ArrayShell* ArrayShell::setImpl(K k, const Variant& v, bool copy) {
 }
 
 template <class K>
-ArrayData *ArrayShell::setRefImpl(K k, CVarRef v, bool copy) {
+ArrayData *PolicyArray::setRefImpl(K k, CVarRef v, bool copy) {
   APILOG << "(" << keystr(k) << ", " << valstr(v) << ", " << copy << ")";
 
   if (copy) {
-    return ArrayShell::copy()->setRef(k, v, false);
+    return PolicyArray::copy()->setRef(k, v, false);
   }
 
   auto const pos = find(k, m_size);
@@ -467,10 +492,10 @@ ArrayData *ArrayShell::setRefImpl(K k, CVarRef v, bool copy) {
 }
 
 template <class K>
-ArrayData *ArrayShell::addImpl(K k, const Variant& v, bool copy) {
+ArrayData *PolicyArray::addImpl(K k, const Variant& v, bool copy) {
   APILOG << "(" << keystr(k) << ", " << valstr(v) << ", " << copy << ");";
   if (copy) {
-    auto result = ArrayShell::copy(m_size * 2 + 1);
+    auto result = PolicyArray::copy(m_size * 2 + 1);
     result->add(k, v, false);
     return result;
   }
@@ -484,10 +509,10 @@ ArrayData *ArrayShell::addImpl(K k, const Variant& v, bool copy) {
 }
 
 template <class K>
-ArrayShell *ArrayShell::addLvalImpl(K k, Variant*& ret, bool copy) {
+PolicyArray *PolicyArray::addLvalImpl(K k, Variant*& ret, bool copy) {
   APILOG << "(" << k << ", " << ret << ", " << copy << ")";
   if (copy) {
-    return ArrayShell::copy()->addLval(k, ret, false);
+    return PolicyArray::copy()->addLval(k, ret, false);
   }
   assert(!exists(k) && m_size <= capacity());
   if (m_size == capacity()) {
@@ -499,11 +524,11 @@ ArrayShell *ArrayShell::addLvalImpl(K k, Variant*& ret, bool copy) {
 }
 
 template <class K>
-ArrayData *ArrayShell::removeImpl(K k, bool copy) {
+ArrayData *PolicyArray::removeImpl(K k, bool copy) {
   APILOG << "(" << keystr(k) << ", " << copy << ")";
 
   if (copy) {
-    return ArrayShell::copy()->remove(k, false);
+    return PolicyArray::copy()->remove(k, false);
   }
 
   auto const pos = find(k, m_size);
@@ -536,35 +561,35 @@ ArrayData *ArrayShell::removeImpl(K k, bool copy) {
   return this;
 }
 
-ssize_t ArrayShell::iter_begin() const {
+ssize_t PolicyArray::iter_begin() const {
   APILOG << "()";
   return m_size ? toInt64(firstIndex(m_size)) : invalid_index;
 }
 
-ssize_t ArrayShell::iter_end() const {
+ssize_t PolicyArray::iter_end() const {
   APILOG << "()";
   return ssize_t(lastIndex(m_size));
 }
 
-ssize_t ArrayShell::iter_advance(ssize_t prev) const {
+ssize_t PolicyArray::iter_advance(ssize_t prev) const {
   APILOG << "(" << prev << ")";
   auto const result = toInt64(nextIndex(toPos(prev), m_size));
   MYLOG << "returning " << result;
   return result;
 }
 
-ssize_t ArrayShell::iter_rewind(ssize_t prev) const {
+ssize_t PolicyArray::iter_rewind(ssize_t prev) const {
   APILOG << "(" << prev << ")";
   return toInt64(prevIndex(toPos(prev), m_size));
 }
 
-bool ArrayShell::validFullPos(const FullPos& fp) const {
+bool PolicyArray::validFullPos(const FullPos& fp) const {
   APILOG << "(" << fp.m_pos << ";" << fp.getResetFlag() << ")";
   assert(fp.getContainer() == this);
   return fp.m_pos != invalid_index;
 }
 
-bool ArrayShell::advanceFullPos(FullPos &fp) {
+bool PolicyArray::advanceFullPos(FullPos &fp) {
   APILOG << "(" << fp.m_pos << ";" << fp.getResetFlag() << ")";
   assert(fp.getContainer() == this);
   if (fp.getResetFlag()) {
@@ -583,11 +608,20 @@ bool ArrayShell::advanceFullPos(FullPos &fp) {
   return true;
 }
 
-CVarRef ArrayShell::endRef() {
-  NOT_IMPLEMENTED();
+// CVarRef PolicyArray::currentRef() {
+//   APILOG << "()";
+//   assert(m_pos != ArrayData::invalid_index);
+//   assert(size_t(m_pos) < m_size);
+//   return val(toPos(m_pos));
+// }
+
+CVarRef PolicyArray::endRef() {
+  APILOG << "()";
+  assert(m_size > 0);
+  return val(toPos(m_size - 1));
 }
 
-HphpArray* ArrayShell::toHphpArray() const {
+HphpArray* PolicyArray::toHphpArray() const {
   auto result = ArrayData::Make(m_size);
   FOR_EACH_RANGE (i, 0, m_size) {
     if (hasStrKey(toPos(i))) {
@@ -599,38 +633,14 @@ HphpArray* ArrayShell::toHphpArray() const {
   return result;
 }
 
-ArrayData* ArrayShell::escalateForSort() {
+ArrayData* PolicyArray::escalateForSort() {
   APILOG << "()";
   return toHphpArray();
 }
 
-void ArrayShell::ksort(int sort_flags, bool ascending) {
-  NOT_IMPLEMENTED();
-}
-
-void ArrayShell::sort(int sort_flags, bool ascending) {
-  NOT_IMPLEMENTED();
-}
-
-void ArrayShell::asort(int sort_flags, bool ascending) {
-  NOT_IMPLEMENTED();
-}
-
-void ArrayShell::uksort(CVarRef cmp_function) {
-  NOT_IMPLEMENTED();
-}
-
-void ArrayShell::usort(CVarRef cmp_function) {
-  NOT_IMPLEMENTED();
-}
-
-void ArrayShell::uasort(CVarRef cmp_function) {
-  NOT_IMPLEMENTED();
-}
-
-ArrayShell *ArrayShell::copy() const {
+PolicyArray *PolicyArray::copy() const {
   APILOG << "()";
-  auto result = NEW(ArrayShell)(
+  auto result = NEW(PolicyArray)(
     *this,
     capacity() + (m_size == capacity()),
     m_allocMode);
@@ -638,39 +648,39 @@ ArrayShell *ArrayShell::copy() const {
   return result;
 }
 
-ArrayShell* ArrayShell::copy(uint capacity) {
+PolicyArray* PolicyArray::copy(uint capacity) {
   APILOG << "(" << capacity << ")";
-  return NEW(ArrayShell)(*this, capacity, m_allocMode);
+  return NEW(PolicyArray)(*this, capacity, m_allocMode);
 }
 
-ArrayShell *ArrayShell::copyWithStrongIterators() const {
+PolicyArray *PolicyArray::copyWithStrongIterators() const {
   APILOG << "()";
-  auto result = ArrayShell::copy();
-  moveStrongIterators(result, const_cast<ArrayShell*>(this));
+  auto result = PolicyArray::copy();
+  moveStrongIterators(result, const_cast<PolicyArray*>(this));
   assert(result->getCount() == 0);
   return result;
 }
 
-ArrayData *ArrayShell::nonSmartCopy() const {
+ArrayData *PolicyArray::nonSmartCopy() const {
   APILOG << "()";
-  //return NEW(ArrayShell)(*this, capacity(), true);
+  //return NEW(PolicyArray)(*this, capacity(), true);
   return toHphpArray()->nonSmartCopy();
 }
 
-ArrayShell *ArrayShell::append(const Variant& v, bool copy) {
+PolicyArray *PolicyArray::append(const Variant& v, bool copy) {
   APILOG << "(" << valstr(v) << ", " << copy << ")";
   if (copy) {
-    return ArrayShell::copy()->append(v, false);
+    return PolicyArray::copy()->append(v, false);
   }
   grow(m_size, m_size + 1, m_size * 2 + 1, m_allocMode);
   appendNoGrow(nextKeyBump(), v);
   return this;
 }
 
-ArrayShell *ArrayShell::appendRef(const Variant& v, bool copy) {
+PolicyArray *PolicyArray::appendRef(const Variant& v, bool copy) {
   APILOG << "(" << valstr(v) << ", " << copy << ")";
   if (copy) {
-    return ArrayShell::copy()->appendRef(v, false);
+    return PolicyArray::copy()->appendRef(v, false);
   }
   //addValWithRef(nextKeyBump(), v);
   auto const k = nextKeyBump();
@@ -685,10 +695,10 @@ ArrayShell *ArrayShell::appendRef(const Variant& v, bool copy) {
   /**
    * Similar to append(v, copy), with reference in v preserved.
    */
-ArrayData *ArrayShell::appendWithRef(CVarRef v, bool copy) {
+ArrayData *PolicyArray::appendWithRef(CVarRef v, bool copy) {
   APILOG << "(" << valstr(v) << ", " << copy << ")";
   if (copy) {
-    return ArrayShell::copy()->appendWithRef(v, false);
+    return PolicyArray::copy()->appendWithRef(v, false);
   }
   if (m_size == capacity()) {
     grow(m_size, m_size + 1, m_size * 2 + 1, m_allocMode);
@@ -699,7 +709,7 @@ ArrayData *ArrayShell::appendWithRef(CVarRef v, bool copy) {
 }
 
 template <class K>
-void ArrayShell::addValWithRef(K k, const Variant& v) {
+void PolicyArray::addValWithRef(K k, const Variant& v) {
   MYLOG << (void*)this << "->addValWithRef("
         << keystr(k) << ", " << valstr(v)
         << "); size=" << m_size;
@@ -714,7 +724,7 @@ void ArrayShell::addValWithRef(K k, const Variant& v) {
   appendNoGrow(k, Variant::NullInit())->setWithRef(v);
 }
 
-void ArrayShell::nextInsertWithRef(const Variant& v) {
+void PolicyArray::nextInsertWithRef(const Variant& v) {
   MYLOG << (void*)this << "->nextInsertWithRef("
         << valstr(v)
         << "); size=" << m_size;
@@ -722,7 +732,7 @@ void ArrayShell::nextInsertWithRef(const Variant& v) {
   // the overzealous gcc issues a spurious warning as such:
   //
   // hphp/runtime/base/array/policy_array.h: In member function 'void
-  // HPHP::ArrayShell::nextInsertWithRef(const HPHP::Variant&)':
+  // HPHP::PolicyArray::nextInsertWithRef(const HPHP::Variant&)':
   // hphp/runtime/base/array/policy_array.h:114:5: error: assuming
   // signed overflow does not occur when assuming that (X + c) < X is
   // always false [-Werror=strict-overflow]
@@ -734,10 +744,10 @@ void ArrayShell::nextInsertWithRef(const Variant& v) {
   appendNoGrow(k, Variant::NullInit())->setWithRef(v);
 }
 
-ArrayData *ArrayShell::plus(const ArrayData *elems, bool copy) {
+ArrayData *PolicyArray::plus(const ArrayData *elems, bool copy) {
   APILOG << "(" << elems << ", " << copy << ")";
   if (copy) {
-    return ArrayShell::copy()->plus(elems, false);
+    return PolicyArray::copy()->plus(elems, false);
   }
 
   assert(elems);
@@ -755,10 +765,10 @@ ArrayData *ArrayShell::plus(const ArrayData *elems, bool copy) {
   return this;
 }
 
-ArrayData *ArrayShell::merge(const ArrayData *elems, bool copy) {
+ArrayData *PolicyArray::merge(const ArrayData *elems, bool copy) {
   APILOG << "(" << elems << ", " << copy << ")";
   if (copy) {
-    return ArrayShell::copy()->merge(elems, false);
+    return PolicyArray::copy()->merge(elems, false);
   }
 
   assert(elems);
@@ -783,9 +793,10 @@ ArrayData *ArrayShell::merge(const ArrayData *elems, bool copy) {
   /**
    * Stack function: pop the last item and return it.
    */
-ArrayData* ArrayShell::pop(Variant &value) {
+ArrayData* PolicyArray::pop(Variant &value) {
+  APILOG << "(" << &value << ")";
   if (getCount() > 1) {
-    return ArrayShell::copy()->pop(value);
+    return PolicyArray::copy()->pop(value);
   }
   if (!m_size) {
     value = uninit_null();
@@ -794,7 +805,14 @@ ArrayData* ArrayShell::pop(Variant &value) {
   auto pos = lastIndex(m_size);
   assert(size_t(pos) < m_size);
   value = val(pos);
-  erase(pos, m_size);
+
+  // Match PHP 5.3.1 semantics
+  if (!hasStrKey(pos)
+      && Store::nextKey() == 1 + static_cast<int64_t>(key(pos))) {
+    nextKeyPop();
+  }
+
+  Store::erase(pos, m_size);
   --m_size;
   // To match PHP-like semantics, the pop operation resets the array's
   // internal iterator.
@@ -802,10 +820,10 @@ ArrayData* ArrayShell::pop(Variant &value) {
   return this;
 }
 
-ArrayData *ArrayShell::dequeue(Variant &value) {
+ArrayData *PolicyArray::dequeue(Variant &value) {
   APILOG << "(" << &value << ")";
   if (getCount() > 1) {
-    return ArrayShell::copy()->dequeue(value);
+    return PolicyArray::copy()->dequeue(value);
   }
 
   // To match PHP-like semantics, we invalidate all strong iterators when an
@@ -829,10 +847,10 @@ ArrayData *ArrayShell::dequeue(Variant &value) {
   return this;
 }
 
-ArrayData* ArrayShell::prepend(CVarRef v, bool copy) {
+ArrayData* PolicyArray::prepend(CVarRef v, bool copy) {
   APILOG << "(" << valstr(v) << ", " << copy << ")";
   if (copy) {
-    return ArrayShell::copy()->prepend(v, false);
+    return PolicyArray::copy()->prepend(v, false);
   }
   // To match PHP-like semantics, we invalidate all strong iterators when an
   // element is added to the beginning of the array.
@@ -849,7 +867,7 @@ ArrayData* ArrayShell::prepend(CVarRef v, bool copy) {
   return this;
 }
 
-void ArrayShell::renumber() {
+void PolicyArray::renumber() {
   APILOG << "()";
   if (!m_size) {
     return;
@@ -905,7 +923,7 @@ void ArrayShell::renumber() {
   assert(i == siKeys.cend());
 }
 
-void ArrayShell::onSetEvalScalar() {
+void PolicyArray::onSetEvalScalar() {
   APILOG << "()";
   //FOR_EACH_RANGE (pos, 0, m_size) {
   for (auto pos = firstIndex(m_size); pos != PosType::invalid;
@@ -926,19 +944,7 @@ void ArrayShell::onSetEvalScalar() {
   }
 }
 
-void ArrayShell::dump() {
-  NOT_IMPLEMENTED();
-}
-
-void ArrayShell::dump(std::string &out) {
-  NOT_IMPLEMENTED();
-}
-
-void ArrayShell::dump(std::ostream &os) {
-  NOT_IMPLEMENTED();
-}
-
-ArrayData *ArrayShell::escalate() const {
+ArrayData *PolicyArray::escalate() const {
   APILOG << "()";
   return ArrayData::escalate();
 }

hphp/runtime/base/array/policy_array.h

@@ -73,7 +73,7 @@ inline PosType toPos(uint32_t n) {
 }
 
 /**
-SimpleArrayStore implements a basic storage strategy for ArrayShell. It
+SimpleArrayStore implements a basic storage strategy for PolicyArray. It
 uses two contiguous arrays, one for keys and the other for
 values. There is no indexing, just linear search, so don't use for
 large arrays.
@@ -173,7 +173,7 @@ protected:
     auto result = toPos(toInt64(current) + 1);
     return toUint32(result) < length ? result : PosType::invalid;
   }
-  PosType prevIndex(PosType current, uint length) const {
+  PosType prevIndex(PosType current, uint /*length*/) const {
     if (current == PosType(0) || current == PosType::invalid) {
       return PosType::invalid;
     }
@@ -184,7 +184,9 @@ protected:
   Primitives for the next available integral key.
    */
   void nextKeyReset() { m_nextKey = 0; }
+  int64_t nextKey() const { return m_nextKey; }
   int64_t nextKeyBump() { return m_nextKey++; }
+  int64_t nextKeyPop() { return m_nextKey--; }
 
   /**
   Prepend v to the array. An uninitialized hole is left in the first
@@ -321,15 +323,15 @@ private:
 };
 
 ////////////////////////////////////////////////////////////////////////////////
-// ArrayShell
+// PolicyArray
 ////////////////////////////////////////////////////////////////////////////////
-class ArrayShell : public ArrayData, private SimpleArrayStore {
+class PolicyArray : public ArrayData, private SimpleArrayStore {
   typedef SimpleArrayStore Store;
   using Store::key;
 
-  ArrayShell(const ArrayShell& rhs) = delete;
-  ArrayShell& operator=(const ArrayShell& rhs) = delete;
-  ArrayShell(const ArrayShell& rhs, uint capacity, ArrayData::AllocationMode);
+  PolicyArray(const PolicyArray& rhs) = delete;
+  PolicyArray& operator=(const PolicyArray& rhs) = delete;
+  PolicyArray(const PolicyArray& rhs, uint capacity, ArrayData::AllocationMode);
 
   template <class K>
   void addValWithRef(K k, const Variant& value);
@@ -340,17 +342,17 @@ class ArrayShell : public ArrayData, private SimpleArrayStore {
 
 public:
   // Memory allocator methods.
-  DECLARE_SMART_ALLOCATION(ArrayShell);
+  DECLARE_SMART_ALLOCATION(PolicyArray);
 
-  explicit ArrayShell(uint size);
+  explicit PolicyArray(uint size);
 
-  virtual ~ArrayShell() FOLLY_OVERRIDE;
+  virtual ~PolicyArray() FOLLY_OVERRIDE;
 
   /**
    * Number of elements this array has.
    */
   virtual ssize_t vsize() const FOLLY_OVERRIDE {
-    // vsize() called, but m_size should never be -1 in ArrayShell
+    // vsize() called, but m_size should never be -1 in PolicyArray
     assert(false);
     return m_size;
   }
@@ -501,7 +503,7 @@ public:
    */
 private:
   template <class K>
-  ArrayShell* setImpl(K k, CVarRef v, bool copy);
+  PolicyArray* setImpl(K k, CVarRef v, bool copy);
 
 public:
   virtual ArrayData* set(int64_t k, CVarRef v, bool copy) FOLLY_OVERRIDE {
@@ -546,14 +548,14 @@ public:
    */
 private:
   template <class K>
-  ArrayShell* addLvalImpl(K k, Variant*& ret, bool copy);
+  PolicyArray* addLvalImpl(K k, Variant*& ret, bool copy);
 
 public:
-  virtual ArrayShell *addLval(int64_t k, Variant *&ret, bool copy)
+  virtual PolicyArray *addLval(int64_t k, Variant *&ret, bool copy)
     FOLLY_OVERRIDE {
     return addLvalImpl(k, ret, copy);
   }
-  virtual ArrayShell *addLval(StringData* k, Variant *&ret, bool copy)
+  virtual PolicyArray *addLval(StringData* k, Variant *&ret, bool copy)
     FOLLY_OVERRIDE {
     return addLvalImpl(k, ret, copy);
   }
@@ -605,12 +607,6 @@ public:
   virtual CVarRef endRef() FOLLY_OVERRIDE;
 
   virtual ArrayData* escalateForSort() FOLLY_OVERRIDE;
-  virtual void ksort(int sort_flags, bool ascending) FOLLY_OVERRIDE;
-  virtual void sort(int sort_flags, bool ascending) FOLLY_OVERRIDE;
-  virtual void asort(int sort_flags, bool ascending) FOLLY_OVERRIDE;
-  virtual void uksort(CVarRef cmp_function) FOLLY_OVERRIDE;
-  virtual void usort(CVarRef cmp_function) FOLLY_OVERRIDE;
-  virtual void uasort(CVarRef cmp_function) FOLLY_OVERRIDE;
 
   /**
    * Make a copy of myself.
@@ -619,9 +615,9 @@ public:
    * Is only implemented for array types that need to be able to go
    * into the static array list.
    */
-  ArrayShell* copy(uint minCapacity);
-  virtual ArrayShell *copy() const FOLLY_OVERRIDE;
-  virtual ArrayShell *copyWithStrongIterators() const FOLLY_OVERRIDE;
+  PolicyArray* copy(uint minCapacity);
+  virtual PolicyArray *copy() const FOLLY_OVERRIDE;
+  virtual PolicyArray *copyWithStrongIterators() const FOLLY_OVERRIDE;
   virtual ArrayData *nonSmartCopy() const FOLLY_OVERRIDE;
 
 private:
@@ -645,8 +641,8 @@ public:
    * then append the value. Return NULL if escalation is not needed, or an
    * escalated array data.
    */
-  virtual ArrayShell *append(CVarRef v, bool copy) FOLLY_OVERRIDE;
-  virtual ArrayShell *appendRef(CVarRef v, bool copy) FOLLY_OVERRIDE;
+  virtual PolicyArray *append(CVarRef v, bool copy) FOLLY_OVERRIDE;
+  virtual PolicyArray *appendRef(CVarRef v, bool copy) FOLLY_OVERRIDE;
 
   /**
    * Similar to append(v, copy), with reference in v preserved.
@@ -683,10 +679,6 @@ public:
 
   virtual void onSetEvalScalar() FOLLY_OVERRIDE;
 
-  virtual void dump() FOLLY_OVERRIDE;
-  virtual void dump(std::string &out) FOLLY_OVERRIDE;
-  virtual void dump(std::ostream &os) FOLLY_OVERRIDE;
-
   virtual ArrayData *escalate() const FOLLY_OVERRIDE;
 };