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

Lang Hames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 14 09:05:56 PDT 2018


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;
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D53247





More information about the llvm-commits mailing list