Add add() API for collections

hphp/idl/collections.idl.php

@@ -321,6 +321,8 @@ DefineFunction(
   array(
     'name'   => "add",
     'flags'  =>  HasDocComment,
+    'desc'   => "Adds the specified value to the end of this Vector using ".
+                "the next available integer key.",
     'return' => array(
       'type'   => Object,
     ),
@@ -925,6 +927,24 @@ DefineFunction(
     ),
   ));
 
+DefineFunction(
+  array(
+    'name'   => "add",
+    'flags'  =>  HasDocComment,
+    'desc'   => "Adds the specified key/value Tuple to this Map. If an ".
+                "element with the same key is already present, an exception ".
+                "is thrown.",
+    'return' => array(
+      'type'   => Object,
+    ),
+    'args'   => array(
+      array(
+        'name'   => "val",
+        'type'   => Variant,
+      ),
+    ),
+  ));
+
 DefineFunction(
   array(
     'name'   => "toArray",
@@ -1474,6 +1494,24 @@ DefineFunction(
     ),
   ));
 
+DefineFunction(
+  array(
+    'name'   => "add",
+    'flags'  =>  HasDocComment,
+    'desc'   => "Adds the specified key/value Tuple to this StableMap. If an ".
+                "element with the same key is already present, an exception ".
+                "is thrown.",
+    'return' => array(
+      'type'   => Object,
+    ),
+    'args'   => array(
+      array(
+        'name'   => "val",
+        'type'   => Variant,
+      ),
+    ),
+  ));
+
 DefineFunction(
   array(
     'name'   => "toArray",

hphp/runtime/ext/ext_collections.cpp

@@ -956,6 +956,15 @@ ObjectData* c_Map::clone() {
   return obj;
 }
 
+Object c_Map::t_add(CVarRef val) {
+  TypedValue* tv = (TypedValue*)(&val);
+  if (UNLIKELY(tv->m_type == KindOfRef)) {
+    tv = tv->m_data.pref->tv();
+  }
+  add(tv);
+  return this;
+}
+
 Object c_Map::t_clear() {
   deleteBuckets();
   freeData();
@@ -1337,6 +1346,32 @@ void c_Map::throwOOB(StringData* key) {
   throwStrOOB(key);
 }
 
+void c_Map::add(TypedValue* val) {
+  if (UNLIKELY(val->m_type != KindOfObject ||
+               !val->m_data.pobj->instanceof(c_Tuple::s_cls))) {
+    Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+      "Parameter must be an instance of Tuple"));
+    throw e;
+  }
+  auto tup = static_cast<c_Tuple*>(val->m_data.pobj);
+  if (UNLIKELY(tup->t_count() != 2)) {
+    Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+      "Expected Tuple containing exactly two elements"));
+    throw e;
+  }
+  TypedValue* tvKey = &tup->getData()[0];
+  TypedValue* tvValue = &tup->getData()[1];
+  assert(tvKey->m_type != KindOfRef);
+  assert(tvValue->m_type != KindOfRef);
+  if (tvKey->m_type == KindOfInt64) {
+    updateImpl<true>(tvKey->m_data.num, tvValue);
+  } else if (IS_STRING_TYPE(tvKey->m_type)) {
+    updateImpl<true>(tvKey->m_data.pstr, tvValue);
+  } else {
+    throwBadKeyType();
+  }
+}
+
 #define STRING_HASH(x)   (int32_t(x) | 0x80000000)
 
 bool inline hitStringKey(const c_Map::Bucket* p, const char* k,
@@ -1448,11 +1483,17 @@ c_Map::Bucket* c_Map::findForNewInsert(size_t h0) const {
 #undef FIND_BODY
 #undef FIND_FOR_INSERT_BODY
 
-bool c_Map::update(int64_t h, TypedValue* data) {
+template <bool throwIfExists>
+bool c_Map::updateImpl(int64_t h, TypedValue* data) {
   assert(data->m_type != KindOfRef);
   Bucket* p = findForInsert(h);
   assert(p);
   if (p->validValue()) {
+    if (throwIfExists) {
+      Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+        "An element with the same key already exists"));
+      throw e;
+    }
     tvRefcountedIncRef(data);
     tvRefcountedDecRef(&p->data);
     p->data.m_data.num = data->m_data.num;
@@ -1475,11 +1516,17 @@ bool c_Map::update(int64_t h, TypedValue* data) {
   return true;
 }
 
-bool c_Map::update(StringData *key, TypedValue* data) {
+template <bool throwIfExists>
+bool c_Map::updateImpl(StringData *key, TypedValue* data) {
   strhash_t h = key->hash();
   Bucket* p = findForInsert(key->data(), key->size(), h);
   assert(p);
   if (p->validValue()) {
+    if (throwIfExists) {
+      Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+        "An element with the same key already exists"));
+      throw e;
+    }
     tvRefcountedIncRef(data);
     tvRefcountedDecRef(&p->data);
     p->data.m_data.num = data->m_data.num;
@@ -1698,9 +1745,9 @@ bool c_Map::OffsetContains(ObjectData* obj, TypedValue* key) {
 }
 
 void c_Map::OffsetAppend(ObjectData* obj, TypedValue* val) {
-  Object e(SystemLib::AllocRuntimeExceptionObject(
-    "[] operator not supported for Maps"));
-  throw e;
+  assert(val->m_type != KindOfRef);
+  auto mp = static_cast<c_Map*>(obj);
+  mp->add(val);
 }
 
 void c_Map::OffsetUnset(ObjectData* obj, TypedValue* key) {
@@ -1930,6 +1977,15 @@ ObjectData* c_StableMap::clone() {
   return obj;
 }
 
+Object c_StableMap::t_add(CVarRef val) {
+  TypedValue* tv = (TypedValue*)(&val);
+  if (UNLIKELY(tv->m_type == KindOfRef)) {
+    tv = tv->m_data.pref->tv();
+  }
+  add(tv);
+  return this;
+}
+
 Object c_StableMap::t_clear() {
   deleteBuckets();
   freeData();
@@ -2303,6 +2359,32 @@ void c_StableMap::throwOOB(StringData* key) {
   throwStrOOB(key);
 }
 
+void c_StableMap::add(TypedValue* val) {
+  if (UNLIKELY(val->m_type != KindOfObject ||
+               !val->m_data.pobj->instanceof(c_Tuple::s_cls))) {
+    Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+      "Parameter must be an instance of Tuple"));
+    throw e;
+  }
+  auto tup = static_cast<c_Tuple*>(val->m_data.pobj);
+  if (UNLIKELY(tup->t_count() != 2)) {
+    Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+      "Expected Tuple containing exactly two elements"));
+    throw e;
+  }
+  TypedValue* tvKey = &tup->getData()[0];
+  TypedValue* tvValue = &tup->getData()[1];
+  assert(tvKey->m_type != KindOfRef);
+  assert(tvValue->m_type != KindOfRef);
+  if (tvKey->m_type == KindOfInt64) {
+    updateImpl<true>(tvKey->m_data.num, tvValue);
+  } else if (IS_STRING_TYPE(tvKey->m_type)) {
+    updateImpl<true>(tvKey->m_data.pstr, tvValue);
+  } else {
+    throwBadKeyType();
+  }
+}
+
 bool inline sm_hit_string_key(const c_StableMap::Bucket* p,
                               const char* k, int len, int32_t hash) ALWAYS_INLINE;
 bool inline sm_hit_string_key(const c_StableMap::Bucket* p,
@@ -2358,9 +2440,15 @@ c_StableMap::Bucket** c_StableMap::findForErase(const char* k, int len,
   return NULL;
 }
 
-bool c_StableMap::update(int64_t h, TypedValue* data) {
+template <bool throwIfExists>
+bool c_StableMap::updateImpl(int64_t h, TypedValue* data) {
   Bucket* p = find(h);
   if (p) {
+    if (throwIfExists) {
+      Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+        "An element with the same key already exists"));
+      throw e;
+    }
     tvRefcountedIncRef(data);
     tvRefcountedDecRef(&p->data);
     p->data.m_data.num = data->m_data.num;
@@ -2380,10 +2468,16 @@ bool c_StableMap::update(int64_t h, TypedValue* data) {
   return true;
 }
 
-bool c_StableMap::update(StringData *key, TypedValue* data) {
+template <bool throwIfExists>
+bool c_StableMap::updateImpl(StringData *key, TypedValue* data) {
   strhash_t h = key->hash();
   Bucket* p = find(key->data(), key->size(), h);
   if (p) {
+    if (throwIfExists) {
+      Object e(SystemLib::AllocInvalidArgumentExceptionObject(
+        "An element with the same key already exists"));
+      throw e;
+    }
     tvRefcountedIncRef(data);
     tvRefcountedDecRef(&p->data);
     p->data.m_data.num = data->m_data.num;
@@ -2754,9 +2848,9 @@ bool c_StableMap::OffsetContains(ObjectData* obj, TypedValue* key) {
 }
 
 void c_StableMap::OffsetAppend(ObjectData* obj, TypedValue* val) {
-  Object e(SystemLib::AllocRuntimeExceptionObject(
-    "[] operator not supported for StableMaps"));
-  throw e;
+  assert(val->m_type != KindOfRef);
+  auto smp = static_cast<c_StableMap*>(obj);
+  smp->add(val);
 }
 
 void c_StableMap::OffsetUnset(ObjectData* obj, TypedValue* key) {

hphp/runtime/ext/ext_collections.ext_hhvm.cpp

@@ -2082,6 +2082,45 @@ TypedValue* tg_3Map_discard(HPHP::VM::ActRec *ar) {
   return &ar->m_r;
 }
 
+/*
+HPHP::Object HPHP::c_Map::t_add(HPHP::Variant const&)
+_ZN4HPHP5c_Map5t_addERKNS_7VariantE
+
+(return value) => rax
+_rv => rdi
+this_ => rsi
+val => rdx
+*/
+
+Value* th_3Map_add(Value* _rv, ObjectData* this_, TypedValue* val) asm("_ZN4HPHP5c_Map5t_addERKNS_7VariantE");
+
+TypedValue* tg_3Map_add(HPHP::VM::ActRec *ar) {
+    TypedValue rv;
+    int64_t count = ar->numArgs();
+    TypedValue* args UNUSED = ((TypedValue*)ar) - 1;
+    ObjectData* this_ = (ar->hasThis() ? ar->getThis() : NULL);
+    if (this_) {
+      if (count == 1LL) {
+        rv.m_type = KindOfObject;
+        th_3Map_add((&rv.m_data), (this_), (args-0));
+        if (rv.m_data.num == 0LL) rv.m_type = KindOfNull;
+        frame_free_locals_inl(ar, 1);
+        memcpy(&ar->m_r, &rv, sizeof(TypedValue));
+        return &ar->m_r;
+      } else {
+        throw_wrong_arguments_nr("Map::add", count, 1, 1, 1);
+      }
+    } else {
+      throw_instance_method_fatal("Map::add");
+    }
+    rv.m_data.num = 0LL;
+    rv.m_type = KindOfNull;
+    frame_free_locals_inl(ar, 1);
+    memcpy(&ar->m_r, &rv, sizeof(TypedValue));
+    return &ar->m_r;
+  return &ar->m_r;
+}
+
 /*
 HPHP::Array HPHP::c_Map::t_toarray()
 _ZN4HPHP5c_Map9t_toarrayEv
@@ -3528,6 +3567,45 @@ TypedValue* tg_9StableMap_discard(HPHP::VM::ActRec *ar) {
   return &ar->m_r;
 }
 
+/*
+HPHP::Object HPHP::c_StableMap::t_add(HPHP::Variant const&)
+_ZN4HPHP11c_StableMap5t_addERKNS_7VariantE
+
+(return value) => rax
+_rv => rdi
+this_ => rsi
+val => rdx
+*/
+
+Value* th_9StableMap_add(Value* _rv, ObjectData* this_, TypedValue* val) asm("_ZN4HPHP11c_StableMap5t_addERKNS_7VariantE");
+
+TypedValue* tg_9StableMap_add(HPHP::VM::ActRec *ar) {
+    TypedValue rv;
+    int64_t count = ar->numArgs();
+    TypedValue* args UNUSED = ((TypedValue*)ar) - 1;
+    ObjectData* this_ = (ar->hasThis() ? ar->getThis() : NULL);
+    if (this_) {
+      if (count == 1LL) {
+        rv.m_type = KindOfObject;
+        th_9StableMap_add((&rv.m_data), (this_), (args-0));
+        if (rv.m_data.num == 0LL) rv.m_type = KindOfNull;
+        frame_free_locals_inl(ar, 1);
+        memcpy(&ar->m_r, &rv, sizeof(TypedValue));
+        return &ar->m_r;
+      } else {
+        throw_wrong_arguments_nr("StableMap::add", count, 1, 1, 1);
+      }
+    } else {
+      throw_instance_method_fatal("StableMap::add");
+    }
+    rv.m_data.num = 0LL;
+    rv.m_type = KindOfNull;
+    frame_free_locals_inl(ar, 1);
+    memcpy(&ar->m_r, &rv, sizeof(TypedValue));
+    return &ar->m_r;
+  return &ar->m_r;
+}
+
 /*
 HPHP::Array HPHP::c_StableMap::t_toarray()
 _ZN4HPHP11c_StableMap9t_toarrayEv

hphp/runtime/ext/ext_collections.h

@@ -227,6 +227,7 @@ class c_Map : public ExtObjectDataFlags<ObjectData::MapAttrInit|
   public: ~c_Map();
   public: void freeData();
   public: void t___construct(CVarRef iterable = null_variant);
+  public: Object t_add(CVarRef val);
   public: Object t_clear();
   public: bool t_isempty();
   public: int64_t t_count();
@@ -303,6 +304,7 @@ class c_Map : public ExtObjectDataFlags<ObjectData::MapAttrInit|
     assert(val->m_type != KindOfRef);
     update(key, val);
   }
+  public: void add(TypedValue* val);
   public: void remove(int64_t key) {
     ++m_versionNumber;
     erase(find(key));
@@ -457,8 +459,16 @@ private:
   Bucket* findForInsert(const char* k, int len, strhash_t prehash) const;
   Bucket* findForNewInsert(size_t h0) const;
 
-  bool update(int64_t h, TypedValue* data);
-  bool update(StringData* key, TypedValue* data);
+  template <bool throwIfExists=false>
+  bool updateImpl(int64_t h, TypedValue* data);
+  template <bool throwIfExists=false>
+  bool updateImpl(StringData* key, TypedValue* data);
+  bool update(int64_t h, TypedValue* data) {
+    return updateImpl<>(h, data);
+  }
+  bool update(StringData* key, TypedValue* data) {
+    return updateImpl<>(key, data);
+  }
   void erase(Bucket* prev);
 
   void resize();
@@ -523,6 +533,7 @@ class c_StableMap : public ExtObjectDataFlags<ObjectData::StableMapAttrInit|
   public: ~c_StableMap();
   public: void freeData();
   public: void t___construct(CVarRef iterable = null_variant);
+  public: Object t_add(CVarRef val);
   public: Object t_clear();
   public: bool t_isempty();
   public: int64_t t_count();
@@ -597,6 +608,7 @@ class c_StableMap : public ExtObjectDataFlags<ObjectData::StableMapAttrInit|
   public: void set(StringData* key, TypedValue* val) {
     update(key, val);
   }
+  public: void add(TypedValue* val);
   public: void remove(int64_t key) {
     ++m_versionNumber;
     erase(findForErase(key));
@@ -713,8 +725,16 @@ private:
   Bucket** findForErase(int64_t h) const;
   Bucket** findForErase(const char* k, int len, strhash_t prehash) const;
 
-  bool update(int64_t h, TypedValue* data);
-  bool update(StringData* key, TypedValue* data);
+  template <bool throwIfExists=false>
+  bool updateImpl(int64_t h, TypedValue* data);
+  template <bool throwIfExists=false>
+  bool updateImpl(StringData* key, TypedValue* data);
+  bool update(int64_t h, TypedValue* data) {
+    return updateImpl<>(h, data);
+  }
+  bool update(StringData* key, TypedValue* data) {
+    return updateImpl<>(key, data);
+  }
   void erase(Bucket** prev);
 
   void resize();

hphp/runtime/ext_hhvm/ext_hhvm_infotabs.cpp

@@ -2330,6 +2330,7 @@ TypedValue* tg_3Map_containsKey(VM::ActRec *ar);
 TypedValue* tg_3Map_remove(VM::ActRec *ar);
 TypedValue* tg_3Map_removeKey(VM::ActRec *ar);
 TypedValue* tg_3Map_discard(VM::ActRec *ar);
+TypedValue* tg_3Map_add(VM::ActRec *ar);
 TypedValue* tg_3Map_toArray(VM::ActRec *ar);
 TypedValue* tg_3Map_copyAsArray(VM::ActRec *ar);
 TypedValue* tg_3Map_toKeysArray(VM::ActRec *ar);
@@ -2370,6 +2371,7 @@ TypedValue* tg_9StableMap_containsKey(VM::ActRec *ar);
 TypedValue* tg_9StableMap_remove(VM::ActRec *ar);
 TypedValue* tg_9StableMap_removeKey(VM::ActRec *ar);
 TypedValue* tg_9StableMap_discard(VM::ActRec *ar);
+TypedValue* tg_9StableMap_add(VM::ActRec *ar);
 TypedValue* tg_9StableMap_toArray(VM::ActRec *ar);
 TypedValue* tg_9StableMap_copyAsArray(VM::ActRec *ar);
 TypedValue* tg_9StableMap_toKeysArray(VM::ActRec *ar);
@@ -5375,7 +5377,7 @@ static const HhbcExtMethodInfo hhbc_ext_methods_VectorIterator[] = {
   { "rewind", tg_14VectorIterator_rewind }
 };
 
-static const long long hhbc_ext_method_count_Map = 32;
+static const long long hhbc_ext_method_count_Map = 33;
 static const HhbcExtMethodInfo hhbc_ext_methods_Map[] = {
   { "__construct", tg_3Map___construct },
   { "isEmpty", tg_3Map_isEmpty },
@@ -5392,6 +5394,7 @@ static const HhbcExtMethodInfo hhbc_ext_methods_Map[] = {
   { "remove", tg_3Map_remove },
   { "removeKey", tg_3Map_removeKey },
   { "discard", tg_3Map_discard },
+  { "add", tg_3Map_add },
   { "toArray", tg_3Map_toArray },
   { "copyAsArray", tg_3Map_copyAsArray },
   { "toKeysArray", tg_3Map_toKeysArray },
@@ -5421,7 +5424,7 @@ static const HhbcExtMethodInfo hhbc_ext_methods_MapIterator[] = {
   { "rewind", tg_11MapIterator_rewind }
 };
 
-static const long long hhbc_ext_method_count_StableMap = 32;
+static const long long hhbc_ext_method_count_StableMap = 33;
 static const HhbcExtMethodInfo hhbc_ext_methods_StableMap[] = {
   { "__construct", tg_9StableMap___construct },
   { "isEmpty", tg_9StableMap_isEmpty },
@@ -5438,6 +5441,7 @@ static const HhbcExtMethodInfo hhbc_ext_methods_StableMap[] = {
   { "remove", tg_9StableMap_remove },
   { "removeKey", tg_9StableMap_removeKey },
   { "discard", tg_9StableMap_discard },
+  { "add", tg_9StableMap_add },
   { "toArray", tg_9StableMap_toArray },
   { "copyAsArray", tg_9StableMap_copyAsArray },
   { "toKeysArray", tg_9StableMap_toKeysArray },

hphp/system/class_map.cpp

@@ -22464,7 +22464,7 @@ const char *g_class_map[] = {
   NULL,
   NULL,
   (const char *)0x10006040, "add", "", (const char*)0, (const char*)0,
-  "/**\n * ( excerpt from http://php.net/manual/en/vector.add.php )\n *\n *\n * @val        mixed\n *\n * @return     object\n */",
+  "/**\n * ( excerpt from http://php.net/manual/en/vector.add.php )\n *\n * Adds the specified value to the end of this Vector using the next\n * available integer key.\n *\n * @val        mixed\n *\n * @return     object\n */",
   (const char *)0x40, (const char *)0x2000, "val", "", (const char *)0xffffffff, "", "", NULL,
   NULL,
   NULL,
@@ -22708,6 +22708,12 @@ const char *g_class_map[] = {
   NULL,
   NULL,
   NULL,
+  (const char *)0x10006040, "add", "", (const char*)0, (const char*)0,
+  "/**\n * ( excerpt from http://php.net/manual/en/map.add.php )\n *\n * Adds the specified key/value Tuple to this Map. If an element with the\n * same key is already present, an exception is thrown.\n *\n * @val        mixed\n *\n * @return     object\n */",
+  (const char *)0x40, (const char *)0x2000, "val", "", (const char *)0xffffffff, "", "", NULL,
+  NULL,
+  NULL,
+  NULL,
   (const char *)0x10006040, "toArray", "", (const char*)0, (const char*)0,
   "/**\n * ( excerpt from http://php.net/manual/en/map.toarray.php )\n *\n * Returns an array built from the keys and values from this Map.\n *\n * @return     map\n */",
   (const char *)0x20, NULL,
@@ -22935,6 +22941,12 @@ const char *g_class_map[] = {
   NULL,
   NULL,
   NULL,
+  (const char *)0x10006040, "add", "", (const char*)0, (const char*)0,
+  "/**\n * ( excerpt from http://php.net/manual/en/stablemap.add.php )\n *\n * Adds the specified key/value Tuple to this StableMap. If an element\n * with the same key is already present, an exception is thrown.\n *\n * @val        mixed\n *\n * @return     object\n */",
+  (const char *)0x40, (const char *)0x2000, "val", "", (const char *)0xffffffff, "", "", NULL,
+  NULL,
+  NULL,
+  NULL,
   (const char *)0x10006040, "toArray", "", (const char*)0, (const char*)0,
   "/**\n * ( excerpt from http://php.net/manual/en/stablemap.toarray.php )\n *\n * Returns an array built from the keys and values from this StableMap.\n *\n * @return     map\n */",
   (const char *)0x20, NULL,

hphp/test/test_code_run.cpp

@@ -10084,6 +10084,42 @@ bool TestCodeRun::TestCollectionClasses() {
         "bool(false)\n"
         );
 
+  MVCRO("<?php\n"
+        "function f() {\n"
+        "  $mp1 = StableMap {'a' => 1, 2 => 'b', 'c' => array()};\n"
+        "  $mp2 = StableMap {};\n"
+        "  $mp3 = StableMap {};\n"
+        "  foreach ($mp1->items() as $t) {\n"
+        "    $mp2->add($t);\n"
+        "  }\n"
+        "  var_dump($mp2);\n"
+        "  foreach ($mp1->items() as $t) {\n"
+        "    $mp3[] = $t;\n"
+        "  }\n"
+        "  var_dump($mp3);\n"
+        "}\n"
+        "f();\n"
+        ,
+        "object(StableMap)#2 (3) {\n"
+        "  [\"a\"]=>\n"
+        "  int(1)\n"
+        "  [2]=>\n"
+        "  string(1) \"b\"\n"
+        "  [\"c\"]=>\n"
+        "  array(0) {\n"
+        "  }\n"
+        "}\n"
+        "object(StableMap)#3 (3) {\n"
+        "  [\"a\"]=>\n"
+        "  int(1)\n"
+        "  [2]=>\n"
+        "  string(1) \"b\"\n"
+        "  [\"c\"]=>\n"
+        "  array(0) {\n"
+        "  }\n"
+        "}\n"
+        );
+
   return true;
 }