[PATCH] D27995: Add the ability for DWARFDie objects to get the parent DWARFDie.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 20 13:16:11 PST 2016


On Tue, Dec 20, 2016 at 1:01 PM Greg Clayton via Phabricator <
reviews at reviews.llvm.org> wrote:

> clayborg added a comment.
>
> I will update the test cases to not add attributes and not do all the
> variants. David: let me know if you agree with my assessment on the A1
> sibling case.
>
>
>
> ================
> Comment at: lib/DebugInfo/DWARF/DWARFUnit.cpp:416
> +      return getDIEAtIndex(I);
> +  }
> +  return DWARFDie();
> ----------------
> Actually it won't because your example actually looks like:
>
> ```
> A
>   A1
>   NULL
> B
>   B1
>   NULL
> NULL
> ```
>
> So the sibling of "A1" would be the first NULL and that NULL wouldn't
> return a sibling (see line 409).
>

Ah, fair enough.


> I can modify the test case to test this though as a follow on patch I was
> going to do was to actually not store the NULL dies in the DieArray.


Sure, sounds good.

I'd probably suggest adding the test now - to demonstrate this works today,
then when you remove NULL dies, it should continue to work/not regress.
(though perhaps the API will look different when you remove NULL dies, I'm
not sure)


> The NULL dies take up a lot of memory, but if I remove them, then I will
> need to make sure the above function behaves correctly by watching for a
> depth to go lower than the current depth, so adding a test as you mentioned
> is a good idea.
>
>
>
> ================
> Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1004
> +  Arg2Die.addAttribute(DW_AT_name, DW_FORM_strp, "j");
> +  Arg2Die.addAttribute(DW_AT_type, DW_FORM_ref_addr, IntDie);
> +
> ----------------
> Yeah, I can remove them. I was thinking about that as I was making the
> test.
>
>
> ================
> Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1112
> +  TestRelations<4, AddrType>();
> +}
> +
> ----------------
> Sounds good, I will just do a single variation for this then.
>
>
> https://reviews.llvm.org/D27995
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161220/2fd253f2/attachment.html>


More information about the llvm-commits mailing list