Provide default traits for PersistentValueMap

Re-try of issue 201643003. This caused linker errors on Win64, since the
linker insists on seeing the StrongMapTrait method implementations even
though they are never used. This will provide default implementations
for them.

R=dcarney@chromium.org

BUG=

Review URL: https://codereview.chromium.org/204343006

Patch from Daniel Vogelheim <vogelheim@chromium.org>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20084 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8-util.h

@@ -29,6 +29,7 @@
 #define V8_UTIL_H_
 
 #include "v8.h"
+#include <map>
 
 /**
  * Support for Persistent containers.
@@ -42,6 +43,90 @@ namespace v8 {
 typedef uintptr_t PersistentContainerValue;
 static const uintptr_t kPersistentContainerNotFound = 0;
 
+
+/**
+ * A default trait implemenation for PersistentValueMap which uses std::map
+ * as a backing map.
+ *
+ * Users will have to implement their own weak callbacks & dispose traits.
+ */
+template<typename K, typename V>
+class StdMapTraits {
+ public:
+  // STL map & related:
+  typedef std::map<K, PersistentContainerValue> Impl;
+  typedef typename Impl::iterator Iterator;
+
+  static bool Empty(Impl* impl) { return impl->empty(); }
+  static size_t Size(Impl* impl) { return impl->size(); }
+  static void Swap(Impl& a, Impl& b) { std::swap(a, b); }  // NOLINT
+  static Iterator Begin(Impl* impl) { return impl->begin(); }
+  static Iterator End(Impl* impl) { return impl->end(); }
+  static K Key(Iterator it) { return it->first; }
+  static PersistentContainerValue Value(Iterator it) { return it->second; }
+  static PersistentContainerValue Set(Impl* impl, K key,
+      PersistentContainerValue value) {
+    std::pair<Iterator, bool> res = impl->insert(std::make_pair(key, value));
+    PersistentContainerValue old_value = kPersistentContainerNotFound;
+    if (!res.second) {
+      old_value = res.first->second;
+      res.first->second = value;
+    }
+    return old_value;
+  }
+  static PersistentContainerValue Get(Impl* impl, K key) {
+    Iterator it = impl->find(key);
+    if (it == impl->end()) return kPersistentContainerNotFound;
+    return it->second;
+  }
+  static PersistentContainerValue Remove(Impl* impl, K key) {
+    Iterator it = impl->find(key);
+    if (it == impl->end()) return kPersistentContainerNotFound;
+    PersistentContainerValue value = it->second;
+    impl->erase(it);
+    return value;
+  }
+};
+
+
+/**
+ * A default trait implementation for PersistentValueMap, which inherits
+ * a std:map backing map from StdMapTraits and holds non-weak persistent
+ * objects.
+ *
+ * Users have to implement their own dispose trait.
+ */
+template<typename K, typename V>
+class StrongMapTraits : public StdMapTraits<K, V> {
+ public:
+  // Weak callback & friends:
+  static const bool kIsWeak = false;
+  typedef typename StdMapTraits<K, V>::Impl Impl;
+  typedef void WeakCallbackDataType;
+  static WeakCallbackDataType* WeakCallbackParameter(
+      Impl* impl, const K& key, Local<V> value);
+  static Impl* ImplFromWeakCallbackData(
+      const WeakCallbackData<V, WeakCallbackDataType>& data);
+  static K KeyFromWeakCallbackData(
+      const WeakCallbackData<V, WeakCallbackDataType>& data);
+  static void DisposeCallbackData(WeakCallbackDataType* data);
+};
+
+
+/**
+ * A default trait implementation for PersistentValueMap, with a std::map
+ * backing map, non-weak persistents as values, and no special dispose
+ * handling. Can be used as-is.
+ */
+template<typename K, typename V>
+class DefaultPersistentValueMapTraits : public StrongMapTraits<K, V> {
+ public:
+  typedef typename StrongMapTraits<K, V>::Impl Impl;
+  static void Dispose(Isolate* isolate, UniquePersistent<V> value,
+      Impl* impl, K key) { }
+};
+
+
 /**
  * A map wrapper that allows using UniquePersistent as a mapped value.
  * C++11 embedders don't need this class, as they can use UniquePersistent
@@ -52,7 +137,7 @@ static const uintptr_t kPersistentContainerNotFound = 0;
  * PersistentContainerValue, with all conversion into and out of V8
  * handles being transparently handled by this class.
  */
