From e8b2c23d2406be8d47f17040534ac70b5e64491e Mon Sep 17 00:00:00 2001
From: Jamieson Pryor <jwhpryor@google.com>
Date: Wed, 23 Oct 2024 16:18:06 -0700
Subject: [PATCH] Fixing ambiguous equality operators for C++20 migration.

This is an issue specifically on CRTP classes templated equality operators that is resolved by making equality operators free standing functions.

e.g.
```
implementation/local_object_test.cc:194:22: error: use of overloaded operator '==' is ambiguous (with operand types 'LocalObject<kClass>' and 'LocalObject<kClass2>')
  EXPECT_FALSE(val_1 == val_2);
               ~~~~~ ^  ~~~~~
```

See https://github.com/google/jni-bind/issues/346.

Sample failure https://github.com/google/jni-bind/actions/runs/11474529819/job/31930608615?pr=345.

PiperOrigin-RevId: 689141602
---
 implementation/BUILD                  | 14 ++---
 implementation/array.h                | 82 +++++++++++++++++++--------
 implementation/array_test.cc          |  3 +-
 implementation/array_view.h           | 21 +++++--
 implementation/class.h                | 56 +++++++++++-------
 implementation/class_loader.h         | 45 ++++++++++-----
 implementation/constructor.h          | 14 +++--
 implementation/default_class_loader.h | 48 +++++++++-------
 implementation/field_ref.h            |  2 +
 implementation/forward_declarations.h |  9 ++-
 implementation/global_class_loader.h  |  1 +
 implementation/global_object.h        |  3 +
 implementation/global_object_test.cc  | 19 +++++++
 implementation/jvm.h                  | 25 ++++----
 implementation/local_array.h          | 30 +++++-----
 implementation/local_class_loader.h   |  2 +
 implementation/local_object.h         | 13 +----
 implementation/local_object_test.cc   | 28 ++++-----
 implementation/no_class_specified.h   | 12 +++-
 implementation/promotion_mechanics.h  | 56 ++++++++++++------
 implementation/ref_base.h             |  2 +-
 21 files changed, 311 insertions(+), 174 deletions(-)

