use heterogeneous lookup and mutation in folly::dynamic

Summary:
This diff allows `folly::dynamic` to lookup and mutate using any type that is
convertible to `StringPiece` (i.e., `const char*`, `std::string`, `fbstring`)
without having to construct a `dynamic` object (which requires copying the
string).

Previously (D8299863), we SFINAE'd and took `StringPiece` by reference. This
caused linker errors for non-member constexpr string constants like the
following which are odr-used.
  struct S {
    static constexpr auto kXXX = "xxx";
  };
  d.at(kXXX); // linker error
Hence in this diff, we template on types that are convertible to dynamic but
NOT to StringPiece, and add a separate overload which takes StringPiece by
value.

Reviewed By: yfeldblum

Differential Revision: D9540388

fbshipit-source-id: 529de31585200947b8b92fcf3126442a8ed51a73

folly/dynamic-inl.h

@@ -24,6 +24,59 @@
 #include <folly/detail/Iterators.h>
 #include <folly/lang/Exception.h>
 
+namespace folly {
+namespace detail {
+struct DynamicHasher {
+  using is_transparent = void;
+
+  size_t operator()(dynamic const& d) const {
+    return d.hash();
+  }
+
+  template <typename T>
+  std::enable_if_t<std::is_convertible<T, StringPiece>::value, size_t>
+  operator()(T const& val) const {
+    // keep consistent with dynamic::hash() for strings
+    return Hash()(static_cast<StringPiece>(val));
+  }
+};
+
+struct DynamicKeyEqual {
+  using is_transparent = void;
+
+  bool operator()(const dynamic& lhs, const dynamic& rhs) const {
+    return std::equal_to<dynamic>()(lhs, rhs);
+  }
+
+  // Dynamic objects contains a map<dynamic, dynamic>. At least one of the
+  // operands should be a dynamic. Hence, an operator() where both operands are
+  // convertible to StringPiece is unnecessary.
+  template <typename A, typename B>
+  std::enable_if_t<
+      std::is_convertible<A, StringPiece>::value &&
+          std::is_convertible<B, StringPiece>::value,
+      bool>
+  operator()(A const& lhs, B const& rhs) const = delete;
+
+  template <typename A>
+  std::enable_if_t<std::is_convertible<A, StringPiece>::value, bool> operator()(
+      A const& lhs,
+      dynamic const& rhs) const {
+    return FOLLY_LIKELY(rhs.type() == dynamic::Type::STRING) &&
+        std::equal_to<StringPiece>()(lhs, rhs.stringPiece());
+  }
+
+  template <typename B>
+  std::enable_if_t<std::is_convertible<B, StringPiece>::value, bool> operator()(
+      dynamic const& lhs,
+      B const& rhs) const {
+    return FOLLY_LIKELY(lhs.type() == dynamic::Type::STRING) &&
+        std::equal_to<StringPiece>()(lhs.stringPiece(), rhs);
+  }
+};
+} // namespace detail
+} // namespace folly
+
 //////////////////////////////////////////////////////////////////////
 
 namespace std {
@@ -140,7 +193,11 @@ dynamic numericOp(dynamic const& a, dynamic const& b) {
  * Note: Later we may add separate order tracking here (a multi-index
  * type of thing.)
  */
-struct dynamic::ObjectImpl : F14NodeMap<dynamic, dynamic> {};
+struct dynamic::ObjectImpl : F14NodeMap<
+                                 dynamic,
+                                 dynamic,
+                                 detail::DynamicHasher,
+                                 detail::DynamicKeyEqual> {};
 
 //////////////////////////////////////////////////////////////////////
 
@@ -575,29 +632,56 @@ inline dynamic& dynamic::operator--() {
   return *this;
 }
 
-inline dynamic const& dynamic::operator[](dynamic const& idx) const& {
-  return at(idx);
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic const&> dynamic::operator[](
+    K&& idx) const& {
+  return at(std::forward<K>(idx));
+}
+
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic&> dynamic::operator[](
+    K&& idx) & {
+  if (!isObject() && !isArray()) {
+    throw_exception<TypeError>("object/array", type());
+  }
+  if (isArray()) {
+    return at(std::forward<K>(idx));
+  }
+  auto& obj = get<ObjectImpl>();
+  auto ret = obj.emplace(std::forward<K>(idx), nullptr);
+  return ret.first->second;
 }
 
-inline dynamic&& dynamic::operator[](dynamic const& idx) && {
-  return std::move((*this)[idx]);
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic&&> dynamic::operator[](
+    K&& idx) && {
+  return std::move((*this)[std::forward<K>(idx)]);
 }
 
-template <class K, class V> inline dynamic& dynamic::setDefault(K&& k, V&& v) {
+inline dynamic const& dynamic::operator[](StringPiece k) const& {
+  return at(k);
+}
+
+inline dynamic&& dynamic::operator[](StringPiece k) && {
+  return std::move((*this)[k]);
+}
+
+template <class K, class V>
+inline dynamic& dynamic::setDefault(K&& k, V&& v) {
   auto& obj = get<ObjectImpl>();
-  return obj.insert(std::make_pair(std::forward<K>(k),
-                                   std::forward<V>(v))).first->second;
+  return obj.emplace(std::forward<K>(k), std::forward<V>(v)).first->second;
 }
 
-template <class K> inline dynamic& dynamic::setDefault(K&& k, dynamic&& v) {
+template <class K>
+inline dynamic& dynamic::setDefault(K&& k, dynamic&& v) {
   auto& obj = get<ObjectImpl>();
-  return obj.insert(std::make_pair(std::forward<K>(k),
-                                   std::move(v))).first->second;
+  return obj.emplace(std::forward<K>(k), std::move(v)).first->second;
 }
 
-template <class K> inline dynamic& dynamic::setDefault(K&& k, const dynamic& v) {
+template <class K>
+inline dynamic& dynamic::setDefault(K&& k, const dynamic& v) {
   auto& obj = get<ObjectImpl>();
-  return obj.insert(std::make_pair(std::forward<K>(k), v)).first->second;
+  return obj.emplace(std::forward<K>(k), v).first->second;
 }
 
 inline dynamic* dynamic::get_ptr(dynamic const& idx) & {
@@ -609,11 +693,27 @@ inline dynamic* dynamic::get_ptr(json_pointer const& jsonPtr) & {
       const_cast<dynamic const*>(this)->get_ptr(jsonPtr));
 }
 
-inline dynamic& dynamic::at(dynamic const& idx) & {
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic const&> dynamic::at(
+    K&& k) const& {
+  return atImpl(std::forward<K>(k));
+}
+
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic&> dynamic::at(K&& idx) & {
+  return const_cast<dynamic&>(const_cast<dynamic const*>(this)->at(idx));
+}
+
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic&&> dynamic::at(K&& idx) && {
+  return std::move(at(idx));
+}
+
+inline dynamic& dynamic::at(StringPiece idx) & {
   return const_cast<dynamic&>(const_cast<dynamic const*>(this)->at(idx));
 }
 
-inline dynamic&& dynamic::at(dynamic const& idx) && {
+inline dynamic&& dynamic::at(StringPiece idx) && {
   return std::move(at(idx));
 }
 
@@ -624,21 +724,40 @@ inline bool dynamic::empty() const {
   return !size();
 }
 
-inline std::size_t dynamic::count(dynamic const& key) const {
-  return find(key) != items().end() ? 1u : 0u;
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic::const_item_iterator>
+dynamic::find(K&& key) const {
+  return get<ObjectImpl>().find(std::forward<K>(key));
+}
+
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, dynamic::item_iterator>
+dynamic::find(K&& key) {
+  return get<ObjectImpl>().find(std::forward<K>(key));
 }
 
-inline dynamic::const_item_iterator dynamic::find(dynamic const& key) const {
+inline dynamic::const_item_iterator dynamic::find(StringPiece key) const {
   return get<ObjectImpl>().find(key);
 }
-inline dynamic::item_iterator dynamic::find(dynamic const& key) {
+
+inline dynamic::item_iterator dynamic::find(StringPiece key) {
   return get<ObjectImpl>().find(key);
 }
 
-template <class K, class V> inline void dynamic::insert(K&& key, V&& val) {
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, std::size_t> dynamic::count(
+    K&& key) const {
+  return find(std::forward<K>(key)) != items().end() ? 1u : 0u;
+}
+
+inline std::size_t dynamic::count(StringPiece key) const {
+  return find(key) != items().end() ? 1u : 0u;
+}
+
+template <class K, class V>
+inline void dynamic::insert(K&& key, V&& val) {
   auto& obj = get<ObjectImpl>();
-  auto rv = obj.insert({ std::forward<K>(key), nullptr });
-  rv.first->second = std::forward<V>(val);
+  obj[std::forward<K>(key)] = std::forward<V>(val);
 }
 
 inline void dynamic::update(const dynamic& mergeObj) {
@@ -699,7 +818,13 @@ inline dynamic dynamic::merge(
   return ret;
 }
 
-inline std::size_t dynamic::erase(dynamic const& key) {
+template <typename K>
+dynamic::IfIsNonStringDynamicConvertible<K, std::size_t> dynamic::erase(
+    K&& key) {
+  auto& obj = get<ObjectImpl>();
+  return obj.erase(std::forward<K>(key));
+}
+inline std::size_t dynamic::erase(StringPiece key) {
   auto& obj = get<ObjectImpl>();
   return obj.erase(key);
 }

folly/dynamic.cpp

@@ -167,15 +167,43 @@ dynamic& dynamic::operator=(dynamic&& o) noexcept {
   return *this;
 }
 
-dynamic& dynamic::operator[](dynamic const& k) & {
-  if (!isObject() && !isArray()) {
+dynamic const& dynamic::atImpl(dynamic const& idx) const& {
+  if (auto* parray = get_nothrow<Array>()) {
+    if (!idx.isInt()) {
+      throw_exception<TypeError>("int64", idx.type());
+    }
+    if (idx < 0 || idx >= parray->size()) {
+      throw_exception<std::out_of_range>("out of range in dynamic array");
+    }
+    return (*parray)[size_t(idx.asInt())];
+  } else if (auto* pobject = get_nothrow<ObjectImpl>()) {
+    auto it = pobject->find(idx);
+    if (it == pobject->end()) {
+      throw_exception<std::out_of_range>(
+          sformat("couldn't find key {} in dynamic object", idx.asString()));
+    }
+    return it->second;
+  } else {
     throw_exception<TypeError>("object/array", type());
   }
-  if (isArray()) {
-    return at(k);
+}
+
+dynamic const& dynamic::at(StringPiece idx) const& {
+  auto* pobject = get_nothrow<ObjectImpl>();
+  if (!pobject) {
+    throw_exception<TypeError>("object/array", type());
   }
+  auto it = pobject->find(idx);
+  if (it == pobject->end()) {
+    throw_exception<std::out_of_range>(
+        sformat("couldn't find key {} in dynamic object", idx));
+  }
+  return it->second;
+}
+
+dynamic& dynamic::operator[](StringPiece k) & {
   auto& obj = get<ObjectImpl>();
-  auto ret = obj.insert({k, nullptr});
+  auto ret = obj.emplace(k, nullptr);
   return ret.first->second;
 }
 
@@ -233,29 +261,6 @@ const dynamic* dynamic::get_ptr(dynamic const& idx) const& {
   }
 }
 
-dynamic const& dynamic::at(dynamic const& idx) const& {
-  if (auto* parray = get_nothrow<Array>()) {
-    if (!idx.isInt()) {
-      throw_exception<TypeError>("int64", idx.type());
-    }
-    if (idx < 0 || idx >= parray->size()) {
-      throw_exception<std::out_of_range>("out of range in dynamic array");
-    }
-    return (*parray)[size_t(idx.asInt())];
-  } else if (auto* pobject = get_nothrow<ObjectImpl>()) {
-    auto it = pobject->find(idx);
-    if (it == pobject->end()) {
-      invoke_noreturn_cold([&] {
-        throw_exception<std::out_of_range>(
-            sformat("couldn't find key {} in dynamic object", idx.asString()));
-      });
-    }
-    return it->second;
-  } else {
-    throw_exception<TypeError>("object/array", type());
-  }
-}
-
 std::size_t dynamic::size() const {
   if (auto* ar = get_nothrow<Array>()) {
     return ar->size();
@@ -301,6 +306,7 @@ std::size_t dynamic::hash() const {
   case BOOL:
     return std::hash<bool>()(getBool());
   case STRING:
+    // keep consistent with detail::DynamicHasher
     return Hash()(getString());
   }
   assume_unreachable();

folly/dynamic.h

@@ -358,6 +358,16 @@ struct dynamic : private boost::operators<dynamic> {
    */
   template <class T> struct IterableProxy;
 
+  /*
+   * Helper for heterogeneous lookup and mutation on objects: at(), find(),
+   * count(), erase(), operator[]
+   */
+  template <typename K, typename T>
+  using IfIsNonStringDynamicConvertible = std::enable_if_t<
+      !std::is_convertible<K, StringPiece>::value &&
+          std::is_convertible<K, dynamic>::value,
+      T>;
+
  public:
   /*
    * You can iterate over the keys, values, or items (std::pair of key and
@@ -376,14 +386,22 @@ struct dynamic : private boost::operators<dynamic> {
    * Returns: items().end() if the key is not present, or a
    * const_item_iterator pointing to the item.
    */
-  const_item_iterator find(dynamic const&) const;
-  item_iterator find(dynamic const&);
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, const_item_iterator> find(K&&) const;
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, item_iterator> find(K&&);
+
+  const_item_iterator find(StringPiece) const;
+  item_iterator find(StringPiece);
 
   /*
    * If this is an object, returns whether it contains a field with
    * the given name.  Otherwise throws TypeError.
    */
-  std::size_t count(dynamic const&) const;
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, std::size_t> count(K&&) const;
+
+  std::size_t count(StringPiece) const;
 
   /*
    * For objects or arrays, provides access to sub-fields by index or
@@ -393,9 +411,20 @@ struct dynamic : private boost::operators<dynamic> {
    * will throw a TypeError.  Using an index that is out of range or
    * object-element that's not present throws std::out_of_range.
    */
-  dynamic const& at(dynamic const&) const&;
-  dynamic&       at(dynamic const&) &;
-  dynamic&&      at(dynamic const&) &&;
+ private:
+  dynamic const& atImpl(dynamic const&) const&;
+
+ public:
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, dynamic const&> at(K&&) const&;
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, dynamic&> at(K&&) &;
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, dynamic&&> at(K&&) &&;
+
+  dynamic const& at(StringPiece) const&;
+  dynamic& at(StringPiece) &;
+  dynamic&& at(StringPiece) &&;
 
   /*
    * Locate element using JSON pointer, per RFC 6901. Returns nullptr if
@@ -436,9 +465,16 @@ struct dynamic : private boost::operators<dynamic> {
    * These functions do not invalidate iterators except when a null value
    * is inserted into an object as described above.
    */
-  dynamic&       operator[](dynamic const&) &;
-  dynamic const& operator[](dynamic const&) const&;
-  dynamic&&      operator[](dynamic const&) &&;
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, dynamic&> operator[](K&&) &;
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, dynamic const&> operator[](K&&) const&;
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, dynamic&&> operator[](K&&) &&;
+
+  dynamic& operator[](StringPiece) &;
+  dynamic const& operator[](StringPiece) const&;
+  dynamic&& operator[](StringPiece) &&;
 
   /*
    * Only defined for objects, throws TypeError otherwise.
@@ -477,7 +513,8 @@ struct dynamic : private boost::operators<dynamic> {
 
   /*
    * Inserts the supplied key-value pair to an object, or throws if
-   * it's not an object.
+   * it's not an object. If the key already exists, insert will overwrite the
+   * value, i.e., similar to insert_or_assign.
    *
    * Invalidates iterators.
    */
@@ -518,7 +555,10 @@ struct dynamic : private boost::operators<dynamic> {
    *
    * Returns the number of elements erased (i.e. 1 or 0).
    */
-  std::size_t erase(dynamic const& key);
+  template <typename K>
+  IfIsNonStringDynamicConvertible<K, std::size_t> erase(K&&);
+
+  std::size_t erase(StringPiece);
 
   /*
    * Erase an element from a dynamic object or array, using an

folly/test/DynamicTest.cpp

@@ -16,12 +16,14 @@
 
 #include <folly/dynamic.h>
 
+#include <folly/Range.h>
 #include <folly/json.h>
 #include <folly/portability/GTest.h>
 
 #include <iterator>
 
 using folly::dynamic;
+using folly::StringPiece;
 
 TEST(Dynamic, Default) {
   dynamic obj;
@@ -33,11 +35,31 @@ TEST(Dynamic, ObjectBasics) {
   EXPECT_EQ(obj.at("a"), false);
   EXPECT_EQ(obj.size(), 1);
   obj.insert("a", true);
+
+  dynamic key{"a"};
+  folly::StringPiece sp{"a"};
+  std::string s{"a"};
+
   EXPECT_EQ(obj.size(), 1);
   EXPECT_EQ(obj.at("a"), true);
-  obj.at("a") = nullptr;
+  EXPECT_EQ(obj.at(sp), true);
+  EXPECT_EQ(obj.at(key), true);
+
+  obj.at(sp) = nullptr;
   EXPECT_EQ(obj.size(), 1);
-  EXPECT_TRUE(obj.at("a") == nullptr);
+  EXPECT_TRUE(obj.at(s) == nullptr);
+
+  obj["a"] = 12;
+  EXPECT_EQ(obj[sp], 12);
+  obj[key] = "foo";
+  EXPECT_EQ(obj["a"], "foo");
+  (void)obj["b"];
+  EXPECT_EQ(obj.size(), 2);
+
+  obj.erase("a");
+  EXPECT_TRUE(obj.find(sp) == obj.items().end());
+  obj.erase("b");
+  EXPECT_EQ(obj.size(), 0);
 
   dynamic newObject = dynamic::object;
 
@@ -142,6 +164,124 @@ TEST(Dynamic, ObjectBasics) {
   EXPECT_EQ(mergeObj1, combinedPreferObj1);
 }
 
+namespace {
+
+struct StaticStrings {
+  static constexpr auto kA = "a";
+  static constexpr const char* kB = "b";
+  static const folly::StringPiece kFoo;
+  static const std::string kBar;
+};
+/* static */ const folly::StringPiece StaticStrings::kFoo{"foo"};
+/* static */ const std::string StaticStrings::kBar{"bar"};
+
+} // namespace
+
+TEST(Dynamic, ObjectHeterogeneousAccess) {
+  dynamic empty;
+  dynamic foo{"foo"};
+  const char* a = "a";
+  StringPiece sp{"a"};
+  std::string str{"a"};
+  dynamic bar{"bar"};
+  const char* b = "b";
+
+  dynamic obj = dynamic::object("a", 123)(empty, 456)(foo, 789);
+
+  // at()
+  EXPECT_EQ(obj.at(empty), 456);
+  EXPECT_EQ(obj.at(nullptr), 456);
+  EXPECT_EQ(obj.at(foo), 789);
+
+  EXPECT_EQ(obj.at(a), 123);
+  EXPECT_EQ(obj.at(StaticStrings::kA), 123);
+  EXPECT_EQ(obj.at("a"), 123);
+
+  EXPECT_EQ(obj.at(sp), 123);
+  EXPECT_EQ(obj.at(StringPiece{"a"}), 123);
+  EXPECT_EQ(obj.at(StaticStrings::kFoo), 789);
+
+  EXPECT_EQ(obj.at(std::string{"a"}), 123);
+  EXPECT_EQ(obj.at(str), 123);
+
+  EXPECT_THROW(obj.at(b), std::out_of_range);
+  EXPECT_THROW(obj.at(StringPiece{b}), std::out_of_range);
+  EXPECT_THROW(obj.at(StaticStrings::kBar), std::out_of_range);
+
+  // find()
+  EXPECT_EQ(obj.find(empty)->second, 456);
+  EXPECT_EQ(obj.find(nullptr)->second, 456);
+  EXPECT_EQ(obj.find(foo)->second, 789);
+
+  EXPECT_EQ(obj.find(a)->second, 123);
+  EXPECT_EQ(obj.find(StaticStrings::kA)->second, 123);
+  EXPECT_EQ(obj.find("a")->second, 123);
+
+  EXPECT_EQ(obj.find(sp)->second, 123);
+  EXPECT_EQ(obj.find(StringPiece{"a"})->second, 123);
+  EXPECT_EQ(obj.find(StaticStrings::kFoo)->second, 789);
+
+  EXPECT_EQ(obj.find(std::string{"a"})->second, 123);
+  EXPECT_EQ(obj.find(str)->second, 123);
+
+  EXPECT_TRUE(obj.find(b) == obj.items().end());
+  EXPECT_TRUE(obj.find(StringPiece{b}) == obj.items().end());
+  EXPECT_TRUE(obj.find(StaticStrings::kBar) == obj.items().end());
+
+  // count()
+  EXPECT_EQ(obj.count(empty), 1);
+  EXPECT_EQ(obj.count(nullptr), 1);
+  EXPECT_EQ(obj.count(foo), 1);
+
+  EXPECT_EQ(obj.count(a), 1);
+  EXPECT_EQ(obj.count(StaticStrings::kA), 1);
+  EXPECT_EQ(obj.count("a"), 1);
+
+  EXPECT_EQ(obj.count(sp), 1);
+  EXPECT_EQ(obj.count(StringPiece{"a"}), 1);
+  EXPECT_EQ(obj.count(StaticStrings::kFoo), 1);
+
+  EXPECT_EQ(obj.count(std::string{"a"}), 1);
+  EXPECT_EQ(obj.count(str), 1);
+
+  EXPECT_EQ(obj.count(b), 0);
+  EXPECT_EQ(obj.count(StringPiece{b}), 0);
+  EXPECT_EQ(obj.count(StaticStrings::kBar), 0);
+
+  // operator[]
+  EXPECT_EQ(obj[empty], 456);
+  EXPECT_EQ(obj[nullptr], 456);
+  EXPECT_EQ(obj[foo], 789);
+
+  EXPECT_EQ(obj[a], 123);
+  EXPECT_EQ(obj[StaticStrings::kA], 123);
+  EXPECT_EQ(obj["a"], 123);
+
+  EXPECT_EQ(obj[sp], 123);
+  EXPECT_EQ(obj[StringPiece{"a"}], 123);
+  EXPECT_EQ(obj[StaticStrings::kFoo], 789);
+
+  EXPECT_EQ(obj[std::string{"a"}], 123);
+  EXPECT_EQ(obj[str], 123);
+
+  EXPECT_EQ(obj[b], nullptr);
+  obj[b] = 42;
+  EXPECT_EQ(obj[StringPiece{b}], 42);
+  obj[StaticStrings::kBar] = 43;
+  EXPECT_EQ(obj["bar"], 43);
+
+  // erase() + dynamic&&
+  EXPECT_EQ(obj.erase(StaticStrings::kB), /* num elements erased */ 1);
+
+  dynamic obj2 = obj;
+  dynamic obj3 = obj;
+  dynamic obj4 = obj;
+  EXPECT_EQ(std::move(obj).find(StaticStrings::kFoo)->second, 789);
+  EXPECT_EQ(std::move(obj2).at(StaticStrings::kA), 123);
+  EXPECT_EQ(std::move(obj3)[nullptr], 456);
+  EXPECT_EQ(std::move(obj4).erase(StaticStrings::kBar), 1);
+}
+
 TEST(Dynamic, CastFromVectorOfBooleans) {
   std::vector<bool> b;
   b.push_back(true);