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

Markus Böck via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 6 11:33:24 PST 2023


zero9178 accepted this revision.
zero9178 added a comment.
This revision is now accepted and ready to land.

LGTM thanks!



================
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
----------------
kuhar wrote:
> zero9178 wrote:
> > 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?
> The number of copies in the loop is still >0 (4 to be exact), but I followed your suggestion and separated the pre-loop count from the in-loop count.
Ahh I see, these seem to come from the `begin` and `end` calls that the for-range loop desugars to. One could probably separate the counting of those too, but I think the current method is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145337



More information about the llvm-commits mailing list