Disallow temporary default values in get_ref_default()

Summary: Passing a temporary default value to `get_ref_default()` returns a dangling reference when it is used. Reviewed By: lbrandy, yfeldblum Differential Revision: D4768769 fbshipit-source-id: 4c58a17dc7662ad553cf88a8544dae20016d2f6f
mtabebe · Mar 27, 2017 · 136c9fb · 136c9fb
1 parent 4cc8030
commit 136c9fb
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 0 deletions.
diff --git a/folly/MapUtil.h b/folly/MapUtil.h
@@ -106,6 +106,24 @@ const typename Map::mapped_type& get_ref_default(
   return (pos != map.end() ? pos->second : dflt);
 }
 
+/**
+ * Passing a temporary default value returns a dangling reference when it is
+ * returned. Lifetime extension is broken by the indirection.
+ * The caller must ensure that the default value outlives the reference returned
+ * by get_ref_default().
+ */
+template <class Map>
+const typename Map::mapped_type& get_ref_default(
+    const Map& map,
+    const typename Map::key_type& key,
+    typename Map::mapped_type&& dflt) = delete;
+
+template <class Map>
+const typename Map::mapped_type& get_ref_default(
+    const Map& map,
+    const typename Map::key_type& key,
+    const typename Map::mapped_type&& dflt) = delete;
+
 /**
  * Given a map and a key, return a reference to the value corresponding to the
  * key in the map, or the given default reference if the key doesn't exist in

diff --git a/folly/test/MapUtilTest.cpp b/folly/test/MapUtilTest.cpp
@@ -19,6 +19,7 @@
 #include <map>
 #include <unordered_map>
 
+#include <folly/Traits.h>
 #include <folly/portability/GTest.h>
 
 using namespace folly;
@@ -138,3 +139,23 @@ TEST(MapUtil, get_ptr_path_mixed) {
   EXPECT_TRUE(get_ptr(cm, "a", 1, "b"));
   EXPECT_FALSE(get_ptr(cm, "b", 1, "b"));
 }
+
+namespace {
+template <typename T, typename = void>
+struct Compiles : std::false_type {};
+
+template <typename T>
+struct Compiles<
+    T,
+    void_t<decltype(get_ref_default(
+        std::declval<std::map<int, typename std::decay<T>::type>>(),
+        std::declval<int>(),
+        std::declval<T>()))>> : std::true_type {};
+}
+
+TEST(MapUtil, get_default_temporary) {
+  EXPECT_TRUE(Compiles<const int&>::value);
+  EXPECT_TRUE(Compiles<int&>::value);
+  EXPECT_FALSE(Compiles<const int&&>::value);
+  EXPECT_FALSE(Compiles<int&&>::value);
+}