[PATCH] D38282: llvm-dwarfdump: implement --find=<name>

[Adrian Prantl via llvm-commits]

> On Sep 28, 2017, at 11:27 AM, David Blaikie via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> dblaikie added inline comments.
> 
> 
> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:69
> +    /// End marker.
> +    ValueIterator() : NumData(0) {}
> +
> ----------------
> = default?

Done in r314443.
> 
> 
> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:71
> +
> +    const ArrayRef<DWARFFormValue> operator*() const {
> +      return AtomForms;
> ----------------
> Should this be const /ref/ return? (or non-const value, though iterators that return by value are problematic)

I could either make it a const SmallVectorImpl<DWARFFormValue>& which leaks an implementation detail or a non-const ArrayRef< DWARFFormValue>. Which one do you think is more appropriate?

> 
> & what about operator->?
> 
> Should this class use one of the iterator facade helpers from llvm/ADT/iterator.h?

I was not aware of that. I'll take a look.

thanks,
adrian


> 
> 
> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:76
> +    ValueIterator operator++(int) {
> +      ValueIterator I(*this);
> +      Next();
> ----------------
> Prefer direct init over copy init:
> 
>  X y = z;
> 
> rather than:
> 
>  X y(z);
> 
> where possible.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D38282
> 
> 
>