diff --git a/hphp/runtime/base/array_data-defs.h b/hphp/runtime/base/array_data-defs.h index 4cd6cff1f..3faa29045 100644 --- a/hphp/runtime/base/array_data-defs.h +++ b/hphp/runtime/base/array_data-defs.h @@ -66,18 +66,16 @@ inline CVarRef ArrayData::get(const StringData* k, bool error) const { return tv ? tvAsCVarRef(tv) : getNotFound(k, error); } -inline ArrayData* ArrayData::lval(CStrRef k, Variant *&ret, bool copy, - bool checkExist) { +inline ArrayData* ArrayData::lval(CStrRef k, Variant *&ret, bool copy) { assert(IsValidKey(k)); - return lval(k.get(), ret, copy, checkExist); + return lval(k.get(), ret, copy); } -inline ArrayData* ArrayData::lval(CVarRef k, Variant *&ret, bool copy, - bool checkExist) { +inline ArrayData* ArrayData::lval(CVarRef k, Variant *&ret, bool copy) { assert(IsValidKey(k)); auto const cell = k.asCell(); - return isIntKey(cell) ? lval(getIntKey(cell), ret, copy, checkExist) - : lval(getStringKey(cell), ret, copy, checkExist); + return isIntKey(cell) ? lval(getIntKey(cell), ret, copy) + : lval(getStringKey(cell), ret, copy); } inline diff --git a/hphp/runtime/base/array_data.h b/hphp/runtime/base/array_data.h index 59fe5ebaa..bccd64981 100644 --- a/hphp/runtime/base/array_data.h +++ b/hphp/runtime/base/array_data.h @@ -114,7 +114,7 @@ public: * Array interface functions. * * 1. For functions that return ArrayData pointers, these are the ones that - * can potentially escalate into a different ArrayData type. Return NULL + * can potentially escalate into a different ArrayData type. Return this * if no escalation is needed. * * 2. All functions with a "key" parameter are type-specialized. @@ -233,17 +233,15 @@ public: Variant getKey(ssize_t pos) const; /** - * Getting l-value (that Variant pointer) at specified key. Return NULL if + * Getting l-value (that Variant pointer) at specified key. Return this if * escalation is not needed, or an escalated array data. */ - virtual ArrayData *lval(int64_t k, Variant *&ret, bool copy, - bool checkExist = false) = 0; - virtual ArrayData *lval(StringData* k, Variant *&ret, bool copy, - bool checkExist = false) = 0; + virtual ArrayData *lval(int64_t k, Variant *&ret, bool copy) = 0; + virtual ArrayData *lval(StringData* k, Variant *&ret, bool copy) = 0; /** * Getting l-value (that Variant pointer) of a new element with the next - * available integer key. Return NULL if escalation is not needed, or an + * available integer key. Return this if escalation is not needed, or an * escalated array data. Note that adding a new element with the next * available integer key may fail, in which case ret is set to point to * the lval blackhole (see Variant::lvalBlackHole() for details). @@ -260,7 +258,7 @@ public: /** * Setting a value at specified key. If "copy" is true, make a copy first - * then set the value. Return NULL if escalation is not needed, or an + * then set the value. Return this if escalation is not needed, or an * escalated array data. */ ArrayData *set(int64_t k, CVarRef v, bool copy); @@ -286,7 +284,7 @@ public: /** * Remove a value at specified key. If "copy" is true, make a copy first - * then remove the value. Return NULL if escalation is not needed, or an + * then remove the value. Return this if escalation is not needed, or an * escalated array data. */ virtual ArrayData *remove(int64_t k, bool copy) = 0; @@ -302,8 +300,8 @@ public: CVarRef get(const StringData* k, bool error = false) const; CVarRef get(CStrRef k, bool error = false) const; CVarRef get(CVarRef k, bool error = false) const; - ArrayData *lval(CStrRef k, Variant *&ret, bool copy, bool checkExist=false); - ArrayData *lval(CVarRef k, Variant *&ret, bool copy, bool checkExist=false); + ArrayData *lval(CStrRef k, Variant *&ret, bool copy); + ArrayData *lval(CVarRef k, Variant *&ret, bool copy); ArrayData *createLvalPtr(CStrRef k, Variant *&ret, bool copy); ArrayData *getLvalPtr(CStrRef k, Variant *&ret, bool copy); ArrayData *set(CStrRef k, CVarRef v, bool copy); diff --git a/hphp/runtime/base/hphp_array.cpp b/hphp/runtime/base/hphp_array.cpp index fccdc9929..f4e16dee4 100644 --- a/hphp/runtime/base/hphp_array.cpp +++ b/hphp/runtime/base/hphp_array.cpp @@ -1112,41 +1112,14 @@ ArrayData* HphpArray::updateRef(StringData* key, CVarRef data) { return this; } -ArrayData* HphpArray::lval(int64_t k, Variant*& ret, bool copy, - bool checkExist /* = false */) { +ArrayData* HphpArray::lval(int64_t k, Variant*& ret, bool copy) { assert(checkInvariants()); - if (checkExist && copy) { - ElmInd pos = isVector() ? (size_t(k) < m_size ? ElmInd(k) : ElmIndEmpty) : - find(k); - if (pos != ElmIndEmpty) { - Elm* e = &m_data[pos]; - if (tvAsVariant(&e->data).isReferenced() || - tvAsVariant(&e->data).isObject()) { - ret = &tvAsVariant(&e->data); - return this; - } - } - } return (!copy ? this : copyImpl())->addLvalImpl(k, &ret); } -ArrayData* HphpArray::lval(StringData* key, Variant*& ret, bool copy, - bool checkExist /* = false */) { +ArrayData* HphpArray::lval(StringData* key, Variant*& ret, bool copy) { assert(checkInvariants()); - strhash_t prehash = key->hash(); - if (checkExist && copy && !isVector()) { - auto pos = find(key, prehash); - if (pos != ElmIndEmpty) { - Elm* e = &m_data[pos]; - TypedValue* tv = &e->data; - if (tvAsVariant(tv).isReferenced() || - tvAsVariant(tv).isObject()) { - ret = &tvAsVariant(tv); - return this; - } - } - } - return (!copy ? this : copyImpl())->addLvalImpl(key, prehash, &ret); + return (!copy ? this : copyImpl())->addLvalImpl(key, key->hash(), &ret); } ArrayData *HphpArray::createLvalPtr(StringData* key, Variant*& ret, bool copy) { diff --git a/hphp/runtime/base/hphp_array.h b/hphp/runtime/base/hphp_array.h index 58779c9e0..852d3589b 100644 --- a/hphp/runtime/base/hphp_array.h +++ b/hphp/runtime/base/hphp_array.h @@ -122,9 +122,8 @@ public: bool exists(const StringData* k) const; // implements ArrayData - ArrayData* lval(int64_t k, Variant*& ret, bool copy, bool checkExist=false); - ArrayData* lval(StringData* k, Variant*& ret, bool copy, - bool checkExist=false); + ArrayData* lval(int64_t k, Variant*& ret, bool copy); + ArrayData* lval(StringData* k, Variant*& ret, bool copy); ArrayData* lvalNew(Variant*& ret, bool copy); // overrides ArrayData diff --git a/hphp/runtime/base/policy_array.cpp b/hphp/runtime/base/policy_array.cpp index aaee8374b..14c5add9b 100644 --- a/hphp/runtime/base/policy_array.cpp +++ b/hphp/runtime/base/policy_array.cpp @@ -268,33 +268,15 @@ void PolicyArray::NvGetKey(const ArrayData* ad, TypedValue* out, ssize_t pos) { } template -ArrayData *PolicyArray::lvalImpl(K k, Variant*& ret, - bool copy, bool checkExist) { +ArrayData *PolicyArray::lvalImpl(K k, Variant*& ret, bool copy) { APILOG(this) << "(" << keystr(k) << ", " << ret << ", " - << copy << ", " << checkExist << ")"; + << copy << ", " << ")"; if (copy) { - return PolicyArray::copy()->lvalImpl(k, ret, false, checkExist); - } - - PosType pos = PosType::invalid; - - if (checkExist && (pos = find(k, m_size)) != PosType::invalid) { - assert(toInt(pos) < m_size); - auto& e = lval(pos); - if (e.isReferenced() || e.isObject()) { - MYLOG << (void*)this << "->lval:" << "found1"; - ret = &e; - return this; - } - } - - // Make sure the search is done. TODO: this may actually search - // twice sometimes. - if (pos == PosType::invalid) { - pos = find(k, m_size); + return PolicyArray::copy()->lvalImpl(k, ret, false); } + PosType pos = find(k, m_size); if (pos != PosType::invalid) { // found, don't overwrite anything assert(toInt(pos) <= m_size); @@ -677,7 +659,7 @@ ArrayData *PolicyArray::merge(const ArrayData *elems, bool copy) { StringData *s = key.getStringData(); Variant *p; // Andrei TODO: make sure this is the right semantics - lval(s, p, false, true); + lval(s, p, false); p->setWithRef(value); } } diff --git a/hphp/runtime/base/policy_array.h b/hphp/runtime/base/policy_array.h index 9532cdb26..b017a111b 100644 --- a/hphp/runtime/base/policy_array.h +++ b/hphp/runtime/base/policy_array.h @@ -375,7 +375,7 @@ public: private: template - ArrayData *lvalImpl(K k, Variant *&ret, bool copy, bool checkExist); + ArrayData *lvalImpl(K k, Variant *&ret, bool copy); using Store::lval; @@ -386,15 +386,13 @@ public: */ virtual ArrayData *lval(int64_t k, Variant *&ret, - bool copy, - bool checkExist = false) FOLLY_OVERRIDE { - return lvalImpl(k, ret, copy, checkExist); + bool copy) FOLLY_OVERRIDE { + return lvalImpl(k, ret, copy); } virtual ArrayData *lval(StringData* k, Variant*& ret, - bool copy, - bool checkExist = false) FOLLY_OVERRIDE { - return lvalImpl(k, ret, copy, checkExist); + bool copy) FOLLY_OVERRIDE { + return lvalImpl(k, ret, copy); } /** diff --git a/hphp/runtime/base/shared_map.cpp b/hphp/runtime/base/shared_map.cpp index e283a3f35..1439a69bd 100644 --- a/hphp/runtime/base/shared_map.cpp +++ b/hphp/runtime/base/shared_map.cpp @@ -110,14 +110,12 @@ inline ArrayData* releaseIfCopied(ArrayData* a1, ArrayData* a2) { return a2; } -ArrayData *SharedMap::lval(int64_t k, Variant *&ret, bool copy, - bool checkExist /* = false */) { +ArrayData *SharedMap::lval(int64_t k, Variant *&ret, bool copy) { ArrayData *escalated = SharedMap::escalate(); return releaseIfCopied(escalated, escalated->lval(k, ret, false)); } -ArrayData *SharedMap::lval(StringData* k, Variant *&ret, bool copy, - bool checkExist /* = false */) { +ArrayData *SharedMap::lval(StringData* k, Variant *&ret, bool copy) { ArrayData *escalated = SharedMap::escalate(); return releaseIfCopied(escalated, escalated->lval(k, ret, false)); } diff --git a/hphp/runtime/base/shared_map.h b/hphp/runtime/base/shared_map.h index b7186d3e8..5751928fd 100644 --- a/hphp/runtime/base/shared_map.h +++ b/hphp/runtime/base/shared_map.h @@ -67,10 +67,8 @@ public: bool exists(int64_t k) const; bool exists(const StringData* k) const; - virtual ArrayData *lval(int64_t k, Variant *&ret, bool copy, - bool checkExist = false); - virtual ArrayData *lval(StringData* k, Variant *&ret, bool copy, - bool checkExist = false); + virtual ArrayData *lval(int64_t k, Variant *&ret, bool copy); + virtual ArrayData *lval(StringData* k, Variant *&ret, bool copy); ArrayData *lvalNew(Variant *&ret, bool copy); static ArrayData *SetInt(ArrayData*, int64_t k, CVarRef v, bool copy); diff --git a/hphp/runtime/base/type_array.h b/hphp/runtime/base/type_array.h index 7605a52be..d5be04068 100644 --- a/hphp/runtime/base/type_array.h +++ b/hphp/runtime/base/type_array.h @@ -465,11 +465,11 @@ class Array : protected SmartPtr { template Variant &lvalAtImpl(const T &key, ACCESSPARAMS_DECL) { + assert(!(flags & AccessFlags::CheckExist)); if (!m_px) ArrayBase::operator=(ArrayData::Create()); Variant *ret = nullptr; ArrayData *escalated = - m_px->lval(key, ret, m_px->getCount() > 1, - flags & AccessFlags::CheckExist); + m_px->lval(key, ret, m_px->getCount() > 1); if (escalated != m_px) ArrayBase::operator=(escalated); assert(ret); return *ret; diff --git a/hphp/runtime/base/type_variant.cpp b/hphp/runtime/base/type_variant.cpp index 395c55b6a..89e23c24f 100644 --- a/hphp/runtime/base/type_variant.cpp +++ b/hphp/runtime/base/type_variant.cpp @@ -1142,19 +1142,17 @@ template Variant& Variant::LvalAtImpl0( Variant *self, T key, Variant *tmp, bool blackHole, ACCESSPARAMS_IMPL) { head: + assert(!(flags & AccessFlags::CheckExist)); if (self->m_type == KindOfArray) { ArrayData *arr = self->m_data.parr; ArrayData *escalated; Variant *ret = nullptr; if (LvalHelper::CheckParams && flags & AccessFlags::Key) { - escalated = arr->lval(key, ret, arr->getCount() > 1, - flags & AccessFlags::CheckExist); + escalated = arr->lval(key, ret, arr->getCount() > 1); } else { typename LvalHelper::KeyType k(ToKey(key)); if (LvalHelper::CheckKey(k)) { - escalated = - arr->lval(k, ret, arr->getCount() > 1, - flags & AccessFlags::CheckExist); + escalated = arr->lval(k, ret, arr->getCount() > 1); } else { if (blackHole) ret = &lvalBlackHole(); else ret = tmp; diff --git a/hphp/runtime/base/types.h b/hphp/runtime/base/types.h index 60f555ddc..fc01f3183 100644 --- a/hphp/runtime/base/types.h +++ b/hphp/runtime/base/types.h @@ -418,7 +418,6 @@ public: NoHipHop = 8, Error_Key = Error | Key, - CheckExist_Key = CheckExist | Key, Error_NoHipHop = Error | NoHipHop, }; static Type IsKey(bool s) { return s ? Key : None; } diff --git a/hphp/runtime/vm/name_value_table_wrapper.cpp b/hphp/runtime/vm/name_value_table_wrapper.cpp index 914a52067..d7ac45c9c 100644 --- a/hphp/runtime/vm/name_value_table_wrapper.cpp +++ b/hphp/runtime/vm/name_value_table_wrapper.cpp @@ -91,13 +91,11 @@ TypedValue* NameValueTableWrapper::NvGetInt(const ArrayData* ad, int64_t k) { return asNVTW(ad)->m_tab->lookup(String(k).get()); } -ArrayData* NameValueTableWrapper::lval(int64_t k, Variant*& ret, bool copy, - bool checkExist) { - return lval(String(k), ret, copy, checkExist); +ArrayData* NameValueTableWrapper::lval(int64_t k, Variant*& ret, bool) { + return lval(String(k), ret, false); } -ArrayData* NameValueTableWrapper::lval(StringData* k, Variant*& ret, - bool copy, bool checkExist) { +ArrayData* NameValueTableWrapper::lval(StringData* k, Variant*& ret, bool) { TypedValue* tv = m_tab->lookup(k); if (!tv) { TypedValue nulVal; diff --git a/hphp/runtime/vm/name_value_table_wrapper.h b/hphp/runtime/vm/name_value_table_wrapper.h index b67d78a43..c28aef689 100644 --- a/hphp/runtime/vm/name_value_table_wrapper.h +++ b/hphp/runtime/vm/name_value_table_wrapper.h @@ -88,10 +88,8 @@ public: // ArrayData implementation static TypedValue* NvGetInt(const ArrayData*, int64_t k); static TypedValue* NvGetStr(const ArrayData*, const StringData* k); - virtual ArrayData* lval(int64_t k, Variant*& ret, bool copy, - bool checkExist = false); - virtual ArrayData* lval(StringData* k, Variant*& ret, - bool copy, bool checkExist = false); + virtual ArrayData* lval(int64_t k, Variant*& ret, bool copy); + virtual ArrayData* lval(StringData* k, Variant*& ret, bool copy); virtual ArrayData* lvalNew(Variant*& ret, bool copy); static ArrayData* SetInt(ArrayData*, int64_t k, CVarRef v, bool copy);