Allow Mojo structs as map keys

mojo/public/cpp/bindings/struct_ptr.h

Index: mojo/public/cpp/bindings/struct_ptr.h
diff --git a/mojo/public/cpp/bindings/struct_ptr.h b/mojo/public/cpp/bindings/struct_ptr.h
index 92f2728a3a08470541c0985e5d12e713e7363dbb..382f368b10ea85bff08847342891793e662fdc44 100644
--- a/mojo/public/cpp/bindings/struct_ptr.h
+++ b/mojo/public/cpp/bindings/struct_ptr.h
@@ -5,6 +5,7 @@
 #ifndef MOJO_PUBLIC_CPP_BINDINGS_STRUCT_PTR_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_STRUCT_PTR_H_
 
+#include <functional>
 #include <new>
 
 #include "base/logging.h"
@@ -14,6 +15,8 @@
 namespace mojo {
 namespace internal {
 
+constexpr size_t kHashSeed = 31;
+
 template <typename Struct>
 class StructHelper {
  public:
@@ -84,6 +87,19 @@ class StructPtr {
     return ptr_->Equals(*other.ptr_);
   }
 
+  // For use in WTF::HashMap only:
+  bool isHashTableDeletedValue() const {

[yzshen1, 2016/09/20 23:44:40]

Does it feel cleaner if we have these WTF-only functions in a separate class,
say StructPtrWTFHelper or something like that?

[yzshen1, 2016/09/21 23:17:52]

This comment is not replied yet.
(Besides, if these methods are outside of the WTF namespace, it seems more
consistent to name them with uppercase initial character.)

[tibell, 2016/09/22 05:17:23]

Done.

+    return ptr_ == reinterpret_cast<Struct*>(1u);
+  }
+
+  // For use in WTF::HashMap only:
+  static void constructDeletedValue(mojo::StructPtr<S>& slot) {
+    // Dirty trick: implant an invalid pointer in |ptr_|. Destructor isn't
+    // called for deleted buckets, so this is okay.
+    new (&slot) StructPtr();
+    slot.ptr_ = reinterpret_cast<Struct*>(1u);
+  }
+
  private:
   // TODO(dcheng): Use an explicit conversion operator.
   typedef Struct* StructPtr::*Testable;
@@ -94,13 +110,6 @@ class StructPtr {
  private:
   friend class internal::StructHelper<Struct>;
 
-  // Forbid the == and != operators explicitly, otherwise StructPtr will be
-  // converted to Testable to do == or != comparison.

[yzshen1, 2016/09/20 23:44:40]

If you would like to allow == and !=, please fix the TODO on line 104. :)

[tibell, 2016/09/21 07:10:53]

Done.

-  template <typename T>
-  bool operator==(const StructPtr<T>& other) const = delete;
-  template <typename T>
-  bool operator!=(const StructPtr<T>& other) const = delete;
-
   void Initialize() {
     DCHECK(!ptr_);
     ptr_ = new Struct();
@@ -122,8 +131,8 @@ class InlinedStructPtr {
  public:
   using Struct = S;
 
-  InlinedStructPtr() : is_null_(true) {}
-  InlinedStructPtr(decltype(nullptr)) : is_null_(true) {}
+  InlinedStructPtr() : state_(NIL) {}
+  InlinedStructPtr(decltype(nullptr)) : state_(NIL) {}
 
   ~InlinedStructPtr() {}
 
@@ -132,7 +141,7 @@ class InlinedStructPtr {
     return *this;
   }
 
-  InlinedStructPtr(InlinedStructPtr&& other) : is_null_(true) { Take(&other); }
+  InlinedStructPtr(InlinedStructPtr&& other) : state_(NIL) { Take(&other); }
   InlinedStructPtr& operator=(InlinedStructPtr&& other) {
     Take(&other);
     return *this;
@@ -144,26 +153,26 @@ class InlinedStructPtr {
   }
 
   void reset() {
-    is_null_ = true;
+    state_ = NIL;
     value_. ~Struct();
     new (&value_) Struct();
   }
 
-  bool is_null() const { return is_null_; }
+  bool is_null() const { return state_ == NIL; }
 
   Struct& operator*() const {
-    DCHECK(!is_null_);
+    DCHECK(state_ == VALID);
     return value_;
   }
   Struct* operator->() const {
-    DCHECK(!is_null_);
+    DCHECK(state_ == VALID);
     return &value_;
   }
   Struct* get() const { return &value_; }
 
   void Swap(InlinedStructPtr* other) {
     std::swap(value_, other->value_);
-    std::swap(is_null_, other->is_null_);
+    std::swap(state_, other->state_);
   }
 
   InlinedStructPtr Clone() const {
@@ -175,36 +184,64 @@ class InlinedStructPtr {
     return value_.Equals(other.value_);
   }
 
+  // For use in WTF::HashMap only:
+  bool isHashTableDeletedValue() const { return state_ == DELETED; }
+
+  // For use in WTF::HashMap only:
+  static void constructDeletedValue(mojo::InlinedStructPtr<S>& slot) {
+    slot = InlinedStructPtr();

[Sam McNally, 2016/09/21 04:57:58]

Should this be constructing an InlinedStructPtr in &slot like the StructPtr
version?

[tibell, 2016/09/21 07:10:53]

Done.

+    slot.state_ = DELETED;
+  }
+
  private:
   // TODO(dcheng): Use an explicit conversion operator.
   typedef Struct InlinedStructPtr::*Testable;
 
  public:
-  operator Testable() const { return is_null_ ? 0 : &InlinedStructPtr::value_; }
+  operator Testable() const {
+    return is_null() ? 0 : &InlinedStructPtr::value_;
+  }
 
  private:
   friend class internal::StructHelper<Struct>;
 
-  // Forbid the == and != operators explicitly, otherwise InlinedStructPtr will

[yzshen1, 2016/09/20 23:44:40]

Please consider fixing TODO on line 197. :)

[tibell, 2016/09/21 07:10:53]

Done.

-  // be converted to Testable to do == or != comparison.
-  template <typename T>
-  bool operator==(const InlinedStructPtr<T>& other) const = delete;
-  template <typename T>
-  bool operator!=(const InlinedStructPtr<T>& other) const = delete;
-
-  void Initialize() { is_null_ = false; }
+  void Initialize() { state_ = VALID; }
 
   void Take(InlinedStructPtr* other) {
     reset();
     Swap(other);
   }
 
+  enum State {
+    VALID,
+    NIL,

[yzshen1, 2016/09/20 23:44:40]

I think NULL might be more commonly used.

[tibell, 2016/09/21 07:10:53]

It will collide with the standard NULL macro. :)

[yzshen1, 2016/09/21 23:17:52]

Ah, of course. :)

+    DELETED,  // For use in WTF::HashMap only
+  };
+
   mutable Struct value_;
-  bool is_null_;
+  State state_;
 
   DISALLOW_COPY_AND_ASSIGN(InlinedStructPtr);
 };
 
 }  // namespace mojo
 
+namespace std {
+
+template <typename T>
+struct hash<mojo::StructPtr<T>> {
+  size_t operator()(const mojo::StructPtr<T>& value) const {
+    return mojo::internal::Hash(mojo::internal::kHashSeed, *value.get());
+  }
+};
+
+template <typename T>
+struct hash<mojo::InlinedStructPtr<T>> {
+  size_t operator()(const mojo::InlinedStructPtr<T>& value) const {
+    return mojo::internal::Hash(mojo::internal::kHashSeed, *value.get());
+  }
+};
+
+}  // namespace std
+
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_STRUCT_PTR_H_