[PATCH] D49679: [DebugInfo] Have custom std::reverse_iterator<DWARFDie>

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h:383-384
+private:
+  llvm::DWARFDie Die;
+  bool AtEnd;
+
----------------
Could a null DWARFDie (a default constructed one, rather than a non-default constructed one that points to a null DIE) be used as the 'rend' value - avoiding the need for the bool?


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h:413-414
+
+  explicit operator bool() const { return Die.isValid() && !AtEnd; }
+
+  const llvm::DWARFDie &operator*() const {
----------------
Is this used/needed? Might be easier to leave it out & require users to do comparisons with the rend iterator?


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h:420
+
+  bool operator==(const reverse_iterator<llvm::DWARFDie::iterator> &X) const {
+    return Die == X.Die && AtEnd == X.AtEnd;
----------------
Any operator that can be a non-member should be, ideally (so op== and op!= for example), so that any implicit conversions can occur equally on the LHS and RHS of the expression.


https://reviews.llvm.org/D49679