From 1cc699dddaac295930b34e1a17a9a88218e751ba Mon Sep 17 00:00:00 2001
From: David Blaikie <dblaikie@gmail.com>
Date: Fri, 17 Apr 2026 18:32:39 -0700
Subject: [PATCH] Make heterogenous hash table lookup opt-in (#6950)

This still only works if the hash of the distinct types are identical
(so it still doesn't address the derived pointer v base pointer case -
well, not in the way we would want to address it, we could use this
change to make derived pointer and base pointer not compare equal, but
that's not very ergonomic)

I think in a follow up maybe I can use a `TranslatingKeyContext` to
translate `Derived*` to `Base*` in general.

No test coverage for this change, since it's a no-compile situation and
we don't seem to generally do no-compile tests.

Discovered while working on #6940

---------

Co-authored-by: Geoff Romer <gromer@google.com>
---
 common/hashtable_key_context.h      | 44 ++++++++++++++++++++++++-----
 common/raw_hashtable_test_helpers.h |  8 ++++++
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/common/hashtable_key_context.h b/common/hashtable_key_context.h
index 32c57fcd4301..e5115a2629a1 100644
--- a/common/hashtable_key_context.h
+++ b/common/hashtable_key_context.h
@@ -18,17 +18,18 @@ namespace Carbon {
 //
 // This provides a hashtable-specific extension point to implement equality
 // comparison within a hashtable key context. By default, it will use
-// `operator==` on the LHS and RHS operands. However, types can provide a
-// dedicated customization point by implementing a free function that can be
-// found by ADL for your type called `CarbonHashtableEq` with the following
-// signature:
+// `operator==` on the LHS and RHS operands if they are of the identical type.
+// However, types can provide a dedicated customization point by implementing a
+// free function that can be found by ADL for your type called
+// `CarbonHashtableEq` with the following signature:
 //
 // ```cpp
 // auto CarbonHashtableEq(const YourType& lhs, const YourType& rhs) -> bool;
 // ```
 //
 // Any such overload will be able to override the default we provide for types
-// that can compare with `==`.
+// that can compare with `==`. This overload may only compare two objects equal
+// if the hash of those two objects are identical.
 //
 // This library also provides any customization points for LLVM or standard
 // library types either lacking `operator==` or where that operator is not
@@ -161,8 +162,37 @@ inline auto CarbonHashtableEq(const llvm::APFloat& lhs,
   return lhs.bitwiseIsEqual(rhs);
 }
 
-template <typename LeftT, typename RightT>
-inline auto CarbonHashtableEq(const LeftT& lhs, const RightT& rhs) -> bool
+inline auto CarbonHashtableEq(llvm::StringRef lhs, const std::string& rhs)
+    -> bool {
+  return lhs == rhs;
+}
+
+template <typename T>
+inline auto CarbonHashtableEq(llvm::MutableArrayRef<T> lhs,
+                              llvm::ArrayRef<T> rhs) -> bool {
+  return lhs == rhs;
+}
+
+template <typename LHS, typename RHS>
+inline auto CarbonHashtableEq(const LHS& lhs, const RHS& rhs) -> bool
+  requires(requires {
+    { CarbonHashtableEq(rhs, lhs) } -> std::convertible_to<bool>;
+  })
+{
+  return CarbonHashtableEq(rhs, lhs);
+}
+
+// Provides symmetric equality so the `CarbonHashtableEq` operands aren't
+// ordered.
+//
+// If this template proves problematic in any way, we can revisit it - the
+// `CarbonHashtableEq` functions don't really need to be symmetric, since they
+// generally represent an implicit conversion which is often only one-way (eg:
+// MutableArrayRef converts to ArrayRef, but not the other way around) - but
+// documenting/describing that asymmetry felt a little awkward too - so maybe
+// this template is an OK solution for now.
+template <typename T>
+inline auto CarbonHashtableEq(const T& lhs, const T& rhs) -> bool
   requires(requires {
     { lhs == rhs } -> std::convertible_to<bool>;
   })
diff --git a/common/raw_hashtable_test_helpers.h b/common/raw_hashtable_test_helpers.h
index ee0ad5d479a9..874f9321191c 100644
--- a/common/raw_hashtable_test_helpers.h
+++ b/common/raw_hashtable_test_helpers.h
@@ -45,6 +45,10 @@ struct TestData : Printable<TestData> {
 
 static_assert(std::is_copy_constructible_v<TestData>);
 
+inline auto CarbonHashtableEq(int lhs, TestData rhs) -> bool {
+  return lhs == rhs;
+}
+
 // Non-trivial type for testing.
 struct MoveOnlyTestData : Printable<TestData> {
   int value;
@@ -85,6 +89,10 @@ struct MoveOnlyTestData : Printable<TestData> {
 static_assert(!std::is_copy_constructible_v<MoveOnlyTestData>);
 static_assert(std::is_move_constructible_v<MoveOnlyTestData>);
 
+inline auto CarbonHashtableEq(int lhs, const MoveOnlyTestData& rhs) -> bool {
+  return lhs == rhs;
+}
+
 // Test stateless key context that produces different hashes from normal.
 // Changing the hash values should result in test failures if the context ever
 // fails to be used.