[PATCH] D78919: Changing DenseMapIterator's comparisons to be hidden friends.

[Barry Revzin via Phabricator via llvm-commits]

BRevzin marked an inline comment as done.
BRevzin added inline comments.


================
Comment at: llvm/include/llvm/ADT/DenseMap.h:1247-1259
+  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 LHS.Ptr == RHS.Ptr;
----------------
dblaikie wrote:
> 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)? 
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78919/new/

https://reviews.llvm.org/D78919