[PATCH] D53247: [ADT] Add initializer_list constructor, equality tests for DenseMap/DenseSet.

[Lang Hames via Phabricator via llvm-commits]

lhames added inline comments.


================
Comment at: include/llvm/ADT/DenseMap.h:656-659
+  for (auto LHSI = LHS.begin(), RHSI = RHS.begin(), E = LHS.end(); LHSI != E;
+       ++LHSI, ++RHSI)
+    if (*LHSI != *RHSI)
+      return false;
----------------
lhames wrote:
> dblaikie wrote:
> > Is this guaranteed to work, given the lack of ordering guarantees? I think the order can depend on insertion order - so equivalent containers might have different iteration orders, perhaps?
> Good point. No -- this won't work.
> 
> This was mostly a convenience for unit tests, so dropping it (at the cost of some slightly more verbose unit tests) seems like the best solution.
Alternatively they could be implemented by iterating over one structure and testing that the corresponding element is present in the other. In either case I think I'll want some extra comments for them, so I will break the equality tests out into a separate patch. 


Repository:
  rL LLVM

https://reviews.llvm.org/D53247