diff --git a/implementation/BUILD b/implementation/BUILD
index e570ee72..5044af27 100644
--- a/implementation/BUILD
+++ b/implementation/BUILD
@@ -83,6 +83,7 @@ cc_library(
     deps = [
         ":array_type_conversion",
         "//:jni_dep",
+        "//implementation/jni_helper:get_array_element_result",
         "//implementation/jni_helper:jni_array_helper",
         "//implementation/jni_helper:lifecycle",
     ],
@@ -296,23 +297,19 @@ cc_library(
     name = "field_ref",
     hdrs = ["field_ref.h"],
     deps = [
-        ":class_ref",
+        ":default_class_loader",
         ":field_selection",
         ":id",
         ":id_type",
         ":no_idx",
         ":promotion_mechanics_tags",
-        ":proxy",
         ":proxy_convenience_aliases",
         ":ref_base",
         ":signature",
         "//:jni_dep",
         "//implementation/jni_helper",
         "//implementation/jni_helper:field_value_getter",
-        "//implementation/jni_helper:static_field_value",
         "//metaprogramming:double_locked_value",
-        "//metaprogramming:optional_wrap",
-        "//metaprogramming:queryable_map",
     ],
 )
 
@@ -365,10 +362,8 @@ cc_library(
     name = "forward_declarations",
     hdrs = ["forward_declarations.h"],
     deps = [
-        ":default_class_loader",
         ":id_type",
-        ":jvm",
-        "//implementation/jni_helper:lifecycle_object",
+        "//implementation/jni_helper:lifecycle",
     ],
 )
 
@@ -397,8 +392,10 @@ cc_library(
     name = "global_object",
     hdrs = ["global_object.h"],
     deps = [
+        ":default_class_loader",
         ":forward_declarations",
         ":jni_type",
+        ":jvm",
         ":local_object",
         ":promotion_mechanics",
         ":ref_base",
@@ -724,6 +721,7 @@ cc_library(
         ":forward_declarations",
         ":jvm",
         ":promotion_mechanics_tags",
+        "//:jni_dep",
         "//implementation/jni_helper:lifecycle",
     ],
 )
diff --git a/implementation/array.h b/implementation/array.h
index a1b925ed..32b48f91 100644
--- a/implementation/array.h
+++ b/implementation/array.h
@@ -34,11 +34,13 @@ struct Array;
 template <std::size_t kRank>
 struct Rank {};
 
+struct ArrayBase {};
+
 ////////////////////////////////////////////////////////////////////////////////
 // Array Non-Object Implementation.
 ////////////////////////////////////////////////////////////////////////////////
 template <typename RawType, std::size_t kRank>
-struct ArrayNonObjectTypeImpl {
+struct ArrayNonObjectTypeImpl : ArrayBase {
   RawType raw_;
 
   constexpr ArrayNonObjectTypeImpl(RawType raw) : raw_(raw) {}
@@ -52,21 +54,48 @@ struct ArrayNonObjectTypeImpl {
                   "JNI Error: Invalid array declaration, use Array { type{}, "
                   "Rank<kRank>{} }.");
   }
+};
 
-  template <typename RawTypeRhs, std::size_t kRankRhs>
-  constexpr bool operator==(const Array<RawTypeRhs, kRankRhs>& rhs) const {
-    if constexpr (std::is_same_v<RawType, RawTypeRhs>) {
-      return (raw_ == rhs.raw_);
-    }
-    return false;
-  }
+template <typename T>
+struct ArrayComparisonHelper {};
 
-  template <typename RawTypeRhs, std::size_t kRankRhs>
-  constexpr bool operator!=(const Array<RawTypeRhs, kRankRhs>& rhs) const {
-    return !(*this == rhs);
-  }
+template <typename RawType, std::size_t kRank>
+struct ArrayComparisonHelper<Array<ArrayNonObjectTypeImpl<RawType, kRank>>> {
+  using type = RawType;
+};
+
+template <typename RawType, std::size_t kRank>
+struct ArrayComparisonHelper<Array<RawType, kRank>> {
+  using type = RawType;
+};
+
+template <typename T>
+using ArrayComparisonHelper_t = typename ArrayComparisonHelper<T>::type;
+
+template <typename T1, typename T2>
+static constexpr bool IsArrayComparable() {
+  return std::is_base_of_v<ArrayBase, T1> && std::is_base_of_v<ArrayBase, T2>;
 };
 
+////////////////////////////////////////////////////////////////////////////////
+// Equality operators.
+////////////////////////////////////////////////////////////////////////////////
+template <typename T1, typename T2>
+constexpr std::enable_if_t<IsArrayComparable<T1, T2>(), bool> operator==(
+    const T1& lhs, const T2& rhs) {
+  if constexpr (std::is_same_v<ArrayComparisonHelper_t<T1>,
+                               ArrayComparisonHelper_t<T2>>) {
+    return (lhs.raw_ == rhs.raw_);
+  }
+  return false;
+}
+
+template <typename T1, typename T2>
+constexpr std::enable_if_t<IsArrayComparable<T1, T2>(), bool> operator!=(
+    const T1& lhs, const T2& rhs) {
+  return !(lhs == rhs);
+}
+
 // Primitive array implementaiton.
 template <typename T, std::size_t kRank, bool HoldsObject>
 struct ArrayImpl : public ArrayNonObjectTypeImpl<T, kRank>,
@@ -79,27 +108,32 @@ struct ArrayImpl : public ArrayNonObjectTypeImpl<T, kRank>,
 // Array Object Implementation.
 ////////////////////////////////////////////////////////////////////////////////
 template <typename RawType, std::size_t kRank_>
-struct ArrayImpl<RawType, kRank_, true> : public ArrayTag<jobjectArray> {
+struct ArrayImpl<RawType, kRank_, true> : public ArrayTag<jobjectArray>,
+                                          ArrayBase {
   RawType raw_;
 
   constexpr ArrayImpl(RawType raw) : raw_(raw) {}
 
   template <std::size_t kRank>
   constexpr ArrayImpl(RawType raw, Rank<kRank>) : raw_(raw) {}
+};
 
-  template <typename RawTypeRhs, std::size_t kRank>
-  constexpr bool operator==(const Array<RawTypeRhs, kRank>& rhs) const {
-    if constexpr (std::is_same_v<RawType, RawTypeRhs>) {
-      return (raw_ == rhs.raw_);
-    }
-    return false;
+template <typename RawTypeLhs, std::size_t kRankLhs, typename RawTypeRhs,
+          std::size_t kRankRhs>
+constexpr bool operator==(const ArrayImpl<RawTypeLhs, kRankLhs, true>& lhs,
+                          const ArrayImpl<RawTypeRhs, kRankRhs, true>& rhs) {
+  if constexpr (std::is_same_v<RawTypeLhs, RawTypeRhs>) {
+    return (lhs.raw_ == rhs.raw_);
   }
+  return false;
+}
 
-  template <typename RawTypeRhs, std::size_t kRank>
-  constexpr bool operator!=(const Array<RawTypeRhs, kRank>& rhs) const {
-    return !(*this == rhs);
-  }
-};
+template <typename RawTypeLhs, std::size_t kRankLhs, typename RawTypeRhs,
+          std::size_t kRankRhs>
+constexpr bool operator!=(const ArrayImpl<RawTypeLhs, kRankLhs, true>& lhs,
+                          const Array<RawTypeRhs, kRankRhs>& rhs) {
+  return !(lhs == rhs);
+}
 
 // This type correlates to those used in declarations,
 //   e.g. Field { Array { Array { jint {} } } }.
diff --git a/implementation/array_test.cc b/implementation/array_test.cc
index 14833456..cc9858a7 100644
--- a/implementation/array_test.cc
+++ b/implementation/array_test.cc
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <type_traits>
+
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include "jni_bind.h"
@@ -108,5 +110,4 @@ static_assert(
 static_assert(FullArrayStripV(arr8) == kClass);
 static_assert(FullArrayStripV(arr9) == kClass);
 
-
 }  // namespace
diff --git a/implementation/array_view.h b/implementation/array_view.h
index ea56dc8d..4e7a0df0 100644
--- a/implementation/array_view.h
+++ b/implementation/array_view.h
@@ -21,16 +21,21 @@
 
 #include <cstddef>
 #include <iterator>
+#include <type_traits>
 
 #include "implementation/array_type_conversion.h"
+#include "implementation/jni_helper/get_array_element_result.h"
 #include "implementation/jni_helper/jni_array_helper.h"
 #include "implementation/jni_helper/lifecycle.h"
 #include "jni_dep.h"
 
 namespace jni {
 
+struct ArrayViewHelperBase {};
+
 template <typename T>
-struct ArrayViewHelper {
+struct ArrayViewHelper : ArrayViewHelperBase {
+  using SpanType = T;
   const T val_;
   operator T() const { return val_; }
 
@@ -38,9 +43,11 @@ struct ArrayViewHelper {
 };
 
 // Primitive Rank 1 Arrays.
-template <typename SpanType, std::size_t kRank = 1, typename Enable = void>
+template <typename SpanType_, std::size_t kRank = 1, typename Enable = void>
 class ArrayView {
  public:
+  using SpanType = SpanType_;
+
   struct Iterator {
     using iterator_category = std::random_access_iterator_tag;
     using difference_type = std::size_t;
@@ -115,12 +122,14 @@ class ArrayView {
 };
 
 // Object arrays, or arrays with rank > 1 (which are object arrays), or strings.
-template <typename SpanType, std::size_t kRank>
+template <typename SpanType_, std::size_t kRank>
 class ArrayView<
-    SpanType, kRank,
-    std::enable_if_t<(kRank > 1) || std::is_same_v<SpanType, jobject> ||
-                     std::is_same_v<SpanType, jstring>>> {
+    SpanType_, kRank,
+    std::enable_if_t<(kRank > 1) || std::is_same_v<SpanType_, jobject> ||
+                     std::is_same_v<SpanType_, jstring>>> {
  public:
+  using SpanType = SpanType_;
+
   // Metafunction that returns the type after a single dereference.
   template <std::size_t>
   struct PinHelper {
diff --git a/implementation/class.h b/implementation/class.h
index e72e8bbf..81dee205 100644
--- a/implementation/class.h
+++ b/implementation/class.h
@@ -156,27 +156,45 @@ struct Class<Extends_, std::tuple<Constructors_...>,
         static_(statik),
         methods_(methods...),
         fields_(fields...) {}
-
-  ////////////////////////////////////////////////////////////////////////////////
-  // Equality operators.
-  ////////////////////////////////////////////////////////////////////////////////
-  template <typename ParentClass, typename... Params, typename... Constructors,
-            typename... StaticMethods, typename... StaticFields,
-            typename... Fields, typename... Methods>
-  constexpr bool operator==(
-      const Class<ParentClass, std::tuple<Constructors...>,
-                  std::tuple<Static<std::tuple<StaticMethods...>,
-                                    std::tuple<StaticFields...>>>,
-                  std::tuple<Methods...>, std::tuple<Fields...>>& rhs) const {
-    // Don't compare the other parameters so classes can be used as parameters
-    // or return values before the class itself is defined.
-    return std::string_view(name_) == std::string_view(rhs.name_);
-  }
-
-  constexpr bool operator==(const NoClass&) const { return false; }
-  constexpr bool operator!=(const NoClass&) const { return false; }
 };
 
+////////////////////////////////////////////////////////////////////////////////
+// Equality operators.
+////////////////////////////////////////////////////////////////////////////////
+template <typename LhsParentClass, typename... LhsParams,
+          typename... LhsConstructors, typename... LhsStaticMethods,
+          typename... LhsStaticFields, typename... LhsFields,
+          typename... LhsMethods, typename RhsParentClass,
+          typename... RhsParams, typename... RhsConstructors,
+          typename... RhsStaticMethods, typename... RhsStaticFields,
+          typename... RhsFields, typename... RhsMethods>
+constexpr bool operator==(
+    const Class<LhsParentClass, std::tuple<LhsConstructors...>,
+                std::tuple<Static<std::tuple<LhsStaticMethods...>,
+                                  std::tuple<LhsStaticFields...>>>,
+                std::tuple<LhsMethods...>, std::tuple<LhsFields...>>& lhs,
+    const Class<RhsParentClass, std::tuple<RhsConstructors...>,
+                std::tuple<Static<std::tuple<RhsStaticMethods...>,
+                                  std::tuple<RhsStaticFields...>>>,
+                std::tuple<RhsMethods...>, std::tuple<RhsFields...>>& rhs) {
+  // Don't compare the other parameters so classes can be used as parameters
+  // or return values before the class itself is defined.
+  return std::string_view(lhs.name_) == std::string_view(rhs.name_);
+}
+
+template <typename... Ts>
+constexpr bool operator==(const Class<Ts...>& lhs, const NoClass&) {
+  return false;
+}
+
+template <typename... Ts>
+constexpr bool operator!=(const Class<Ts...>& lhs, const NoClass&) {
+  return true;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CTAD.
+////////////////////////////////////////////////////////////////////////////////
 template <typename... Params>
 Class(const char*, Params...)
     -> Class<metaprogramming::BaseFilterWithDefault_t<
diff --git a/implementation/class_loader.h b/implementation/class_loader.h
index efd94fb1..c2d49ce1 100644
--- a/implementation/class_loader.h
+++ b/implementation/class_loader.h
@@ -19,6 +19,7 @@
 
 // IWYU pragma: private, include "third_party/jni_wrapper/jni_bind.h"
 
+#include <cstddef>
 #include <tuple>
 #include <type_traits>
 #include <utility>
@@ -68,20 +69,6 @@ class ClassLoader : public Object {
         parent_loader_(parent_loader),
         supported_classes_(supported_class_set.supported_classes_) {}
 
-  bool constexpr operator==(
-      const ClassLoader<ParentLoader_, SupportedClasses_...>& rhs) const {
-    return (*this).parent_loader_ == rhs.parent_loader_ &&
-           (*this).supported_classes_ == rhs.supported_classes_;
-  }
-  template <typename T>
-  bool constexpr operator==(const T& rhs) const {
-    return false;
-  }
-  template <typename T>
-  bool constexpr operator!=(const T& rhs) const {
-    return !(*this == rhs);
-  }
-
   template <const auto& class_v, std::size_t... Is>
   constexpr std::size_t IdxOfClassHelper(
       std::integer_sequence<std::size_t, Is...>) const {
@@ -131,6 +118,33 @@ class ClassLoader : public Object {
   }
 };
 
+////////////////////////////////////////////////////////////////////////////////
+// Equality operators.
+////////////////////////////////////////////////////////////////////////////////
+template <typename ParentLoader_, typename... SupportedClasses_>
+bool constexpr operator==(
+    const ClassLoader<ParentLoader_, SupportedClasses_...>& lhs,
+    const ClassLoader<ParentLoader_, SupportedClasses_...>& rhs) {
+  return lhs.parent_loader_ == rhs.parent_loader_ &&
+         lhs.supported_classes_ == rhs.supported_classes_;
+}
+
+template <typename ParentLoader_, typename... SupportedClasses_, typename T>
+bool constexpr operator==(
+    const ClassLoader<ParentLoader_, SupportedClasses_...>& lhs, const T& rhs) {
+  return false;
+}
+
+template <typename ParentLoader_, typename... SupportedClasses_, typename T>
+bool constexpr operator!=(
+    const ClassLoader<ParentLoader_, SupportedClasses_...>& lhs, const T& rhs) {
+  return !(lhs == rhs);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// CTAD.
+////////////////////////////////////////////////////////////////////////////////
+
 // Note: Null is chosen, not default, because LoadedBy requires a syntax like
 // LoadedBy{ClassLoader{"kClass"}} (using the CTAD loader type below), but
 // we want to prevent explicit usage of a default loader (as it makes no sense).
@@ -146,6 +160,9 @@ template <typename... SupportedClasses_>
 ClassLoader(SupportedClassSet<SupportedClasses_...>)
     -> ClassLoader<DefaultClassLoader, SupportedClasses_...>;
 
+////////////////////////////////////////////////////////////////////////////////
+// Ancestral lookups.
+////////////////////////////////////////////////////////////////////////////////
 template <typename T, std::size_t I>
 constexpr auto& GetAncestor(const T& loader) {
   if constexpr (I == 0) {
diff --git a/implementation/constructor.h b/implementation/constructor.h
index d3f8546e..aa1c6b57 100644
--- a/implementation/constructor.h
+++ b/implementation/constructor.h
@@ -19,10 +19,12 @@
 
 // IWYU pragma: private, include "third_party/jni_wrapper/jni_bind.h"
 
-#include "method.h"
-#include "params.h"
-#include "return.h"
+#include <cstddef>
+#include <string>
+#include <type_traits>
+
 #include "implementation/jni_helper/jni_helper.h"
+#include "implementation/params.h"
 
 namespace jni {
 
@@ -52,15 +54,15 @@ class Constructor : public ConstructorBase {
   constexpr explicit Constructor(Args...) {}
 };
 
-template <typename... ParamsRaw>
-Constructor(ParamsRaw...) -> Constructor<ParamsRaw...>;
-
 template <typename... LhsParams, typename... RhsParams>
 constexpr bool operator==(const Constructor<LhsParams...>& lhs,
                           const Constructor<RhsParams...>& rhs) {
   return lhs.params_ == rhs.params_;
 }
 
+template <typename... ParamsRaw>
+Constructor(ParamsRaw...) -> Constructor<ParamsRaw...>;
+
 //==============================================================================
 // Represents a constructor used at runtime and has index data about where it
 // exists in the static class definition which is embedded on the caller's
diff --git a/implementation/default_class_loader.h b/implementation/default_class_loader.h
index 1e4f8944..b939be59 100644
--- a/implementation/default_class_loader.h
+++ b/implementation/default_class_loader.h
@@ -62,18 +62,21 @@ class DefaultClassLoader {
   constexpr std::size_t IdxOfAncestor(std::size_t cur_idx = 0) const {
     return kClassNotInLoaderSetIdx;
   }
+};
 
-  template <typename T>
-  bool constexpr operator==(const T& rhs) const {
-    return false;
-  }
-  bool constexpr operator==(const DefaultClassLoader&) const { return true; }
+template <typename T>
+bool constexpr operator==(const DefaultClassLoader& lhs, const T& rhs) {
+  return false;
+}
+bool constexpr operator==(const DefaultClassLoader& lhs,
+                          const DefaultClassLoader& rhs) {
+  return true;
+}
 
-  template <typename T>
-  bool constexpr operator!=(const T& rhs) const {
-    return !(*this == rhs);
-  }
-};
+template <typename T>
+bool constexpr operator!=(const DefaultClassLoader& lhs, const T& rhs) {
+  return !(lhs == rhs);
+}
 
 // Class loader that cannot supply any classes. This should be the root loader
 // for most user defined classes.
@@ -95,19 +98,22 @@ class NullClassLoader {
   constexpr std::size_t IdxOfAncestor(std::size_t cur_idx = 0) const {
     return kClassNotInLoaderSetIdx;
   }
-
-  template <typename T>
-  bool constexpr operator==(const T& rhs) const {
-    return false;
-  }
-  bool constexpr operator==(const NullClassLoader&) const { return true; }
-
-  template <typename T>
-  bool constexpr operator!=(const T& rhs) const {
-    return !(*this == rhs);
-  }
 };
 
+template <typename T>
+constexpr bool operator==(const NullClassLoader& lhs, const T& rhs) {
+  return false;
+}
+constexpr bool operator==(const NullClassLoader& lhs,
+                          const NullClassLoader& rhs) {
+  return true;
+}
+
+template <typename T>
+constexpr bool operator!=(const NullClassLoader& lhs, const T& rhs) {
+  return !(lhs == rhs);
+}
+
 static constexpr NullClassLoader kNullClassLoader;
 static constexpr DefaultClassLoader kDefaultClassLoader;
 
diff --git a/implementation/field_ref.h b/implementation/field_ref.h
index a364582b..e000dbf0 100644
--- a/implementation/field_ref.h
+++ b/implementation/field_ref.h
@@ -19,10 +19,12 @@
 
 // IWYU pragma: private, include "third_party/jni_wrapper/jni_bind.h"
 
+#include <cstddef>
 #include <type_traits>
 #include <utility>
 #include <vector>
 
+#include "implementation/default_class_loader.h"
 #include "implementation/field_selection.h"
 #include "implementation/id.h"
 #include "implementation/id_type.h"
diff --git a/implementation/forward_declarations.h b/implementation/forward_declarations.h
index f3c475df..6aa11935 100644
--- a/implementation/forward_declarations.h
+++ b/implementation/forward_declarations.h
@@ -19,10 +19,10 @@
 
 // IWYU pragma: private, include "third_party/jni_wrapper/jni_bind.h"
 
-#include "implementation/default_class_loader.h"
+#include <cstddef>
+
 #include "implementation/id_type.h"
-#include "implementation/jni_helper/lifecycle_object.h"
-#include "implementation/jvm.h"
+#include "implementation/jni_helper/lifecycle.h"
 
 namespace jni {
 
@@ -37,6 +37,9 @@ namespace jni {
 // Provide this base tag to UserDefined to enable custom types.
 struct JniUserDefinedCorpusTag {};
 
+// ArrayViewHelperBase (shared by all `ArrayView`).
+struct ArrayViewHelperBase;
+
 // ArrayView Helper.
 template <typename T>
 struct ArrayViewHelper;
diff --git a/implementation/global_class_loader.h b/implementation/global_class_loader.h
index 7b221671..cd08d62c 100644
--- a/implementation/global_class_loader.h
+++ b/implementation/global_class_loader.h
@@ -37,6 +37,7 @@ class GlobalClassLoader
  public:
   using Base = ClassLoaderRef<LifecycleType::GLOBAL, class_loader_v_, jvm_v_>;
   using Base::Base;
+  using SpanType = jobject;
 
   template <const auto& class_loader_v, const auto& jvm_v>
   GlobalClassLoader(GlobalClassLoader<class_loader_v, jvm_v>&& rhs)
diff --git a/implementation/global_object.h b/implementation/global_object.h
index 8cefa543..88056ad9 100644
--- a/implementation/global_object.h
+++ b/implementation/global_object.h
@@ -19,10 +19,12 @@
 
 // IWYU pragma: private, include "third_party/jni_wrapper/jni_bind.h"
 
+#include "implementation/default_class_loader.h"
 #include "implementation/forward_declarations.h"
 #include "implementation/jni_helper/lifecycle.h"
 #include "implementation/jni_helper/lifecycle_object.h"
 #include "implementation/jni_type.h"
+#include "implementation/jvm.h"
 #include "implementation/local_object.h"
 #include "implementation/promotion_mechanics.h"
 #include "implementation/ref_base.h"
@@ -44,6 +46,7 @@ class GlobalObject
  public:
   using Base = GlobalObjectImpl<class_v_, class_loader_v_, jvm_v_>;
   using Base::Base;
+  using SpanType = jobject;
   using LifecycleT = LifecycleHelper<jobject, LifecycleType::GLOBAL>;
 
   template <const auto& class_v, const auto& class_loader_v, const auto& jvm_v>
diff --git a/implementation/global_object_test.cc b/implementation/global_object_test.cc
index 69387eda..7e19f79d 100644
--- a/implementation/global_object_test.cc
+++ b/implementation/global_object_test.cc
@@ -79,6 +79,25 @@ TEST_F(JniTest, GlobalObject_ConstructsFromNonStandardConstructor) {
   EXPECT_EQ(jobject{global_object}, AsGlobal(Fake<jobject>()));
 }
 
+TEST_F(JniTest, GlobalObject_ComparesAgainstOtherGlobalObjects) {
+  static constexpr Class kClass{
+      "kClass",
+      Constructor{jfloat{}, jfloat{}},
+  };
+  static constexpr Class kClass2{
+      "kClass2",
+  };
+  GlobalObject<kClass> val_1{AdoptGlobal{}, Fake<jobject>(1)};
+  GlobalObject<kClass2> val_2{AdoptGlobal{}, Fake<jobject>(2)};
+
+  EXPECT_TRUE(val_1 == val_1);
+  EXPECT_FALSE(val_1 == val_2);
+  EXPECT_TRUE(val_1 != val_2);
+  EXPECT_TRUE(val_2 == val_2);
+  EXPECT_TRUE(val_2 != val_1);
+  EXPECT_FALSE(val_1 == val_2);
+}
+
 TEST_F(JniTest, GlobalObject_DoesNotDeleteAnyLocalsForAdoptedGlobalJobject) {
   static constexpr Class kClass{"kClass"};
 
diff --git a/implementation/jvm.h b/implementation/jvm.h
index a98c4304..52e9c2fb 100644
--- a/implementation/jvm.h
+++ b/implementation/jvm.h
@@ -50,18 +50,23 @@ class Jvm {
     return IdxOfClassLoaderHelper<class_loader_v>(
         std::make_integer_sequence<std::size_t, sizeof...(ClassLoaderTs)>());
   }
+};
 
-  template <typename T>
-  bool constexpr operator==(const T& rhs) const {
-    return false;
-  }
-  bool constexpr operator==(const Jvm&) const { return true; }
+template <typename... ClassLoaderTs, typename T>
+constexpr bool operator==(const Jvm<ClassLoaderTs...>& lhs, const T& rhs) {
+  return false;
+}
 
-  template <typename T>
-  bool constexpr operator!=(const T& rhs) const {
-    return !(*this == rhs);
-  }
-};
+template <typename... ClassLoaderLhsTs>
+constexpr bool operator==(const Jvm<ClassLoaderLhsTs...>& lhs,
+                          const Jvm<ClassLoaderLhsTs...>& rhs) {
+  return true;
+}
+
+template <typename... ClassLoaderTs, typename T>
+bool constexpr operator!=(const Jvm<ClassLoaderTs...>& lhs, const T& rhs) {
+  return !(lhs == rhs);
+}
 
 template <typename... ClassLoaderTs>
 Jvm(ClassLoaderTs...) -> Jvm<ClassLoaderTs...>;
diff --git a/implementation/local_array.h b/implementation/local_array.h
index 40745b39..5a291bfa 100644
--- a/implementation/local_array.h
+++ b/implementation/local_array.h
@@ -41,16 +41,17 @@ namespace jni {
 // Currently GlobalArrays do not exist, as reasoning about the lifecycles of the
 // underlying objects is non-trivial, e.g. a GlobalArray taking a local object
 // would result in a possibly unexpected extension of lifetime.
-template <typename SpanType, std::size_t kRank_ = 1,
+template <typename SpanType_, std::size_t kRank_ = 1,
           const auto& class_v_ = kNoClassSpecified,
           const auto& class_loader_v_ = kDefaultClassLoader,
           const auto& jvm_v_ = kDefaultJvm>
 class LocalArray
     : public ArrayRef<
-          JniT<SpanType, class_v_, class_loader_v_, jvm_v_, kRank_>> {
+          JniT<SpanType_, class_v_, class_loader_v_, jvm_v_, kRank_>> {
  public:
   static constexpr Class kClass{class_v_.name_};
   static constexpr std::size_t kRank = kRank_;
+  using SpanType = SpanType_;
 
   using RawValT = std::conditional_t<std::is_same_v<jobject, SpanType>,
                                      std::decay_t<decltype(kClass)>, SpanType>;
@@ -80,11 +81,12 @@ class LocalArray
       : Base(AdoptLocal{}, rhs.Release()) {}
 
   // Rvalue ctor.
-  template <typename SpanType_, std::size_t kRank, const auto& class_v,
+  template <typename SpanTypeRhs_, std::size_t kRank, const auto& class_v,
             const auto& class_loader_v, const auto& jvm_v>
-  LocalArray(LocalArray<SpanType_, kRank, class_v, class_loader_v, jvm_v>&& rhs)
+  LocalArray(
+      LocalArray<SpanTypeRhs_, kRank, class_v, class_loader_v, jvm_v>&& rhs)
       : Base(AdoptLocal{}, rhs.Release()) {
-    static_assert(std::is_same_v<SpanType, SpanType_> && kRank == kRank_ &&
+    static_assert(std::is_same_v<SpanType, SpanTypeRhs_> && kRank == kRank_ &&
                   class_v == class_v_ && class_loader_v == class_loader_v_);
   }
 
@@ -93,9 +95,9 @@ class LocalArray
   // e.g.
   //  LocalArray arr { 5, LocalObject<kClass> {args...} };
   //  LocalArray arr { 5, GlobalObject<kClass> {args...} };
-  template <template <const auto&, const auto&, const auto&>
-            class ObjectContainer,
-            const auto& class_v, const auto& class_loader_v, const auto& jvm_v>
+  template <
+      template <const auto&, const auto&, const auto&> class ObjectContainer,
+      const auto& class_v, const auto& class_loader_v, const auto& jvm_v>
   LocalArray(
       std::size_t size,
       const ObjectContainer<class_v, class_loader_v, jvm_v>& local_object)
@@ -108,9 +110,9 @@ class LocalArray
   operator jobject() { return static_cast<jobject>(Base::object_ref_); }
 };
 
-template <template <const auto&, const auto&, const auto&>
-          class ObjectContainer,
-          const auto& class_v_, const auto& class_loader_v_, const auto& jvm_v_>
+template <
+    template <const auto&, const auto&, const auto&> class ObjectContainer,
+    const auto& class_v_, const auto& class_loader_v_, const auto& jvm_v_>
 LocalArray(std::size_t,
            const ObjectContainer<class_v_, class_loader_v_, jvm_v_>&)
     -> LocalArray<jobject, class_v_, class_loader_v_, jvm_v_>;
@@ -123,9 +125,9 @@ LocalArray(
     -> LocalArray<jobject, class_v_, class_loader_v_, jvm_v_>;
 
 template <typename SpanType>
-LocalArray(std::size_t,
-           SpanType) -> LocalArray<SpanType, 1, kNoClassSpecified,
-                                   kDefaultClassLoader, kDefaultJvm>;
+LocalArray(std::size_t, SpanType)
+    -> LocalArray<SpanType, 1, kNoClassSpecified, kDefaultClassLoader,
+                  kDefaultJvm>;
 
 template <typename SpanType, std::size_t kRank_minus_1>
 LocalArray(std::size_t, LocalArray<SpanType, kRank_minus_1>)
diff --git a/implementation/local_class_loader.h b/implementation/local_class_loader.h
index 8e9e9b12..7989d6ff 100644
--- a/implementation/local_class_loader.h
+++ b/implementation/local_class_loader.h
@@ -25,6 +25,7 @@
 #include "implementation/jni_helper/lifecycle.h"
 #include "implementation/jvm.h"
 #include "implementation/promotion_mechanics_tags.h"
+#include "jni_dep.h"
 
 namespace jni {
 
@@ -35,6 +36,7 @@ class LocalClassLoader
  public:
   using Base = ClassLoaderRef<LifecycleType::LOCAL, class_loader_v_, jvm_v_>;
   using Base::Base;
+  using SpanType = jobject;
 
   template <const auto& class_loader_v, const auto& jvm_v>
   explicit LocalClassLoader(LocalClassLoader<class_loader_v, jvm_v>&& lhs)
diff --git a/implementation/local_object.h b/implementation/local_object.h
index 1dc59450..5dbeb2fa 100644
--- a/implementation/local_object.h
+++ b/implementation/local_object.h
@@ -42,6 +42,7 @@ class LocalObject : public LocalObjectImpl<class_v_, class_loader_v_, jvm_v_> {
  public:
   using Base = LocalObjectImpl<class_v_, class_loader_v_, jvm_v_>;
   using Base::Base;
+  using SpanType = jobject;
 
   template <typename T>
   LocalObject(ArrayViewHelper<T> array_view_helper)
@@ -56,18 +57,6 @@ template <const auto& class_v_, const auto& class_loader_v_, const auto& jvm_v_>
 LocalObject(LocalObject<class_v_, class_loader_v_, jvm_v_>&&)
     -> LocalObject<class_v_, class_loader_v_, jvm_v_>;
 
-template <const auto& class_v_, const auto& class_loader_v_, const auto& jvm_v_>
-bool operator==(const jobject& lhs,
-                const LocalObject<class_v_, class_loader_v_, jvm_v_>& rhs) {
-  return lhs == static_cast<jobject>(rhs);
-}
-
-template <const auto& class_v_, const auto& class_loader_v_, const auto& jvm_v_>
-bool operator!=(const jobject& lhs,
-                const LocalObject<class_v_, class_loader_v_, jvm_v_>& rhs) {
-  return !(lhs == rhs);
-}
-
 }  // namespace jni
 
 #endif  // JNI_BIND_LOCAL_OBJECT_H
diff --git a/implementation/local_object_test.cc b/implementation/local_object_test.cc
index dc0562ad..840658b8 100644
--- a/implementation/local_object_test.cc
+++ b/implementation/local_object_test.cc
@@ -209,25 +209,25 @@ TEST_F(JniTest, LocalObject_ComparesAgainstjobjects) {
   EXPECT_FALSE(AsNewLocalReference(Fake<jobject>()) != val_1);
 }
 
-TEST_F(JniTest, LocalObject_ComparesAgainstOtherLocalObjects_InContainers) {
-  struct A {
-    LocalObject<kClass> val_1;
-    LocalObject<kClass2> val_2;
+struct A {
+  LocalObject<kClass> val_1;
+  LocalObject<kClass2> val_2;
 
-    A(LocalObject<kClass>&& val_1, LocalObject<kClass2>&& val_2)
-        : val_1(std::move(val_1)), val_2(std::move(val_2)) {}
+  A(LocalObject<kClass>&& val_1, LocalObject<kClass2>&& val_2)
+      : val_1(std::move(val_1)), val_2(std::move(val_2)) {}
 
-    A(A&& rhs) : val_1(std::move(rhs.val_1)), val_2(std::move(rhs.val_2)) {}
+  A(A&& rhs) : val_1(std::move(rhs.val_1)), val_2(std::move(rhs.val_2)) {}
+};
 
-    bool operator==(const A& rhs) const {
-      return val_1 == rhs.val_1 && val_2 == rhs.val_2;
-    }
+constexpr bool operator==(const A& lhs, const A& rhs) {
+  return lhs.val_1 == rhs.val_1 && lhs.val_2 == rhs.val_2;
+}
 
-    bool operator!=(const A& rhs) const {
-      return val_1 != rhs.val_1 || val_2 != rhs.val_2;
-    }
-  };
+constexpr bool operator!=(const A& lhs, const A& rhs) {
+  return lhs.val_1 != rhs.val_1 || lhs.val_2 != rhs.val_2;
+}
 
+TEST_F(JniTest, LocalObject_ComparesAgainstOtherLocalObjects_InContainers) {
   A val_1{LocalObject<kClass>{Fake<jobject>(1)}, {Fake<jobject>(2)}};
   A val_2{LocalObject<kClass2>{Fake<jobject>(1)}, {Fake<jobject>(3)}};
 
diff --git a/implementation/no_class_specified.h b/implementation/no_class_specified.h
index 96fce2cd..ae90446f 100644
--- a/implementation/no_class_specified.h
+++ b/implementation/no_class_specified.h
@@ -34,6 +34,8 @@ struct RootObject {
 static constexpr RootObject kObject{};
 
 static constexpr struct NoClass {
+  // For compatability reasons, this must be defined (not default) because some
+  // compilers will complain about defaulted constructors being deleted.
   constexpr NoClass() {}
 
   const char* name_ = "__JNI_BIND__NO_CLASS__";
@@ -41,11 +43,15 @@ static constexpr struct NoClass {
   const Static<std::tuple<>, std::tuple<>> static_{};
   const std::tuple<> methods_{};
   const std::tuple<> fields_{};
-
-  constexpr bool operator==(const NoClass&) const { return true; }
-  constexpr bool operator!=(const NoClass&) const { return true; }
 } kNoClassSpecified;
 
+constexpr bool operator==(const NoClass& lhs, const NoClass& rhs) {
+  return true;
+}
+constexpr bool operator!=(const NoClass& lhs, const NoClass& rhs) {
+  return false;
+}
+
 }  // namespace jni
 
 #endif  // JNI_BIND_IMPLEMENTATION_NO_CLASS_SPECIFIED_H_
diff --git a/implementation/promotion_mechanics.h b/implementation/promotion_mechanics.h
index 04bb7558..d457eeee 100644
--- a/implementation/promotion_mechanics.h
+++ b/implementation/promotion_mechanics.h
@@ -56,28 +56,48 @@ struct EntryBase : public Base {
                             LifecycleHelper<Span, lifecycleType>::NewReference(
                                 static_cast<Span>(object)))
                       : nullptr) {}
+};
 
-  // Comparison operator for pinned Scoped type (not deep equality).
-  template <typename T,
-            typename = std::enable_if_t<(std::is_base_of_v<RefBase<Span>, T> ||
-                                         std::is_same_v<T, ViableSpan>)>>
-  bool operator==(const T& rhs) const {
-    if constexpr (std::is_base_of_v<RefBase<Span>, T>) {
-      return static_cast<Span>(rhs.object_ref_) == Base::object_ref_;
-    } else if constexpr (std::is_same_v<T, ViableSpan>) {
-      return rhs == Base::object_ref_;
-    }
-  }
+////////////////////////////////////////////////////////////////////////////////
+// Equality operators.
+// Note: Post C++20 these [in]equality operators are required to be reversable.
+////////////////////////////////////////////////////////////////////////////////
+template <typename T, typename Enable = void>
+struct IsComparableHelper {
+  using type = T;
+};
 
-  // Comparison inequality operator for pinned Scoped type (not deep equality).
-  template <typename T,
-            typename = std::enable_if_t<(std::is_base_of_v<RefBase<Span>, T> ||
-                                         std::is_same_v<T, ViableSpan>)>>
-  bool operator!=(const T& rhs) const {
-    return !(*this == rhs);
-  }
+template <typename T>
+struct IsComparableHelper<T, std::void_t<typename T::SpanType>> {
+  using type = typename T::SpanType;
 };
 
+template <typename T>
+using IsComparableHelper_t = typename IsComparableHelper<T>::type;
+
+// Returns if the two types are "JNI Bind" comparable. e.g.
+// jobject == LocalObject or LocalObject == jobject.
+template <typename T1, typename T2>
+constexpr bool IsJniBindComparable() {
+  return std::is_same_v<IsComparableHelper_t<T1>, IsComparableHelper_t<T2>> &&
+         (std::is_base_of_v<RefBaseBase, T1> ||
+          std::is_base_of_v<RefBaseBase, T2>);
+}
+
+template <typename T1, typename T2>
+constexpr std::enable_if_t<IsJniBindComparable<T1, T2>(), bool> operator==(
+    const T1& lhs, const T2& rhs) {
+  return static_cast<IsComparableHelper_t<T1>>(lhs) ==
+         static_cast<IsComparableHelper_t<T2>>(rhs);
+}
+
+template <typename T1, typename T2>
+constexpr std::enable_if_t<IsJniBindComparable<T1, T2>(), bool> operator!=(
+    const T1& lhs, const T2& rhs) {
+  return static_cast<IsComparableHelper_t<T1>>(lhs) !=
+         static_cast<IsComparableHelper_t<T2>>(rhs);
+}
+
 // Local scoped entry augmentation.
 template <LifecycleType lifecycleType, typename JniT, typename ViableSpan,
           typename... ViableSpans>
diff --git a/implementation/ref_base.h b/implementation/ref_base.h
index 6bbcc630..277ae664 100644
--- a/implementation/ref_base.h
+++ b/implementation/ref_base.h
@@ -50,13 +50,13 @@ class RefBase : public RefBaseBase {
             typename = std::enable_if_t<std::is_same_v<T, StorageType>>>
   RefBase(RefBase<T>&& rhs) : object_ref_(rhs.Release()) {}
 
+  // Releases ownership of the underlying object, further use is undefined.
   StorageType Release() {
     StorageType return_value = object_ref_;
     object_ref_ = nullptr;
 
     return return_value;
   }
-
   explicit operator StorageType() const { return object_ref_; }
 
  protected: