[PATCH] D145337: [ADT] Avoid needless iterator copies in `zippy`

[Markus Böck via Phabricator via llvm-commits]

zero9178 added inline comments.


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:789
+  template <size_t... Ns> void tup_inc(std::index_sequence<Ns...>) {
+    (void)(++std::get<Ns>(iterators), ...);
   }
----------------
ultra nit: We only usually use explicit casts to void for silencing unused variable warning don't we? Looks a bit odd here imo


================
Comment at: llvm/unittests/ADT/IteratorTest.cpp:731
+  // just when the range is moved into `zippy`.
+  EXPECT_GE(copy_count, 2u);
+  // We expect the number of copies to be much smaller than the number of loop
----------------
How fragile is this test? Isn't the initial copy count more or less an implementation detail of zippy? Would it maybe suffice to create the zippy range before the loop, record the count, then do the loop, and then ensure that the count after the loop is equal to before?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145337