fix missing eraseInto overload for key with permissive constructor

Summary:
If a key type has a templated constructor that can implicitly
accept const_iterator, then a call to erase or eraseInto with a non-const
iterator results in an ambiguous overload. This diff adds an extra
overload of eraseInto to avoid the ambiguity, as well as adding unit
tests that cover the rare corner case.

Reviewed By: yfeldblum

Differential Revision: D19809069

fbshipit-source-id: 499849da2d6ad64070b13eb0a9ed86a3d49ad9b0

folly/container/F14Map.h

@@ -537,6 +537,15 @@ class F14BasicMap {
     return table_.makeIter(itemPos);
   }
 
+  // This form avoids ambiguity when key_type has a templated constructor
+  // that accepts const_iterator
+  template <typename BeforeDestroy>
+  FOLLY_ALWAYS_INLINE iterator
+  eraseInto(iterator pos, BeforeDestroy&& beforeDestroy) {
+    const_iterator cpos{pos};
+    return eraseInto(cpos, beforeDestroy);
+  }
+
   template <typename BeforeDestroy>
   iterator eraseInto(
       const_iterator first,

folly/container/test/F14MapTest.cpp

@@ -1512,6 +1512,52 @@ TEST(F14FastMap, eraseInto) {
       F14FastMap<folly::test::MoveOnlyTestInt, folly::test::MoveOnlyTestInt>>();
 }
 
+template <typename M>
+void runPermissiveConstructorTest() {
+  M t;
+  M const& ct{t};
+
+  for (int i = 10; i <= 100; i += 10) {
+    t[i] = i;
+  }
+  t.erase(10);
+  EXPECT_EQ(t.size(), 9);
+  t.erase(t.find(20));
+  EXPECT_EQ(t.size(), 8);
+  t.erase(ct.find(30));
+  EXPECT_EQ(t.size(), 7);
+  t.eraseInto(40, [](auto&&, auto&&) {});
+  EXPECT_EQ(t.size(), 6);
+  t.eraseInto(t.find(50), [](auto&&, auto&&) {});
+  EXPECT_EQ(t.size(), 5);
+  t.eraseInto(ct.find(60), [](auto&&, auto&&) {});
+  EXPECT_EQ(t.size(), 4);
+}
+
+TEST(F14ValueMap, permissiveConstructor) {
+  runPermissiveConstructorTest<F14ValueMap<
+      folly::test::PermissiveConstructorTestInt,
+      folly::test::PermissiveConstructorTestInt>>();
+}
+
+TEST(F14NodeMap, permissiveConstructor) {
+  runPermissiveConstructorTest<F14NodeMap<
+      folly::test::PermissiveConstructorTestInt,
+      folly::test::PermissiveConstructorTestInt>>();
+}
+
+TEST(F14VectorMap, permissiveConstructor) {
+  runPermissiveConstructorTest<F14VectorMap<
+      folly::test::PermissiveConstructorTestInt,
+      folly::test::PermissiveConstructorTestInt>>();
+}
+
+TEST(F14FastMap, permissiveConstructor) {
+  runPermissiveConstructorTest<F14FastMap<
+      folly::test::PermissiveConstructorTestInt,
+      folly::test::PermissiveConstructorTestInt>>();
+}
+
 TEST(F14ValueMap, heterogeneousLookup) {
   using Hasher = folly::transparent<folly::hasher<folly::StringPiece>>;
   using KeyEqual = folly::transparent<std::equal_to<folly::StringPiece>>;

folly/container/test/TrackingTypes.h

@@ -83,6 +83,35 @@ struct ThrowOnCopyTestInt {
   }
 };
 
+struct PermissiveConstructorTestInt {
+  int x;
+
+  PermissiveConstructorTestInt() noexcept : x(0) {}
+  /* implicit */ PermissiveConstructorTestInt(int x0) : x(x0) {}
+
+  template <typename T>
+  /* implicit */ PermissiveConstructorTestInt(T&& src)
+      : x(std::forward<T>(src)) {}
+
+  PermissiveConstructorTestInt(PermissiveConstructorTestInt&& rhs) noexcept
+      : x(rhs.x) {}
+  PermissiveConstructorTestInt(PermissiveConstructorTestInt const&) = delete;
+  PermissiveConstructorTestInt& operator=(
+      PermissiveConstructorTestInt&& rhs) noexcept {
+    x = rhs.x;
+    return *this;
+  }
+  PermissiveConstructorTestInt& operator=(PermissiveConstructorTestInt const&) =
+      delete;
+
+  bool operator==(PermissiveConstructorTestInt const& rhs) const {
+    return x == rhs.x;
+  }
+  bool operator!=(PermissiveConstructorTestInt const& rhs) const {
+    return !(*this == rhs);
+  }
+};
+
 // Tracked is implicitly constructible across tags
 struct Counts {
   uint64_t copyConstruct{0};
@@ -555,6 +584,14 @@ struct hash<folly::test::ThrowOnCopyTestInt> {
   }
 };
 
+template <>
+struct hash<folly::test::PermissiveConstructorTestInt> {
+  std::size_t operator()(
+      folly::test::PermissiveConstructorTestInt const& val) const {
+    return val.x;
+  }
+};
+
 template <int Tag>
 struct hash<folly::test::Tracked<Tag>> {
   size_t operator()(folly::test::Tracked<Tag> const& tracked) const {