-template<class K, class V, class Traits>
+template<typename K, typename V, typename Traits>
 class PersistentValueMap {
  public:
   V8_INLINE explicit PersistentValueMap(Isolate* isolate) : isolate_(isolate) {}
@@ -66,6 +151,11 @@ class PersistentValueMap {
    */
   V8_INLINE size_t Size() { return Traits::Size(&impl_); }
 
+  /**
+   * Return whether the map holds weak persistents.
+   */
+  V8_INLINE bool IsWeak() { return Traits::kIsWeak; }
+
   /**
    * Get value stored in map.
    */
@@ -85,13 +175,21 @@ class PersistentValueMap {
    * Return true if a value was found.
    */
   V8_INLINE bool SetReturnValue(const K& key,
-    ReturnValue<Value>& returnValue);
+      ReturnValue<Value>& returnValue) {
+    PersistentContainerValue value = Traits::Get(&impl_, key);
+    bool hasValue = value != 0;
+    if (hasValue) {
+      returnValue.SetInternal(
+          *reinterpret_cast<internal::Object**>(FromVal(value)));
+    }
+    return hasValue;
+  }
 
   /**
    * Call Isolate::SetReference with the given parent and the map value.
    */
   V8_INLINE void SetReference(const K& key,
-      const v8::Persistent<v8::Object>& parent) {
+      const Persistent<Object>& parent) {
     GetIsolate()->SetReference(
       reinterpret_cast<internal::Object**>(parent.val_),
       reinterpret_cast<internal::Object**>(FromVal(Traits::Get(&impl_, key))));
@@ -125,7 +223,19 @@ class PersistentValueMap {
   * Traverses the map repeatedly,
   * in case side effects of disposal cause insertions.
   **/
-  void Clear();
+  void Clear() {
+    typedef typename Traits::Iterator It;
+    HandleScope handle_scope(isolate_);
+    // TODO(dcarney): figure out if this swap and loop is necessary.
+    while (!Traits::Empty(&impl_)) {
+      typename Traits::Impl impl;
+      Traits::Swap(impl_, impl);
+      for (It i = Traits::Begin(&impl); i != Traits::End(&impl); ++i) {
+        Traits::Dispose(isolate_, Release(Traits::Value(i)).Pass(), &impl,
+          Traits::Key(i));
+      }
+    }
+  }
 
  private:
   PersistentValueMap(PersistentValueMap&);
@@ -147,10 +257,19 @@ class PersistentValueMap {
   }
 
   static void WeakCallback(
-      const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data);
+      const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data) {
+    if (Traits::kIsWeak) {
+      typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data);
+      K key = Traits::KeyFromWeakCallbackData(data);
+      PersistentContainerValue value = Traits::Remove(impl, key);
+      Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key);
+    }
+  }
+
   V8_INLINE static V* FromVal(PersistentContainerValue v) {
     return reinterpret_cast<V*>(v);
   }
+
   V8_INLINE static PersistentContainerValue ClearAndLeak(
       UniquePersistent<V>* persistent) {
     V* v = persistent->val_;
@@ -177,41 +296,58 @@ class PersistentValueMap {
   typename Traits::Impl impl_;
 };
 
-template <class K, class V, class Traits>
-bool PersistentValueMap<K, V, Traits>::SetReturnValue(const K& key,
-    ReturnValue<Value>& returnValue) {
-  PersistentContainerValue value = Traits::Get(&impl_, key);
-  bool hasValue = value != 0;
-  if (hasValue) {
-    returnValue.SetInternal(
-        *reinterpret_cast<internal::Object**>(FromVal(value)));
-  }
-  return hasValue;
+
+/**
+ * A map that uses UniquePersistent as value and std::map as the backing
+ * implementation. Persistents are held non-weak.
+ *
+ * C++11 embedders don't need this class, as they can use
+ * UniquePersistent directly in std containers.
+ */
+template<typename K, typename V,
+    typename Traits = DefaultPersistentValueMapTraits<K, V> >
+class StdPersistentValueMap : public PersistentValueMap<K, V, Traits> {
+ public:
+  explicit StdPersistentValueMap(Isolate* isolate)
+      : PersistentValueMap<K, V, Traits>(isolate) {}
+};
+
+
+/**
+ * Empty default implementations for StrongTraits methods.
+ *
+ * These should not be necessary, since they're only used in code that
+ * is surrounded by if(Traits::kIsWeak), which for StrongMapTraits is
+ * compile-time false. Most compilers can live without them; however
+ * the compiler we use from 64-bit Win differs.
+ *
+ * TODO(vogelheim): Remove these once they're no longer necessary.
+ */
+template<typename K, typename V>
+typename StrongMapTraits<K, V>::WeakCallbackDataType*
+    StrongMapTraits<K, V>::WeakCallbackParameter(
+        Impl* impl, const K& key, Local<V> value) {
+  return NULL;
 }
 
-template <class K, class V, class Traits>
-void PersistentValueMap<K, V, Traits>::Clear() {
-  typedef typename Traits::Iterator It;
-  HandleScope handle_scope(isolate_);
-  // TODO(dcarney): figure out if this swap and loop is necessary.
-  while (!Traits::Empty(&impl_)) {
-    typename Traits::Impl impl;
-    Traits::Swap(impl_, impl);
-    for (It i = Traits::Begin(&impl); i != Traits::End(&impl); ++i) {
-      Traits::Dispose(isolate_, Release(Traits::Value(i)).Pass(), &impl,
-        Traits::Key(i));
-    }
-  }
+
+template<typename K, typename V>
+typename StrongMapTraits<K, V>::Impl*
+    StrongMapTraits<K, V>::ImplFromWeakCallbackData(
+        const WeakCallbackData<V, WeakCallbackDataType>& data) {
+  return NULL;
+}
+
+
+template<typename K, typename V>
+K StrongMapTraits<K, V>::KeyFromWeakCallbackData(
+    const WeakCallbackData<V, WeakCallbackDataType>& data) {
+  return K();
 }
 
 
-template <class K, class V, class Traits>
-void PersistentValueMap<K, V, Traits>::WeakCallback(
-    const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data) {
-  typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data);
-  K key = Traits::KeyFromWeakCallbackData(data);
-  PersistentContainerValue value = Traits::Remove(impl, key);
-  Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key);
+template<typename K, typename V>
+void StrongMapTraits<K, V>::DisposeCallbackData(WeakCallbackDataType* data) {
 }
 
 }  // namespace v8

test/cctest/test-api.cc

@@ -3445,47 +3445,15 @@ THREADED_TEST(UniquePersistent) {
 }
 
 
-template<typename K, typename V, bool is_weak>
-class StdPersistentValueMapTraits {
+template<typename K, typename V>
+class WeakStdMapTraits : public v8::StdMapTraits<K, V> {
  public:
-  static const bool kIsWeak = is_weak;
-  typedef v8::PersistentContainerValue VInt;
-  typedef std::map<K, VInt> Impl;
+  typedef typename v8::DefaultPersistentValueMapTraits<K, V>::Impl Impl;
+  static const bool kIsWeak = true;
   struct WeakCallbackDataType {
     Impl* impl;
     K key;
   };
-  typedef typename Impl::iterator Iterator;
-  static bool Empty(Impl* impl) { return impl->empty(); }
-  static size_t Size(Impl* impl) { return impl->size(); }
-  static void Swap(Impl& a, Impl& b) { std::swap(a, b); }  // NOLINT
-  static Iterator Begin(Impl* impl) { return impl->begin(); }
-  static Iterator End(Impl* impl) { return impl->end(); }
-  static K Key(Iterator it) { return it->first; }
-  static VInt Value(Iterator it) { return it->second; }
-  static VInt Set(Impl* impl, K key, VInt value) {
-    std::pair<Iterator, bool> res = impl->insert(std::make_pair(key, value));
-    VInt old_value = v8::kPersistentContainerNotFound;
-    if (!res.second) {
-      old_value = res.first->second;
-      res.first->second = value;
-    }
-    return old_value;
-  }
-  static VInt Get(Impl* impl, K key) {
-    Iterator it = impl->find(key);
-    if (it == impl->end()) return v8::kPersistentContainerNotFound;
-    return it->second;
-  }
-  static VInt Remove(Impl* impl, K key) {
-    Iterator it = impl->find(key);
-    if (it == impl->end()) return v8::kPersistentContainerNotFound;
-    VInt value = it->second;
-    impl->erase(it);
-    return value;
-  }
-  static void Dispose(v8::Isolate* isolate, v8::UniquePersistent<V> value,
-      Impl* impl, K key) {}
   static WeakCallbackDataType* WeakCallbackParameter(
       Impl* impl, const K& key, Local<V> value) {
     WeakCallbackDataType* data = new WeakCallbackDataType;
@@ -3504,15 +3472,15 @@ class StdPersistentValueMapTraits {
   static void DisposeCallbackData(WeakCallbackDataType* data) {
     delete data;
   }
+  static void Dispose(v8::Isolate* isolate, v8::UniquePersistent<V> value,
+      Impl* impl, K key) { }
 };
 
 
-template<bool is_weak>
+template<typename Map>
 static void TestPersistentValueMap() {
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
-  typedef v8::PersistentValueMap<int, v8::Object,
-      StdPersistentValueMapTraits<int, v8::Object, is_weak> > Map;
   Map map(isolate);
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
@@ -3538,7 +3506,7 @@ static void TestPersistentValueMap() {
     CHECK_EQ(1, static_cast<int>(map.Size()));
   }
   CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
-  if (is_weak) {
+  if (map.IsWeak()) {
     reinterpret_cast<v8::internal::Isolate*>(isolate)->heap()->
         CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
   } else {
@@ -3550,8 +3518,13 @@ static void TestPersistentValueMap() {
 
 
 TEST(PersistentValueMap) {
-  TestPersistentValueMap<false>();
-  TestPersistentValueMap<true>();
+  // Default case, w/o weak callbacks:
+  TestPersistentValueMap<v8::StdPersistentValueMap<int, v8::Object> >();
+
+  // Custom traits with weak callbacks:
+  typedef v8::StdPersistentValueMap<int, v8::Object,
+      WeakStdMapTraits<int, v8::Object> > WeakPersistentValueMap;
+  TestPersistentValueMap<WeakPersistentValueMap>();
 }