[PATCH] D38282: llvm-dwarfdump: implement --find=<name>
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 28 11:36:44 PDT 2017
> 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
>
>
>
More information about the llvm-commits
mailing list