[PATCH] D48899: [dsymutil] Convert recursion in lookForDIEsToKeep into worklist.

[Frederic Riss via Phabricator via llvm-commits]

friss added a comment.

This looks like it should work and keep the current semantics. Did you find that this gives identical output as the current algorithm in your testing?



================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:877
+
+    Worklist.emplace_back(Current.Die); // Continuation marker.
+    std::vector<DWARFDie> Children(Current.Die.begin(), Current.Die.end());
----------------
I would add a comment here explaining that the worklist is LIFO. It took me a moment to realize why you would enqueue the continuation first. 


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:878-880
+    std::vector<DWARFDie> Children(Current.Die.begin(), Current.Die.end());
+    std::reverse(Children.begin(), Children.end());
+    for (auto Child : Children)
----------------
Can you use something like make_reverse_iterator to avoid copying the list?


Repository:
  rL LLVM

https://reviews.llvm.org/D48899