Split get_default() into two for deferred default construction and added forwarding...

Summary:
As it stood currently folly::get_default() would unnecessarily
construct a value into the third parameter, which was unnecessary in the code
path where the element was found in the map.  Also the default value can be
forwarded to the return type in the code path where the element is not found
and an rvalue parameter is passed as the default value

Reviewed By: yfeldblum

Differential Revision: D6390315

fbshipit-source-id: ef692b827d5a36751b4eb1e12042869e8fbba2e5

diff --git a/folly/MapUtil.h b/folly/MapUtil.h
index b1fbfadcaf59ecf3db6d14e80f35e472152fa249..433489273785a6f2677c6f36563ea2a292d8fb58 100644 (file)
--- a/folly/MapUtil.h
+++ b/folly/MapUtil.h
@@ -18,6 +18,7 @@
 
 #include <folly/Conv.h>
 #include <folly/Optional.h>
+#include <folly/functional/Invoke.h>
 #include <tuple>
 
 namespace folly {
@@ -26,13 +27,21 @@ namespace folly {
  * Given a map and a key, return the value corresponding to the key in the map,
  * or a given default value if the key doesn't exist in the map.
  */
-template <class Map, typename Key = typename Map::key_type>
-typename Map::mapped_type get_default(
-    const Map& map,
-    const Key& key,
-    const typename Map::mapped_type& dflt = typename Map::mapped_type()) {
+template <typename Map, typename Key>
+typename Map::mapped_type get_default(const Map& map, const Key& key) {
   auto pos = map.find(key);
-  return (pos != map.end() ? pos->second : dflt);
+  return (pos != map.end()) ? (pos->second) : (typename Map::mapped_type{});
+}
+template <
+    class Map,
+    typename Key = typename Map::key_type,
+    typename Value = typename Map::mapped_type,
+    typename std::enable_if<!is_invocable<Value>::value>::type* = nullptr>
+typename Map::mapped_type
+get_default(const Map& map, const Key& key, Value&& dflt) {
+  using M = typename Map::mapped_type;
+  auto pos = map.find(key);
+  return (pos != map.end()) ? (pos->second) : M(std::forward<Value>(dflt));
 }
 
 /**

diff --git a/folly/test/MapUtilTest.cpp b/folly/test/MapUtilTest.cpp
index 52e63e81f7a900360a54a6d3ccd5f0fc1cf2f4be..7ef13f718fcf757e50fc2e7a202bcbb118cabc01 100644 (file)
--- a/folly/test/MapUtilTest.cpp
+++ b/folly/test/MapUtilTest.cpp
@@ -16,6 +16,7 @@
 
 #include <folly/MapUtil.h>
 
+#include <cstddef>
 #include <map>
 #include <unordered_map>
 
@@ -246,3 +247,52 @@ TEST(MapUtil, get_ref_default_path_temporary) {
   EXPECT_FALSE(GetRefDefaultPathCompiles<const int&&>::value);
   EXPECT_FALSE(GetRefDefaultPathCompiles<int&&>::value);
 }
+
+namespace {
+
+class TestConstruction {
+ public:
+  TestConstruction() {
+    EXPECT_TRUE(false);
+  }
+  TestConstruction(TestConstruction&&) {
+    EXPECT_TRUE(false);
+  }
+  TestConstruction(const TestConstruction&) {
+    EXPECT_TRUE(false);
+  }
+
+  explicit TestConstruction(std::string&& string)
+      : string_{std::move(string)} {}
+  explicit TestConstruction(int&& integer) : integer_{integer} {}
+
+  TestConstruction& operator=(const TestConstruction&) = delete;
+  TestConstruction& operator=(TestConstruction&&) = delete;
+
+  int integer_{};
+  std::string string_{};
+};
+
+} // namespace
+
+TEST(MapUtil, test_get_default_deferred_construction) {
+  auto map = std::unordered_map<int, TestConstruction>{};
+  map.emplace(
+      std::piecewise_construct,
+      std::forward_as_tuple(1),
+      std::forward_as_tuple(1));
+
+  EXPECT_EQ(map.at(1).integer_, 1);
+
+  {
+    auto val = get_default(map, 0, 1);
+    EXPECT_EQ(val.integer_, 1);
+    EXPECT_EQ(val.string_, "");
+  }
+
+  {
+    auto val = get_default(map, 0, "something");
+    EXPECT_EQ(val.integer_, 0);
+    EXPECT_EQ(val.string_, "something");
+  }
+}