From 57f1460cc7d7a357fb519e714cf006c1547b13d6 Mon Sep 17 00:00:00 2001 From: Jan Oravec Date: Wed, 22 May 2013 17:17:41 -0700 Subject: [PATCH] Use virtual method return type covariance in clone() Let clone() return pointer to the actual type instead of returning pointer to generic ObjectData. Avoids unnecessary static casts. --- hphp/runtime/ext/ext_closure.cpp | 7 ++-- hphp/runtime/ext/ext_closure.h | 2 +- hphp/runtime/ext/ext_collections.cpp | 50 +++++++++++++--------------- hphp/runtime/ext/ext_collections.h | 10 +++--- hphp/runtime/ext/ext_datetime.cpp | 21 +++++------- hphp/runtime/ext/ext_datetime.h | 6 ++-- hphp/runtime/ext/ext_domdocument.cpp | 7 ++-- hphp/runtime/ext/ext_domdocument.h | 2 +- 8 files changed, 48 insertions(+), 57 deletions(-) diff --git a/hphp/runtime/ext/ext_closure.cpp b/hphp/runtime/ext/ext_closure.cpp index 7d71b428d..1a7bb3b76 100644 --- a/hphp/runtime/ext/ext_closure.cpp +++ b/hphp/runtime/ext/ext_closure.cpp @@ -70,13 +70,12 @@ c_Closure* c_Closure::init(int numArgs, ActRec* ar, TypedValue* sp) { return this; } -ObjectData* c_Closure::clone() { - ObjectData* obj = ObjectData::clone(); - auto closure = static_cast(obj); +c_Closure* c_Closure::clone() { + auto closure = static_cast(ObjectData::clone()); closure->m_VMStatics = m_VMStatics; closure->m_thisOrClass = m_thisOrClass; closure->m_func = m_func; - return obj; + return closure; } diff --git a/hphp/runtime/ext/ext_closure.h b/hphp/runtime/ext/ext_closure.h index a6f8ca5d6..d55ae86af 100644 --- a/hphp/runtime/ext/ext_closure.h +++ b/hphp/runtime/ext/ext_closure.h @@ -43,7 +43,7 @@ class c_Closure : public ExtObjectData { // implemented by HPHP public: c_Closure *create(); public: - public: ObjectData* clone(); + public: c_Closure* clone(); c_Closure* init(int numArgs, ActRec* ar, TypedValue* sp); ObjectData* getThisOrClass() { return m_thisOrClass; } diff --git a/hphp/runtime/ext/ext_collections.cpp b/hphp/runtime/ext/ext_collections.cpp index 9736232e4..cbb10e3da 100644 --- a/hphp/runtime/ext/ext_collections.cpp +++ b/hphp/runtime/ext/ext_collections.cpp @@ -188,9 +188,8 @@ Array c_Vector::o_toArray() const { return toArrayImpl(); } -ObjectData* c_Vector::clone() { - ObjectData* obj = ObjectData::clone(); - auto target = static_cast(obj); +c_Vector* c_Vector::clone() { + auto target = static_cast(ObjectData::clone()); uint sz = m_size; TypedValue* data; target->m_capacity = target->m_size = sz; @@ -198,7 +197,7 @@ ObjectData* c_Vector::clone() { for (int i = 0; i < sz; ++i) { tvDup(&m_data[i], &data[i]); } - return obj; + return target; } Object c_Vector::t_add(CVarRef val) { @@ -1065,11 +1064,10 @@ Array c_Map::o_toArray() const { return toArrayImpl(); } -ObjectData* c_Map::clone() { - ObjectData* obj = ObjectData::clone(); - auto target = static_cast(obj); +c_Map* c_Map::clone() { + auto target = static_cast(ObjectData::clone()); - if (!m_size) return obj; + if (!m_size) return target; assert(m_nLastSlot != 0); target->m_size = m_size; @@ -1088,7 +1086,7 @@ ObjectData* c_Map::clone() { } } - return obj; + return target; } Object c_Map::t_add(CVarRef val) { @@ -1403,7 +1401,7 @@ Object c_Map::t_differencebykey(CVarRef it) { } ObjectData* obj = it.getObjectData(); c_Map* target; - Object ret = target = static_cast(clone()); + Object ret = target = clone(); if (obj->getCollectionType() == Collection::MapType) { auto mp = static_cast(obj); for (uint i = 0; i <= mp->m_nLastSlot; ++i) { @@ -2236,11 +2234,10 @@ Array c_StableMap::o_toArray() const { return toArrayImpl(); } -ObjectData* c_StableMap::clone() { - ObjectData* obj = ObjectData::clone(); - auto target = static_cast(obj); +c_StableMap* c_StableMap::clone() { + auto target = static_cast(ObjectData::clone()); - if (!m_size) return obj; + if (!m_size) return target; target->m_size = m_size; target->m_nTableSize = m_nTableSize; @@ -2273,7 +2270,7 @@ ObjectData* c_StableMap::clone() { } target->m_pListTail = last; - return obj; + return target; } Object c_StableMap::t_add(CVarRef val) { @@ -2583,7 +2580,7 @@ Object c_StableMap::t_differencebykey(CVarRef it) { } ObjectData* obj = it.getObjectData(); c_StableMap* target; - Object ret = target = static_cast(clone()); + Object ret = target = clone(); if (obj->getCollectionType() == Collection::StableMapType) { auto smp = static_cast(obj); c_StableMap::Bucket* p = smp->m_pListHead; @@ -3493,11 +3490,10 @@ Array c_Set::o_toArray() const { return toArrayImpl(); } -ObjectData* c_Set::clone() { - ObjectData* obj = ObjectData::clone(); - auto target = static_cast(obj); +c_Set* c_Set::clone() { + auto target = static_cast(ObjectData::clone()); - if (!m_size) return obj; + if (!m_size) return target; assert(m_nLastSlot != 0); target->m_size = m_size; @@ -3513,7 +3509,7 @@ ObjectData* c_Set::clone() { } } - return obj; + return target; } Object c_Set::t_add(CVarRef val) { @@ -4256,7 +4252,7 @@ Array c_Pair::o_toArray() const { return toArrayImpl(); } -ObjectData* c_Pair::clone() { +c_Pair* c_Pair::clone() { auto pair = NEWOBJ(c_Pair)(); pair->incRefCount(); pair->m_size = 2; @@ -4678,7 +4674,7 @@ ArrayData* collectionDeepCopyArray(ArrayData* arr) { } ObjectData* collectionDeepCopyVector(c_Vector* vec) { - Object o = vec = static_cast(vec->clone()); + Object o = vec = vec->clone(); size_t sz = vec->m_size; for (size_t i = 0; i < sz; ++i) { collectionDeepCopyTV(&vec->m_data[i]); @@ -4687,7 +4683,7 @@ ObjectData* collectionDeepCopyVector(c_Vector* vec) { } ObjectData* collectionDeepCopyMap(c_Map* mp) { - Object o = mp = static_cast(mp->clone()); + Object o = mp = mp->clone(); uint lastSlot = mp->m_nLastSlot; for (uint i = 0; i <= lastSlot; ++i) { c_Map::Bucket* p = mp->fetchBucket(i); @@ -4699,7 +4695,7 @@ ObjectData* collectionDeepCopyMap(c_Map* mp) { } ObjectData* collectionDeepCopyStableMap(c_StableMap* smp) { - Object o = smp = static_cast(smp->clone()); + Object o = smp = smp->clone(); for (c_StableMap::Bucket* p = smp->m_pListHead; p; p = p->pListNext) { collectionDeepCopyTV(&p->data); } @@ -4707,12 +4703,12 @@ ObjectData* collectionDeepCopyStableMap(c_StableMap* smp) { } ObjectData* collectionDeepCopySet(c_Set* st) { - Object o = st = static_cast(st->clone()); + Object o = st = st->clone(); return o.detach(); } ObjectData* collectionDeepCopyPair(c_Pair* pair) { - Object o = pair = static_cast(pair->clone()); + Object o = pair = pair->clone(); collectionDeepCopyTV(&pair->elm0); collectionDeepCopyTV(&pair->elm1); return o.detach(); diff --git a/hphp/runtime/ext/ext_collections.h b/hphp/runtime/ext/ext_collections.h index 8093a5690..d7c5f7394 100644 --- a/hphp/runtime/ext/ext_collections.h +++ b/hphp/runtime/ext/ext_collections.h @@ -142,7 +142,7 @@ class c_Vector : public ExtObjectDataFlags(obj); +c_DateTime* c_DateTime::clone() { + c_DateTime* dt = static_cast(ObjectData::clone()); dt->m_dt = m_dt->cloneDateTime(); - return obj; + return dt; } c_DateTimeZone::c_DateTimeZone(Class* cb) : @@ -209,11 +208,10 @@ Array c_DateTimeZone::ti_listidentifiers() { return TimeZone::GetNames(); } -ObjectData *c_DateTimeZone::clone() { - ObjectData *obj = ObjectData::clone(); - c_DateTimeZone *dtz = static_cast(obj); +c_DateTimeZone* c_DateTimeZone::clone() { + c_DateTimeZone* dtz = static_cast(ObjectData::clone()); dtz->m_tz = m_tz->cloneTimeZone(); - return obj; + return dtz; } c_DateInterval::c_DateInterval(Class* cb) : @@ -316,11 +314,10 @@ String c_DateInterval::t_format(CStrRef format) { return m_di->format(format); } -ObjectData *c_DateInterval::clone() { - ObjectData *obj = ObjectData::clone(); - c_DateInterval *di = static_cast(obj); +c_DateInterval* c_DateInterval::clone() { + c_DateInterval *di = static_cast(ObjectData::clone()); di->m_di = m_di->cloneDateInterval(); - return obj; + return di; } /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/ext/ext_datetime.h b/hphp/runtime/ext/ext_datetime.h index 85f6d4edb..f6d3f4aca 100644 --- a/hphp/runtime/ext/ext_datetime.h +++ b/hphp/runtime/ext/ext_datetime.h @@ -85,7 +85,7 @@ class c_DateTime : public ExtObjectData { private: SmartObject m_dt; public: - virtual ObjectData *clone(); + virtual c_DateTime* clone(); }; /////////////////////////////////////////////////////////////////////////////// @@ -141,7 +141,7 @@ class c_DateTimeZone : public ExtObjectData { private: SmartObject m_tz; public: - virtual ObjectData *clone(); + virtual c_DateTimeZone* clone(); }; /////////////////////////////////////////////////////////////////////////////// @@ -180,7 +180,7 @@ class c_DateInterval : public ExtObjectDataFlags m_di; public: - virtual ObjectData *clone(); + virtual c_DateInterval* clone(); }; diff --git a/hphp/runtime/ext/ext_domdocument.cpp b/hphp/runtime/ext/ext_domdocument.cpp index 969a94000..c68f7c9e3 100644 --- a/hphp/runtime/ext/ext_domdocument.cpp +++ b/hphp/runtime/ext/ext_domdocument.cpp @@ -2002,12 +2002,11 @@ Variant c_DOMNode::t_appendchild(CObjRef newnode) { return create_node_object(new_child, doc(), false); } -ObjectData *c_DOMNode::clone() { - ObjectData *obj = ObjectData::clone(); - c_DOMNode *node = static_cast(obj); +c_DOMNode* c_DOMNode::clone() { + c_DOMNode* node = static_cast(ObjectData::clone()); node->m_node = m_node; node->m_doc = doc(); - return obj; + return node; } Variant c_DOMNode::t_clonenode(bool deep /* = false */) { diff --git a/hphp/runtime/ext/ext_domdocument.h b/hphp/runtime/ext/ext_domdocument.h index 7a0b9462f..b08bc5334 100644 --- a/hphp/runtime/ext/ext_domdocument.h +++ b/hphp/runtime/ext/ext_domdocument.h @@ -135,7 +135,7 @@ class c_DOMNode : public ExtObjectDataFlags