[multimapcache] Change multi-map cache to use a std::function instead of CRTP.

This makes it so one uses a passed in std::function, instead of an impl class to
map a key to a list of values to be cached.

Author: gottesmm

include/swift/Basic/MultiMapCache.h

@@ -18,40 +18,35 @@
 
 namespace swift {
 
-/// A CRTP write-once multi-map cache that can be small. It uses a DenseMap
+/// A write-once multi-map cache that can be small. It uses a DenseMap
 /// internally, so it can be used as a cache without needing to be frozen like
-/// FrozenMultiMap (which uses only a vector internally). The Impl class
-/// implements the method constructValuesForKey that is used to compute the
-/// actual cache value.
-///
-/// NOTE: constructValuesForKeys is assumed to take a KeyTy and a
-/// SmallVectorImpl<ValueTy>. It must append all results to that accumulator and
-/// not read any contents of the accumulator.
+/// FrozenMultiMap (which uses only a vector internally). The cached value is
+/// computed by a passed in std::function. The std::function is able to map
+/// multiple values to a specific key via the out array.
 ///
 /// NOTE: We store the (size, length) of each ArrayRef<ValueTy> instead of
 /// storing the ArrayRef to avoid data invalidation issues caused by SmallVector
 /// switching from small to large representations.
 ///
 /// For an example of a subclass implementation see:
 /// unittests/Basic/MultiMapCacheTest.cpp.
-template <typename ImplType, typename KeyTy, typename ValueTy,
+template <typename KeyTy, typename ValueTy,
           typename MapTy =
               llvm::DenseMap<KeyTy, Optional<std::tuple<unsigned, unsigned>>>,
           typename VectorTy = std::vector<ValueTy>,
           typename VectorTyImpl = VectorTy>
 class MultiMapCache {
+  std::function<bool(const KeyTy &, VectorTyImpl &)> function;
   MapTy valueToDataOffsetIndexMap;
   VectorTy data;
 
   constexpr static unsigned ArrayStartOffset = 0;
   constexpr static unsigned ArrayLengthOffset = 1;
 
-  constexpr ImplType &asImpl() const {
-    auto *self = const_cast<MultiMapCache *>(this);
-    return reinterpret_cast<ImplType &>(*self);
-  }
-
 public:
+  MultiMapCache(std::function<bool(const KeyTy &, VectorTyImpl &)> function)
+      : function(function) {}
+
   void clear() {
     valueToDataOffsetIndexMap.clear();
     data.clear();
@@ -81,7 +76,7 @@ class MultiMapCache {
 
     // We assume that constructValuesForKey /only/ inserts to the end of data
     // and does not inspect any other values in the data array.
-    if (!asImpl().constructValuesForKey(key, data)) {
+    if (!function(key, data)) {
       return None;
     }
 
@@ -94,9 +89,9 @@ class MultiMapCache {
   }
 };
 
-template <typename ImplType, typename KeyTy, typename ValueTy>
+template <typename KeyTy, typename ValueTy>
 using SmallMultiMapCache = MultiMapCache<
-    ImplType, KeyTy, ValueTy,
+    KeyTy, ValueTy,
     llvm::SmallDenseMap<KeyTy, Optional<std::tuple<unsigned, unsigned>>, 8>,
     SmallVector<ValueTy, 32>, SmallVectorImpl<ValueTy>>;
 

unittests/Basic/MultiMapCacheTest.cpp

@@ -17,24 +17,49 @@
 
 using namespace swift;
 
-namespace {
-
-/// A multimap cache that caches the initial 4 powers of each key.
-struct PowerMultiMapCache
-    : public MultiMapCache<PowerMultiMapCache, unsigned, unsigned> {
-  bool constructValuesForKey(unsigned key, std::vector<unsigned> &data) {
-    // Construct the first 3 powers of key.
-    data.push_back(key);
-    data.push_back(key * key);
-    data.push_back(key * key * key);
-    return true;
+TEST(MultiMapCache, powersTest) {
+  std::function<bool(unsigned, std::vector<unsigned> &)> cacheCompute =
+      [&](unsigned key, std::vector<unsigned> &outArray) {
+        outArray.push_back(key);
+        outArray.push_back(key * key);
+        outArray.push_back(key * key * key);
+        return true;
+      };
+  MultiMapCache<unsigned, unsigned> cache(cacheCompute);
+
+  EXPECT_TRUE(cache.empty());
+  EXPECT_EQ(cache.size(), 0u);
+  for (unsigned index : range(1, 256)) {
+    auto array = *cache.get(index);
+    for (unsigned power : array) {
+      EXPECT_EQ(power % index, 0);
+    }
+  }
+  EXPECT_FALSE(cache.empty());
+  EXPECT_EQ(cache.size(), 255);
+  for (unsigned index : range(1, 256)) {
+    auto array = *cache.get(index);
+    for (unsigned power : array) {
+      EXPECT_EQ(power % index, 0);
+    }
   }
-};
+  EXPECT_FALSE(cache.empty());
+  EXPECT_EQ(cache.size(), 255);
 
-} // end anonymous namespace
+  cache.clear();
+  EXPECT_TRUE(cache.empty());
+  EXPECT_EQ(cache.size(), 0);
+}
 
-TEST(MultiMapCache, powersTest) {
-  PowerMultiMapCache cache;
+TEST(MultiMapCache, smallTest) {
+  std::function<bool(unsigned, SmallVectorImpl<unsigned> &)> cacheCompute =
+      [&](unsigned key, SmallVectorImpl<unsigned> &outArray) {
+        outArray.push_back(key);
+        outArray.push_back(key * key);
+        outArray.push_back(key * key * key);
+        return true;
+      };
+  SmallMultiMapCache<unsigned, unsigned> cache(cacheCompute);
 
   EXPECT_TRUE(cache.empty());
   EXPECT_EQ(cache.size(), 0u);