From a00777262b110c838d809d34aee5db5ddb9f0ace Mon Sep 17 00:00:00 2001
From: Arthur O'Dwyer <arthur.j.odwyer@gmail.com>
Date: Wed, 3 Aug 2022 15:47:11 -0400
Subject: [PATCH] [small_map] Use hidden friends for operator==/!=

`clang -std=c++2b` complains about the old way:

    ./src/base/small_map.h:555:40: warning: all paths through
    this function will call itself [-Winfinite-recursion]
        const const_iterator& other) const {
                                           ^

There are two pieces to the "right" solution: First, use the hidden
friend idiom to make sure all our operator==s are symmetrical and
don't treat the first argument differently from the second. Second,
eliminate the heterogeneous comparison operators; just rely
on the existing implicit conversion from `iterator` to `const_iterator`
to do the right thing.
---
 src/base/small_map.h | 42 ++++++++++++------------------------------
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/src/base/small_map.h b/src/base/small_map.h
index 3e17d71..8c462f0 100644
--- a/src/base/small_map.h
+++ b/src/base/small_map.h
@@ -194,21 +194,18 @@ class small_map {
       }
     }
 
-    inline bool operator==(const iterator& other) const {
-      if (array_iter_ != NULL) {
-        return array_iter_ == other.array_iter_;
+    friend bool operator==(const iterator& a, const iterator& b) {
+      if (a.array_iter_ != NULL) {
+        return a.array_iter_ == b.array_iter_;
       } else {
-        return other.array_iter_ == NULL && hash_iter_ == other.hash_iter_;
+        return b.array_iter_ == NULL && a.hash_iter_ == b.hash_iter_;
       }
     }
 
-    inline bool operator!=(const iterator& other) const {
-      return !(*this == other);
+    friend bool operator!=(const iterator& a, const iterator& b) {
+      return !(a == b);
     }
 
-    bool operator==(const const_iterator& other) const;
-    bool operator!=(const const_iterator& other) const;
-
    private:
     friend class small_map;
     friend class const_iterator;
@@ -277,16 +274,16 @@ class small_map {
       }
     }
 
-    inline bool operator==(const const_iterator& other) const {
-      if (array_iter_ != NULL) {
-        return array_iter_ == other.array_iter_;
+    friend bool operator==(const const_iterator& a, const const_iterator& b) {
+      if (a.array_iter_ != NULL) {
+        return a.array_iter_ == b.array_iter_;
       } else {
-        return other.array_iter_ == NULL && hash_iter_ == other.hash_iter_;
+        return b.array_iter_ == NULL && a.hash_iter_ == b.hash_iter_;
       }
     }
 
-    inline bool operator!=(const const_iterator& other) const {
-      return !(*this == other);
+    friend bool operator!=(const const_iterator& a, const const_iterator& b) {
+      return !(a == b);
     }
 
    private:
@@ -549,21 +546,6 @@ class small_map {
   }
 };
 
-template <typename NormalMap, int kArraySize, typename EqualKey,
-          typename Functor>
-inline bool small_map<NormalMap, kArraySize, EqualKey,
-                      Functor>::iterator::operator==(
-    const const_iterator& other) const {
-  return other == *this;
-}
-template <typename NormalMap, int kArraySize, typename EqualKey,
-          typename Functor>
-inline bool small_map<NormalMap, kArraySize, EqualKey,
-                      Functor>::iterator::operator!=(
-    const const_iterator& other) const {
-  return other != *this;
-}
-
 }
 
 #endif  // UTIL_GTL_SMALL_MAP_H_