From 78b2965a1cf7eedf1107c1bb54f010f1215d9cb4 Mon Sep 17 00:00:00 2001 From: Pavlo Kushnir Date: Wed, 26 Nov 2014 19:28:18 -0800 Subject: [PATCH] Fix dynamic::insert Summary: folly::dynamic::insert is broken. When we try to insert an rvalue and the key already exists in dynamic, insert will replace the value with empty dynamic. Test Plan: unit test attached Reviewed By: stepan@fb.com Subscribers: trunkagent, njormrod, folly-diffs@ FB internal diff: D1704995 Signature: t1:1704995:1417053631:8e0163693df721c54b016f8c27e81e7bf60194cb --- folly/dynamic-inl.h | 11 ++--------- folly/test/DynamicTest.cpp | 9 +++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/folly/dynamic-inl.h b/folly/dynamic-inl.h index 81f063fb..f0a580cb 100644 --- a/folly/dynamic-inl.h +++ b/folly/dynamic-inl.h @@ -636,15 +636,8 @@ inline dynamic::const_item_iterator dynamic::find(dynamic const& key) const { template inline void dynamic::insert(K&& key, V&& val) { auto& obj = get(); - auto rv = obj.insert(std::make_pair(std::forward(key), - std::forward(val))); - if (!rv.second) { - // note, the second use of std:forward(val) is only correct - // if the first one did not result in a move. obj[key] = val - // would be preferrable but doesn't compile because dynamic - // is (intentionally) not default constructable - rv.first->second = std::forward(val); - } + auto rv = obj.insert({ std::forward(key), nullptr }); + rv.first->second = std::forward(val); } inline std::size_t dynamic::erase(dynamic const& key) { diff --git a/folly/test/DynamicTest.cpp b/folly/test/DynamicTest.cpp index 4701a1bf..ff38e338 100644 --- a/folly/test/DynamicTest.cpp +++ b/folly/test/DynamicTest.cpp @@ -84,6 +84,15 @@ TEST(Dynamic, ObjectBasics) { EXPECT_EQ(d3.at("123"), 42); EXPECT_EQ(d3.at(123), 321); + dynamic objInsert = folly::dynamic::object(); + dynamic objA = folly::dynamic::object("1", "2"); + dynamic objB = folly::dynamic::object("1", "2"); + + objInsert.insert("1", std::move(objA)); + objInsert.insert("1", std::move(objB)); + + EXPECT_EQ(objInsert.find("1")->second.size(), 1); + // We don't allow objects as keys in objects. EXPECT_ANY_THROW(newObject[d3] = 12); } -- 2.34.1