fix bugs in fallback F14 containers

Summary:
Previously, `visitContiguousRanges` for fallback F14 sets did not build and
`visitAllocationClasses` was invoking the visitor with the wrong order.

This diff fixes these bugs and moves the tests so that it'll get run regardless
of whether vector intrinsics are available.

Reviewed By: nbronson

Differential Revision: D8542771

fbshipit-source-id: 3aee3ab9e3278110ec8349bd9c858d19df779eb1

folly/container/F14Map.h

@@ -54,8 +54,6 @@ class F14BasicMap : public std::unordered_map<K, M, H, E, A> {
   using Super = std::unordered_map<K, M, H, E, A>;
 
  public:
-  using typename Super::value_type;
-
   using Super::Super;
   F14BasicMap() : Super() {}
 
@@ -74,13 +72,13 @@ class F14BasicMap : public std::unordered_map<K, M, H, E, A> {
       visitor(bc * sizeof(typename Super::pointer), 1);
     }
     visitor(
-        this->size(), sizeof(StdNodeReplica<K, typename Super::value_type, H>));
+        sizeof(StdNodeReplica<K, typename Super::value_type, H>), this->size());
   }
 
   template <typename V>
   void visitContiguousRanges(V&& visitor) const {
-    for (value_type const& entry : *this) {
-      value_type const* b = std::addressof(entry);
+    for (typename Super::value_type const& entry : *this) {
+      typename Super::value_type const* b = std::addressof(entry);
       visitor(b, b + 1);
     }
   }

folly/container/F14Set.h

@@ -68,13 +68,13 @@ class F14BasicSet : public std::unordered_set<K, H, E, A> {
       visitor(bc * sizeof(typename Super::pointer), 1);
     }
     visitor(
-        this->size(), sizeof(StdNodeReplica<K, typename Super::value_type, H>));
+        sizeof(StdNodeReplica<K, typename Super::value_type, H>), this->size());
   }
 
   template <typename V>
   void visitContiguousRanges(V&& visitor) const {
-    for (value_type const& entry : *this) {
-      value_type const* b = std::addressof(entry);
+    for (typename Super::value_type const& entry : *this) {
+      typename Super::value_type const* b = std::addressof(entry);
       visitor(b, b + 1);
     }
   }

folly/container/test/F14MapTest.cpp

@@ -106,6 +106,46 @@ TEST(F14Map, getAllocatedMemorySize) {
   runAllocatedMemorySizeTests<folly::fbstring, long>();
 }
 
+template <typename M>
+void runVisitContiguousRangesTest() {
+  M map;
+
+  for (int i = 0; i < 1000; ++i) {
+    map[i] = i;
+    map.erase(i / 2);
+  }
+
+  std::unordered_map<uintptr_t, bool> visited;
+  for (auto& entry : map) {
+    visited[reinterpret_cast<uintptr_t>(&entry)] = false;
+  }
+
+  map.visitContiguousRanges([&](auto b, auto e) {
+    for (auto i = b; i != e; ++i) {
+      auto iter = visited.find(reinterpret_cast<uintptr_t>(i));
+      ASSERT_TRUE(iter != visited.end());
+      EXPECT_FALSE(iter->second);
+      iter->second = true;
+    }
+  });
+}
+
+TEST(F14ValueMap, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14ValueMap<int, int>>();
+}
+
+TEST(F14NodeMap, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14NodeMap<int, int>>();
+}
+
+TEST(F14VectorMap, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14VectorMap<int, int>>();
+}
+
+TEST(F14FastMap, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14FastMap<int, int>>();
+}
+
 ///////////////////////////////////
 #if FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE
 ///////////////////////////////////
@@ -1337,46 +1377,6 @@ TEST(F14FastMap, heterogeneousInsert) {
       TransparentTrackedEqual<1>>>();
 }
 
-template <typename M>
-void runVisitContiguousRangesTest() {
-  M map;
-
-  for (int i = 0; i < 1000; ++i) {
-    map[i] = i;
-    map.erase(i / 2);
-  }
-
-  std::unordered_map<uintptr_t, bool> visited;
-  for (auto& entry : map) {
-    visited[reinterpret_cast<uintptr_t>(&entry)] = false;
-  }
-
-  map.visitContiguousRanges([&](auto b, auto e) {
-    for (auto i = b; i != e; ++i) {
-      auto iter = visited.find(reinterpret_cast<uintptr_t>(i));
-      ASSERT_TRUE(iter != visited.end());
-      EXPECT_FALSE(iter->second);
-      iter->second = true;
-    }
-  });
-}
-
-TEST(F14ValueMap, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14ValueMap<int, int>>();
-}
-
-TEST(F14NodeMap, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14NodeMap<int, int>>();
-}
-
-TEST(F14VectorMap, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14VectorMap<int, int>>();
-}
-
-TEST(F14FastMap, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14FastMap<int, int>>();
-}
-
 ///////////////////////////////////
 #endif // FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE
 ///////////////////////////////////

folly/container/test/F14SetTest.cpp

@@ -117,6 +117,46 @@ TEST(F14Set, getAllocatedMemorySize) {
   }
 }
 
