[PATCH] D49679: [DebugInfo] Have custom DWARFDie:: reverse_iterator

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 24 03:30:33 PDT 2018


JDevlieghere added a comment.

In https://reviews.llvm.org/D49679#1173122, @labath wrote:

> Sorry I wrote the reply without looking at the patch in detail. I did not notice you demoted the iterator back to "forward". In case this is not a bidirectional operator, but a collection of two forward iterators that happen to iterate in the reverse order, the specializing std::reverse_iterator is not the right approach.
>
> I have to admit I haven't thought about the specifics of how the reverse iterator would work until now. However, now that I think about it, I think it could be implemented as a pair of DWARFDie + bool flag. The trick would be that we store the element that we really dereference to, instead of its successor. The bool flag would be used denote the before_begin state, as we cannot represent rend() this way. The implementation would do something like:
>
>   operator++() {
>     assert(!AtEnd && "Incrementing rend");
>     DWARFDie D = Die.getPreviousSibling();
>     if (D)
>       Die = D;
>     else
>       AtEnd = true;
>   }
>  
>   operator--() {
>     if (AtEnd) {
>       AtEnd = false;
>       return;
>     }
>     Die = Die.getNextSibling();
>     assert(!Die.isNULL() && "Decrementing rbegin");
>   }
>


I think I'm missing something; what are you implementing here exactly and what problem is it trying to solve?

> However, if you don't need that extra complexity now, then I suppose this solution is fine too.


https://reviews.llvm.org/D49679





More information about the llvm-commits mailing list