[PATCH] D23649: [ADT] Change the range-based for loop in DFS to normal loop. NFC.

Tim Shen via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 18 01:35:34 PDT 2016


timshen created this revision.
timshen added a reviewer: dblaikie.
timshen added a subscriber: llvm-commits.

Using range-based for loop copies the iterator, so it ends up not
updating *Opt.

Before the change, the DFS result is correct, because Visited keeps track
of visited nodes. It's just DFS is slower, because for every time
toNext() is called, the for loop starts over at child_begin().

https://reviews.llvm.org/D23649

Files:
  include/llvm/ADT/DepthFirstIterator.h

Index: include/llvm/ADT/DepthFirstIterator.h
===================================================================
--- include/llvm/ADT/DepthFirstIterator.h
+++ include/llvm/ADT/DepthFirstIterator.h
@@ -107,7 +107,8 @@
       if (!Opt)
         Opt.emplace(GT::child_begin(Node));
 
-      for (NodeRef Next : make_range(*Opt, GT::child_end(Node))) {
+      for (auto &It = *Opt; It != GT::child_end(Node); ++It) {
+        NodeRef Next = *It;
         // Has our next sibling been visited?
         if (this->Visited.insert(Next).second) {
           // No, do it now.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23649.68494.patch
Type: text/x-patch
Size: 568 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160818/9ac7d4d0/attachment.bin>


More information about the llvm-commits mailing list