+template <typename S>
+void runVisitContiguousRangesTest() {
+  S set;
+
+  for (int i = 0; i < 1000; ++i) {
+    set.insert(i);
+    set.erase(i / 2);
+  }
+
+  std::unordered_map<uintptr_t, bool> visited;
+  for (auto& entry : set) {
+    visited[reinterpret_cast<uintptr_t>(&entry)] = false;
+  }
+
+  set.visitContiguousRanges([&](auto b, auto e) {
+    for (auto i = b; i != e; ++i) {
+      auto iter = visited.find(reinterpret_cast<uintptr_t>(i));
+      ASSERT_TRUE(iter != visited.end());
+      EXPECT_FALSE(iter->second);
+      iter->second = true;
+    }
+  });
+}
+
+TEST(F14ValueSet, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14ValueSet<int>>();
+}
+
+TEST(F14NodeSet, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14NodeSet<int>>();
+}
+
+TEST(F14VectorSet, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14VectorSet<int>>();
+}
+
+TEST(F14FastSet, visitContiguousRanges) {
+  runVisitContiguousRangesTest<folly::F14FastSet<int>>();
+}
+
 ///////////////////////////////////
 #if FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE
 ///////////////////////////////////
@@ -983,46 +1023,6 @@ TEST(F14FastSet, heterogeneousInsert) {
       TransparentTrackedEqual<1>>>();
 }
 
-template <typename S>
-void runVisitContiguousRangesTest() {
-  S set;
-
-  for (int i = 0; i < 1000; ++i) {
-    set.insert(i);
-    set.erase(i / 2);
-  }
-
-  std::unordered_map<uintptr_t, bool> visited;
-  for (auto& entry : set) {
-    visited[reinterpret_cast<uintptr_t>(&entry)] = false;
-  }
-
-  set.visitContiguousRanges([&](auto b, auto e) {
-    for (auto i = b; i != e; ++i) {
-      auto iter = visited.find(reinterpret_cast<uintptr_t>(i));
-      ASSERT_TRUE(iter != visited.end());
-      EXPECT_FALSE(iter->second);
-      iter->second = true;
-    }
-  });
-}
-
-TEST(F14ValueSet, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14ValueSet<int>>();
-}
-
-TEST(F14NodeSet, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14NodeSet<int>>();
-}
-
-TEST(F14VectorSet, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14VectorSet<int>>();
-}
-
-TEST(F14FastSet, visitContiguousRanges) {
-  runVisitContiguousRangesTest<F14FastSet<int>>();
-}
-
 namespace {
 struct CharArrayHasher {
   template <std::size_t N>