<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 21, 2016 at 5:04 PM Adrian Prantl via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">aprantl added inline comments.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: lib/DebugInfo/DWARF/DWARFDie.cpp:359<br class="gmail_msg">
   if (!isValid())<br class="gmail_msg">
-    return;<br class="gmail_msg">
+    return -1U;<br class="gmail_msg">
   DataExtractor debug_info_data = U->getDebugInfoExtractor();<br class="gmail_msg">
----------------<br class="gmail_msg">
Is there a better alternative to returning a magic value?<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: tools/dsymutil/DwarfLinker.cpp:1799<br class="gmail_msg">
   if (DIE.hasChildren())<br class="gmail_msg">
-    for (auto Child = DIE.getFirstChild(); Child && !Child.isNULL();<br class="gmail_msg">
-         Child = Child.getSibling())<br class="gmail_msg">
+    for (auto Child = DIE.getFirstChild(); Child; Child = Child.getSibling())<br class="gmail_msg">
       Info.Prune &= analyzeContextInfo(Child, MyIdx, CU, CurrentDeclContext,<br class="gmail_msg">
----------------<br class="gmail_msg">
Might make sense to add a method that returns something like a SiblingIterator at some point in the future?<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1050<br class="gmail_msg">
   auto B2 = B1.getSibling();<br class="gmail_msg">
-  EXPECT_TRUE(B2.getSibling().isNULL());<br class="gmail_msg">
+  EXPECT_FALSE(B2.getSibling().isValid());<br class="gmail_msg">
<br class="gmail_msg">
----------------<br class="gmail_msg">
Should we introduce something like a hasSibling() method? This almost sounds like it has a corrupted sibling DIE.<br class="gmail_msg"></blockquote><div><br>The right thing, as you note above, is to have an iterator provided by the parent - children shouldn't/needn't know about their siblings.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D28040" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D28040</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>