[PATCH] D28303: Add iterator support to DWARFDie to allow child DIE iteration.

[Greg Clayton via Phabricator via llvm-commits]

clayborg marked an inline comment as done.
clayborg added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDie.h:391
+    if (Die.isNULL())
+      Die = DWARFDie();
+    return *this;
----------------
I would rather not tackle reconstituting the NULL Die in this patch. I would rather do that if we remove the NULL dies. I think it is fair to say that when iterating across the children that we skip the NULL DIEs. I can fix the issue where we have only a NULL DIE as the only child.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDie.h:397
+  bool operator==(const iterator &X) const { return Die == X.Die; }
+  const DWARFDie *operator->() const { return &Die; }
+};
----------------
dblaikie wrote:
> If you have op* return a reference (as it's meant to to conform to the iterator concept in the C++ standard) do you still need to write op->, or do you get that for free from the facade?
If I return a "const DWARFDie &" from operator* I get:

```
/Volumes/Data/Users/gclayton/Documents/src/llvm/tot/llvm/include/llvm/ADT/iterator.h:138:12: error: cannot initialize return object of type 'llvm::DWARFDie *' with an rvalue of type 'const llvm::DWARFDie *'
```

For the following code in iterator.h:

```
  PointerT operator->() const {
    return &static_cast<const DerivedT *>(this)->operator*();
  }
```

If I make operator* return a "DWARFDie &" then it works and I can remove operator->:

```
  DWARFDie &operator*() const { return const_cast<DWARFDie &>(Die); }
```

Without the cast it complains about a const function converting to a non const DWARFDie &. Which solution do you prefer?


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDie.h:410-412
+inline iterator_range<DWARFDie::iterator> DWARFDie::children() const {
+  return make_range(begin(), end());
+}
----------------
dblaikie wrote:
> Probably just put this one directly inline in the class, since it can be
It can't it needs to know what a DWARFDie::iterator is. I tried but got a compile error.


https://reviews.llvm.org/D28303