Changing DenseMapIterator's comparisons to be hidden friends.

[BRevzin]

Summary

The existing implementation breaks in C++20 (sorry, see this StackOverflow answer), the new implementation works fine under all language versions.

[BRevzin]

This review changed just the one type, I went ahead and fixed the rest of the clang build in a bigger review.

[dblaikie]

llvm/include/llvm/ADT/DenseMap.h

1247–1259

Should these use ConstIterator parameters? Guess that'd reduce the number of instantiations/versions of this function (rather than having const and non-const versions of the function instantiated)?

[BRevzin]

llvm/include/llvm/ADT/DenseMap.h

1247–1259

FYI I moved this review to https://reviews.llvm.org/D78938.

The problem is, if I declared it using ConstIterator then both the iterator and const iterator versions would be declaring the same function. I don't know of a way to avoid that.

llvm/include/llvm/ADT/DenseMap.h

public:
   pointer operator->() const {
     assert(isHandleInSync() && "invalid iterator access!");
     assert(Ptr != End && "dereferencing end() iterator");
     if (shouldReverseIterate<KeyT>())
       return &(Ptr[-1]);
     return Ptr;
   }
 
-  bool operator==(const ConstIterator &RHS) const {
-    assert((!Ptr || isHandleInSync()) && "handle not in sync!");
+  friend bool operator==(const DenseMapIterator &LHS,
+                         const DenseMapIterator &RHS) {
+    assert((!LHS.Ptr || LHS.isHandleInSync()) && "handle not in sync!");
     assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");
-    assert(getEpochAddress() == RHS.getEpochAddress() &&
+    assert(LHS.getEpochAddress() == RHS.getEpochAddress() &&
            "comparing incomparable iterators!");
-    return Ptr == RHS.Ptr;
+    return LHS.Ptr == RHS.Ptr;
   }
-  bool operator!=(const ConstIterator &RHS) const {
-    assert((!Ptr || isHandleInSync()) && "handle not in sync!");
-    assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");
-    assert(getEpochAddress() == RHS.getEpochAddress() &&
-           "comparing incomparable iterators!");
-    return Ptr != RHS.Ptr;
+
+  friend bool operator!=(const DenseMapIterator &LHS,
+                         const DenseMapIterator &RHS) {
+    return !(LHS == RHS);
   }
 
   inline DenseMapIterator& operator++() {  // Preincrement
     assert(isHandleInSync() && "invalid iterator access!");
     assert(Ptr != End && "incrementing end() iterator");
     if (shouldReverseIterate<KeyT>()) {
       --Ptr;
       RetreatPastEmptyBuckets();
       return *this;