callWithExtractedKey fix for old libstdc++

Summary:
libstdc++ versions that don't include N4387 ("perfect initialization" for
pairs and tuples) can't use the two-arg pair constructor as an emplace
target when the emplace args don't exactly match the destination types.
(libstdc++.so.6.0.22 is the first one that contains the improvement,
which corresponds to gcc 6.1.0).  This diff guards the optimization added
in D21419822 (https://github.com/facebook/folly/commit/7d618e4fde39b44e0952880653e10ec97400583f) with an enable_if test to avoid the problem, and includes
the template parameter changes necessary to get the target types to the
test site.

(Note: this ignores all push blocking failures!)

Differential Revision: D21766560

fbshipit-source-id: f8b2a0230504237f976e14b0740bb31bc7a57b77

folly/container/F14Map.h

@@ -414,7 +414,8 @@ class F14BasicMap {
  public:
   template <typename... Args>
   std::pair<iterator, bool> emplace(Args&&... args) {
-    auto rv = folly::detail::callWithExtractedKey<key_type, UsableAsKey>(
+    auto rv =
+        folly::detail::callWithExtractedKey<key_type, mapped_type, UsableAsKey>(
             table_.alloc(),
             [&](auto&&... inner) {
               return table_.tryEmplaceValue(

folly/container/detail/Util.h

@@ -84,10 +84,26 @@ struct TemporaryEmplaceKey {
 // about heterogeneous lookup you can just pass a single-arg template
 // that extends std::false_type.
 
-template <typename Func, typename KeyType, typename Arg1, typename Arg2>
+// TODO(T31574848): We can remove the std::enable_if_t once we no longer
+// target platforms without N4387 ("perfect initialization" for pairs
+// and tuples).  libstdc++ at gcc-6.1.0 is the first release that contains
+// the improved set of pair constructors.
+template <
+    typename KeyType,
+    typename MappedType,
+    typename Func,
+    typename UsableKeyType,
+    typename Arg1,
+    typename Arg2,
+    std::enable_if_t<
+        std::is_constructible<
+            std::pair<KeyType const, MappedType>,
+            Arg1&&,
+            Arg2&&>::value,
+        int> = 0>
 auto callWithKeyAndPairArgs(
     Func&& f,
-    KeyType const& key,
+    UsableKeyType const& key,
     std::tuple<Arg1>&& first_args,
     std::tuple<Arg2>&& second_args) {
   return f(
@@ -96,10 +112,16 @@ auto callWithKeyAndPairArgs(
       std::forward<Arg2>(std::get<0>(second_args)));
 }
 
-template <typename Func, typename KeyType, typename... Args1, typename... Args2>
+template <
+    typename KeyType,
+    typename MappedType,
+    typename Func,
+    typename UsableKeyType,
+    typename... Args1,
+    typename... Args2>
 auto callWithKeyAndPairArgs(
     Func&& f,
-    KeyType const& key,
+    UsableKeyType const& key,
     std::tuple<Args1...>&& first_args,
     std::tuple<Args2...>&& second_args) {
   return f(
@@ -109,9 +131,13 @@ auto callWithKeyAndPairArgs(
       std::move(second_args));
 }
 
+template <typename>
+using ExactKeyMatchOnly = std::false_type;
+
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    typename MappedType,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func,
     typename Arg1,
@@ -128,7 +154,7 @@ auto callWithExtractedKey(
     std::tuple<Args2...>&& second_args) {
   // we found a usable key in the args :)
   auto const& key = std::get<0>(first_args);
-  return callWithKeyAndPairArgs(
+  return callWithKeyAndPairArgs<KeyType, MappedType>(
       std::forward<Func>(f),
       key,
       std::tuple<Arg1&&>(std::move(first_args)),
@@ -137,7 +163,8 @@ auto callWithExtractedKey(
 
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    typename MappedType,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func,
     typename... Args1,
@@ -151,7 +178,7 @@ auto callWithExtractedKey(
   // we will need to materialize a temporary key :(
   TemporaryEmplaceKey<KeyType, Alloc> key(
       a, std::tuple<Args1&&...>(std::move(first_args)));
-  return callWithKeyAndPairArgs(
+  return callWithKeyAndPairArgs<KeyType, MappedType>(
       std::forward<Func>(f),
       const_cast<KeyType const&>(key.value()),
       std::forward_as_tuple(std::move(key.value())),
@@ -160,11 +187,12 @@ auto callWithExtractedKey(
 
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    typename MappedType,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func>
 auto callWithExtractedKey(Alloc& a, Func&& f) {
-  return callWithExtractedKey<KeyType, UsableAsKey>(
+  return callWithExtractedKey<KeyType, MappedType, UsableAsKey>(
       a,
       std::forward<Func>(f),
       std::piecewise_construct,
@@ -174,13 +202,14 @@ auto callWithExtractedKey(Alloc& a, Func&& f) {
 
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    typename MappedType,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func,
     typename U1,
     typename U2>
 auto callWithExtractedKey(Alloc& a, Func&& f, U1&& x, U2&& y) {
-  return callWithExtractedKey<KeyType, UsableAsKey>(
+  return callWithExtractedKey<KeyType, MappedType, UsableAsKey>(
       a,
       std::forward<Func>(f),
       std::piecewise_construct,
@@ -190,13 +219,14 @@ auto callWithExtractedKey(Alloc& a, Func&& f, U1&& x, U2&& y) {
 
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    typename MappedType,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func,
     typename U1,
     typename U2>
 auto callWithExtractedKey(Alloc& a, Func&& f, std::pair<U1, U2> const& p) {
-  return callWithExtractedKey<KeyType, UsableAsKey>(
+  return callWithExtractedKey<KeyType, MappedType, UsableAsKey>(
       a,
       std::forward<Func>(f),
       std::piecewise_construct,
@@ -206,7 +236,8 @@ auto callWithExtractedKey(Alloc& a, Func&& f, std::pair<U1, U2> const& p) {
 
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    typename MappedType,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func,
     typename U1,
@@ -215,7 +246,7 @@ auto callWithExtractedKey(Alloc& a, Func&& f, std::pair<U1, U2>&& p) {
   // std::move(p.first) is wrong because if U1 is an lvalue reference the
   // result will incorrectly be an rvalue ref.  static_cast here allows
   // proper ref collapsing
-  return callWithExtractedKey<KeyType, UsableAsKey>(
+  return callWithExtractedKey<KeyType, MappedType, UsableAsKey>(
       a,
       std::forward<Func>(f),
       std::piecewise_construct,
@@ -228,7 +259,7 @@ auto callWithExtractedKey(Alloc& a, Func&& f, std::pair<U1, U2>&& p) {
 
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func,
     typename Arg,
@@ -244,7 +275,7 @@ auto callWithConstructedKey(Alloc&, Func&& f, Arg&& arg) {
 
 template <
     typename KeyType,
-    template <typename> class UsableAsKey,
+    template <typename> class UsableAsKey = ExactKeyMatchOnly,
     typename Alloc,
     typename Func,
     typename... Args>

folly/container/test/UtilTest.cpp

@@ -301,14 +301,11 @@ void runKeyExtractCases(
   }
 }
 
-template <typename T>
-using FalseFunc1 = std::false_type;
-
 struct DoEmplace1 {
   template <typename F, typename P>
   void operator()(F&& f, P&& p) const {
     std::allocator<char> a;
-    detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+    detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
         a, std::forward<F>(f), std::forward<P>(p));
   }
 };
@@ -317,14 +314,14 @@ struct DoEmplace2 {
   template <typename F, typename U1, typename U2>
   void operator()(F&& f, std::pair<U1, U2> const& p) const {
     std::allocator<char> a;
-    detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+    detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
         a, std::forward<F>(f), p.first, p.second);
   }
 
   template <typename F, typename U1, typename U2>
   void operator()(F&& f, std::pair<U1, U2>&& p) const {
     std::allocator<char> a;
-    detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+    detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
         a, std::forward<F>(f), std::move(p.first), std::move(p.second));
   }
 };
@@ -333,7 +330,7 @@ struct DoEmplace3 {
   template <typename F, typename U1, typename U2>
   void operator()(F&& f, std::pair<U1, U2> const& p) const {
     std::allocator<char> a;
-    detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+    detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
         a,
         std::forward<F>(f),
         std::piecewise_construct,
@@ -344,7 +341,7 @@ struct DoEmplace3 {
   template <typename F, typename U1, typename U2>
   void operator()(F&& f, std::pair<U1, U2>&& p) const {
     std::allocator<char> a;
-    detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+    detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
         a,
         std::forward<F>(f),
         std::piecewise_construct,
@@ -361,7 +358,7 @@ struct DoEmplace3Value {
   template <typename F, typename U1, typename U2>
   void operator()(F&& f, std::pair<U1, U2> const& p) const {
     std::allocator<char> a;
-    detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+    detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
         a,
         std::forward<F>(f),
         std::piecewise_construct,
@@ -372,7 +369,7 @@ struct DoEmplace3Value {
   template <typename F, typename U1, typename U2>
   void operator()(F&& f, std::pair<U1, U2>&& p) const {
     std::allocator<char> a;
-    detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+    detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
         a,
         std::forward<F>(f),
         std::piecewise_construct,
@@ -390,7 +387,7 @@ TEST(Util, callWithExtractedKey) {
   // Calling the default pair constructor via emplace is valid, but not
   // very useful in real life.  Verify that it works.
   std::allocator<char> a;
-  detail::callWithExtractedKey<Tracked<0>, FalseFunc1>(
+  detail::callWithExtractedKey<Tracked<0>, Tracked<1>>(
       a, [](Tracked<0> const& key, auto&&... args) {
         EXPECT_TRUE(key == 0);
         std::pair<Tracked<0> const, Tracked<1>> p(
@@ -417,23 +414,23 @@ TEST(Util, callWithConstructedKey) {
     uint64_t k3 = 0;
     sink.reset();
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(a, sinkFunc, k1);
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, k1);
     // copy is expected on successful emplace
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{1, 0, 0, 0}), 0);
 
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(a, sinkFunc, k2);
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, k2);
     // no copies or moves on failing emplace with value_type
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{0, 0, 0, 0}), 0);
 
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(a, sinkFunc, k3);
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, k3);
     // copy convert expected for failing emplace with wrong type
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{0, 0, 1, 0}), 0);
 
     sink.reset();
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(a, sinkFunc, k3);
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, k3);
     // copy convert + move expected for successful emplace with wrong type
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{0, 1, 1, 0}), 0);
   }
@@ -443,27 +440,23 @@ TEST(Util, callWithConstructedKey) {
     uint64_t k3 = 0;
     sink.reset();
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(
-        a, sinkFunc, std::move(k1));
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, std::move(k1));
     // move is expected on successful emplace
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{0, 1, 0, 0}), 0);
 
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(
-        a, sinkFunc, std::move(k2));
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, std::move(k2));
     // no copies or moves on failing emplace with value_type
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{0, 0, 0, 0}), 0);
 
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(
-        a, sinkFunc, std::move(k3));
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, std::move(k3));
     // move convert expected for failing emplace with wrong type
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{0, 0, 0, 1}), 0);
 
     sink.reset();
     resetTracking();
-    detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(
-        a, sinkFunc, std::move(k3));
+    detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc, std::move(k3));
     // move convert + move expected for successful emplace with wrong type
     EXPECT_EQ(Tracked<0>::counts().dist(Counts{0, 1, 0, 1}), 0);
   }
@@ -471,7 +464,7 @@ TEST(Util, callWithConstructedKey) {
   // Calling the default pair constructor via emplace is valid, but not
   // very useful in real life.  Verify that it works.
   sink.reset();
-  detail::callWithConstructedKey<Tracked<0>, FalseFunc1>(a, sinkFunc);
+  detail::callWithConstructedKey<Tracked<0>>(a, sinkFunc);
   EXPECT_TRUE(sink.has_value());
 }
 
@@ -516,51 +509,51 @@ TEST(Util, callWithExtractedHeterogeneousKey) {
 
   // none of the std::move below actually get consumed
 
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a, ExpectArgTypes<std::string, std::string&, int&&>{0, &str}, str, 0);
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<std::string, std::string const&, int const&>{
           1, &strPair.first},
       strPair);
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<std::string, std::string&, int&&>{2, &str},
       std::move(strLRefPair));
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<std::string, std::string&, int const&>{10, &str},
       strLRefPair);
 
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<std::string, std::string&&, int&&>{3, &str},
       std::move(str),
       0);
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<std::string, std::string&&, int&&>{4, &strPair.first},
       std::move(strPair));
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<std::string, std::string&, int const&>{5, &str},
       strRRefPair);
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<std::string, std::string&&, int&&>{11, &str},
       std::move(strRRefPair));
 
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a, ExpectArgTypes<StringPiece, StringPiece&, int&&>{6, &sp}, sp, 0);
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a,
       ExpectArgTypes<StringPiece, StringPiece const&, int const&>{
           7, &spPair.first},
       spPair);
 
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a, ExpectArgTypes<std::string, std::string&&, int&&>{8}, ptr, 0);
-  detail::callWithExtractedKey<std::string, IsStringPiece>(
+  detail::callWithExtractedKey<std::string, int, IsStringPiece>(
       a, ExpectArgTypes<std::string, std::string&&, int const&>{9}, ptrPair);
 }