From 27275e6729db7531536c8b6ee7e5eaa570b1bcce Mon Sep 17 00:00:00 2001
From: Jon Ross-Perkins <jperkins@google.com>
Date: Thu, 5 Dec 2024 11:37:32 -0800
Subject: [PATCH] Change the IdBase operator== to fix reversed operator
 warnings (#4636)

I believe our flags enable the warning by default, it's just that it
doesn't catch this in clang-16 (maybe more; I reproduced with clang-18
and didn't keep digging). For example:

```
toolchain/parse/tree_test.cpp:86:28: error: ISO C++20 considers use of overloaded operator '==' (with operand types 'value_type' (aka 'Carbon::Parse::NodeIdInCategory<Carbon::Parse::NodeCategory::Decl>') and 'AnyDeclId' (aka 'NodeIdInCategory<NodeCategory::Decl>')) to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator]
   86 |   EXPECT_TRUE(*any_decl_id == any_decl_id2);
      |               ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~
```

The different `operator==` approach works except for with
`Parse::NodeId::Invalid`, which seems easy to replace with a
`.is_valid()` check.
---
 toolchain/base/index_base.h         | 14 ++++----------
 toolchain/check/handle_modifier.cpp |  2 +-
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/toolchain/base/index_base.h b/toolchain/base/index_base.h
index 1c13237ae613..ef81877c2f70 100644
--- a/toolchain/base/index_base.h
+++ b/toolchain/base/index_base.h
@@ -58,14 +58,8 @@ struct IdBase : public AnyIdBase, public Printable<IdT> {
   }
 
   // Support simple equality comparison for ID types.
-  friend constexpr auto operator==(IdT lhs, IdT rhs) -> bool {
-    return lhs.index == rhs.index;
-  }
-  // Support equality comparison when the RHS is convertible to IdT.
-  template <typename RHSType>
-    requires std::convertible_to<RHSType, IdT>
-  friend auto operator==(IdT lhs, RHSType rhs) -> bool {
-    return lhs.index == IdT(rhs).index;
+  constexpr auto operator==(IdBase<IdT> rhs) const -> bool {
+    return index == rhs.index;
   }
 };
 
@@ -79,8 +73,8 @@ struct IndexBase : public IdBase<IdT> {
   using IdBase<IdT>::IdBase;
 
   // Support relational comparisons for index types.
-  friend auto operator<=>(IdT lhs, IdT rhs) -> std::strong_ordering {
-    return lhs.index <=> rhs.index;
+  auto operator<=>(IndexBase<IdT> rhs) const -> std::strong_ordering {
+    return this->index <=> rhs.index;
   }
 };
 
diff --git a/toolchain/check/handle_modifier.cpp b/toolchain/check/handle_modifier.cpp
index 7faed2ddf64d..7398aa4ebde4 100644
--- a/toolchain/check/handle_modifier.cpp
+++ b/toolchain/check/handle_modifier.cpp
@@ -71,7 +71,7 @@ static auto HandleModifier(Context& context, Parse::NodeId node_id,
     for (auto later_order = static_cast<int8_t>(order) + 1;
          later_order <= static_cast<int8_t>(ModifierOrder::Last);
          ++later_order) {
-      if (s.ordered_modifier_node_ids[later_order] != Parse::NodeId::Invalid) {
+      if (s.ordered_modifier_node_ids[later_order].is_valid()) {
         closest_later_modifier = s.ordered_modifier_node_ids[later_order];
         break;
       }