[PATCH] D142556: [DebugInfo] Merge partially matching chains of inlined locations

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:155-157
+    // The + 1 is to account for the &*rev_it = &(it - 1) relationship.
+    ARIt = LocVec::reverse_iterator(ALocs.begin() + IT->second + 1);
+    BRIt = LocVec::reverse_iterator(BLocs.begin() + I + 1);
----------------
dstenb wrote:
> dblaikie wrote:
> > Would it be easier to express/read as arithmetic on rbegin, rather than begin->arithmetic->reverse?
> The corresponding initializations based on `rbegin` would be:
> 
> ```
> ARIt = ALocs.rbegin() + (ALocs.size() - IT->second - 1);
> BRIt = BLocs.rbegin() + (BLocs.size() - I - 1);
> ```
> 
> I think that they are about the same in terms of readability, so happy to pick either or.
Yeah, I think I'd lean /slightly/ towards the latter/not having to explicitly construct a reverse iterator, but no big deal either way, indeed. Your call.


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

https://reviews.llvm.org/D142556