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

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:882
+    // Add children in reverse order to process the children in order.
+    for (auto Child : reverse(Current.Die.children()))
+      Worklist.emplace_back(Child, Current.Flags);
----------------
friss wrote:
> I think this is broken. A reverse_iterator operator* is implemented as such:
> ```
> reference operator*() const {_Iter __tmp = current; return *--__tmp;}
> ```
> (_Iter is the base iterator type).
> Unfortunately, the DWARFDie iterators contain a DWARFDie (rather than holding a reference), so this returns a reference inside a temporary.
> DWARFDies are just temporary helper structures to make sure we always keep a DIE and its compile unit in sync, the iterators can't point to a permanent copy. I think you should just abandon the range-based for-loop and use getPreviousSibling() directly.
> 
I created D49234 as this can also occur with the forward iterator. 


https://reviews.llvm.org/D48899