Drop ambiguous MaybeHandle comparison and hashing ops.

The default equality comparison operators and hashing functions for
Handles are ambiguous. The intended semantics might have either been
based on Handle locations or on object identity. This is why such
operators do not exist on Handle. The same argument applies to the
MaybeHandle class as well. Comments in that regard were also added.

R=bmeurer@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#30468}

diff --git a/src/compiler/js-operator.cc b/src/compiler/js-operator.cc
index f96584a..f551b88 100644 (file)
--- a/src/compiler/js-operator.cc
+++ b/src/compiler/js-operator.cc
@@ -15,7 +15,8 @@ namespace internal {
 namespace compiler {
 
 bool operator==(VectorSlotPair const& lhs, VectorSlotPair const& rhs) {
-  return lhs.slot() == rhs.slot() && lhs.vector() == rhs.vector();
+  return lhs.slot() == rhs.slot() &&
+         lhs.vector().location() == rhs.vector().location();
 }
 
 
@@ -25,7 +26,7 @@ bool operator!=(VectorSlotPair const& lhs, VectorSlotPair const& rhs) {
 
 
 size_t hash_value(VectorSlotPair const& p) {
-  return base::hash_combine(p.slot(), p.vector());
+  return base::hash_combine(p.slot(), p.vector().location());
 }
 
 

diff --git a/src/compiler/js-operator.h b/src/compiler/js-operator.h
index 0f4fd35..d583f64 100644 (file)
--- a/src/compiler/js-operator.h
+++ b/src/compiler/js-operator.h
@@ -27,16 +27,15 @@ class VectorSlotPair {
 
   bool IsValid() const { return !vector_.is_null(); }
 
-  MaybeHandle<TypeFeedbackVector> vector() const { return vector_; }
+  Handle<TypeFeedbackVector> vector() const { return vector_; }
   FeedbackVectorICSlot slot() const { return slot_; }
 
   int index() const {
-    Handle<TypeFeedbackVector> vector;
-    return vector_.ToHandle(&vector) ? vector->GetIndex(slot_) : -1;
+    return vector_.is_null() ? -1 : vector_->GetIndex(slot_);
   }
 
  private:
-  const MaybeHandle<TypeFeedbackVector> vector_;
+  const Handle<TypeFeedbackVector> vector_;
   const FeedbackVectorICSlot slot_;
 };
 

diff --git a/src/compiler/js-type-feedback-lowering.cc b/src/compiler/js-type-feedback-lowering.cc
index 2f106df..6f43c4e 100644 (file)
--- a/src/compiler/js-type-feedback-lowering.cc
+++ b/src/compiler/js-type-feedback-lowering.cc
@@ -41,10 +41,9 @@ Reduction JSTypeFeedbackLowering::ReduceJSLoadNamed(Node* node) {
   // We need to make optimistic assumptions to continue.
   if (!(flags() & kDeoptimizationEnabled)) return NoChange();
   LoadNamedParameters const& p = LoadNamedParametersOf(node->op());
-  Handle<TypeFeedbackVector> vector;
-  if (!p.feedback().vector().ToHandle(&vector)) return NoChange();
+  if (p.feedback().vector().is_null()) return NoChange();
   if (p.name().is_identical_to(factory()->length_string())) {
-    LoadICNexus nexus(vector, p.feedback().slot());
+    LoadICNexus nexus(p.feedback().vector(), p.feedback().slot());
     MapHandleList maps;
     if (nexus.ExtractMaps(&maps) > 0) {
       for (Handle<Map> map : maps) {

diff --git a/src/handles.h b/src/handles.h
index 312b750..85fa839 100644 (file)
--- a/src/handles.h
+++ b/src/handles.h
@@ -71,8 +71,14 @@ class HandleBase {
 // ----------------------------------------------------------------------------
 // A Handle provides a reference to an object that survives relocation by
 // the garbage collector.
-// Handles are only valid within a HandleScope.
-// When a handle is created for an object a cell is allocated in the heap.
+//
+// Handles are only valid within a HandleScope. When a handle is created
+// for an object a cell is allocated in the current HandleScope.
+//
+// Also note that Handles do not provide default equality comparison or hashing
+// operators on purpose. Such operators would be misleading, because intended
+// semantics is ambiguous between Handle location and object identity. Instead
+// use either {is_identical_to} or {location} explicitly.
 template <typename T>
 class Handle final : public HandleBase {
  public:
@@ -160,7 +166,12 @@ V8_INLINE Handle<T> handle(T* object) {
 // A Handle can be converted into a MaybeHandle. Converting a MaybeHandle
 // into a Handle requires checking that it does not point to NULL.  This
 // ensures NULL checks before use.
+//
 // Do not use MaybeHandle as argument type.
+//
+// Also note that Handles do not provide default equality comparison or hashing
+// operators on purpose. Such operators would be misleading, because intended
+// semantics is ambiguous between Handle location and object identity.
 template <typename T>
 class MaybeHandle final {
  public:
@@ -211,15 +222,6 @@ class MaybeHandle final {
 
   bool is_null() const { return location_ == nullptr; }
 
-  template <typename S>
-  V8_INLINE bool operator==(MaybeHandle<S> that) const {
-    return this->location_ == that.location_;
-  }
-  template <typename S>
-  V8_INLINE bool operator!=(MaybeHandle<S> that) const {
-    return this->location_ != that.location_;
-  }
-
  protected:
   T** location_ = nullptr;
 
@@ -227,19 +229,10 @@ class MaybeHandle final {
   // other's location_.
   template <typename>
   friend class MaybeHandle;
-  // Utility functions are allowed to access location_.
-  template <typename S>
-  friend size_t hash_value(MaybeHandle<S>);
 };
 
-template <typename T>
-V8_INLINE size_t hash_value(MaybeHandle<T> maybe_handle) {
-  uintptr_t v = bit_cast<uintptr_t>(maybe_handle.location_);
-  DCHECK_EQ(0u, v & ((1u << kPointerSizeLog2) - 1));
-  return v >> kPointerSizeLog2;
-}
-
 
+// ----------------------------------------------------------------------------
 // A stack-allocated class that governs a number of local handles.
 // After a handle scope has been created, all local handles will be
 // allocated within that handle scope until either the handle scope is