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

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 5 16:12:03 PST 2017


On Thu, Jan 5, 2017 at 4:08 PM Greg Clayton via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
> > On Jan 5, 2017, at 3:46 PM, David Blaikie via Phabricator <
> reviews at reviews.llvm.org> wrote:
> >
> > dblaikie accepted this revision.
> > dblaikie added a comment.
> > This revision is now accepted and ready to land.
> >
> > One minor comment.
> >
> > Also would still love to hear from Chris about testing more interesting
> cases of non-trivial DWARF and API interactions. I mean perhaps we'll end
> up with all parsing failures testable with llvm-dwarfdump and something
> like obj2yaml, then all API interactions we'll test with the DWARF
> generation APIs and make sure they can express all valid DWARF. (though I'm
> wondering at that point if it wouldn't be better for us to just create the
> DWARF objects directly - in fact, in this case we could do that, perhaps -
> rather than parsing DWARF, we could just create the DWARF parser API
> objects directly and check that the accessors all work, etc)
>
> We probably can move some of the bits that are in the obj2yaml into a
> library and have the tool link against the library. This would then allow
> unit tests to link against the library and convert yaml to DWARF for
> testing invalid DWARF. A lot of DWARF tests must be done at the API level
> so we can verify we don’t break anything as we modify the DWARF parser.
>

I'd be surprised by that last bit - it seems most of the invalid DWARF
tests should fail in fairly coarse ways that aren't too interesting to the
API interactions (though I could see various lazy parsing scenarios hitting
errors relatively late & could be interesting to check their errors?).


>
> >
> >
> >
> > ================
> > Comment at: unittests/DebugInfo/DWARF/DwarfGenerator.cpp:112
> > +void dwarfgen::DIE::setForceChildren() {
> > +  if (Die)
> > +    Die->setForceChildren(true);
> > ----------------
> > Is this conditional needed - or do we generally assume it's an invariant
> that the Die is non-null for most of the operations on dwarfgen::DIE?
> >
> >
> > https://reviews.llvm.org/D28303
> >
> >
> >
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170106/50a01d49/attachment.html>


More information about the llvm-commits mailing list