Because the test that tests everything would have been a superset of this test, making this redundant.  Not a big deal I suppose but that was the thinking <br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 19, 2017 at 9:57 AM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Jun 19, 2017 at 9:53 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Yea, i had put this off because i was planning on adding a test where I hand constructed a yaml file that would systematically test every possible symbol.  To add a test for this now would require editing some source by hand, compiling with msvc, and checking in the updated pdb in binary.  Do you think I should just go ahead and do that, or work on the every symbol test?<br></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>Not sure I quite follow - if the YAML based testing can hit this, why would it need to wait for a YAML test that tests everything? Could add one that tests this then add all the other cases to it in a follow-up?<br><br>But if it's coming 'soon', either way's probably OK. Would be good to followup on this thread with the revision number of the test case for posterity. :)<br><br>- Dave<br> </div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div dir="ltr">On Mon, Jun 19, 2017 at 9:28 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Test coverage?</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 12, 2017 at 1:47 PM Zachary Turner via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: zturner<br>
Date: Mon Jun 12 15:46:35 2017<br>
New Revision: 305229<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305229&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=305229&view=rev</a><br>
Log:<br>
Fix a null pointer dereference in llvm-pdbutil pretty.<br>
<br>
Static data members were causing a problem because I mistakenly<br>
assumed all members would affect a class's layout and so the<br>
Layout member would be non-null.<br>
<br>
Modified:<br>
  Â  llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp<br>
  Â  llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp?rev=305229&r1=305228&r2=305229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp?rev=305229&r1=305228&r2=305229&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp Mon Jun 12 15:46:35 2017<br>
@@ -181,13 +181,14 @@ void UDTLayoutBase::initializeChildren(c<br>
  Â  Â  Â if (Data->getDataKind() == PDB_DataKind::Member)<br>
  Â  Â  Â  Â Members.push_back(std::move(Data));<br>
  Â  Â  Â else<br>
-  Â  Â  Â  Other.push_back(std::move(Child));<br>
+  Â  Â  Â  Other.push_back(std::move(Data));<br>
  Â  Â } else if (auto VT = unique_dyn_cast<PDBSymbolTypeVTable>(Child))<br>
  Â  Â  Â VTables.push_back(std::move(VT));<br>
  Â  Â else if (auto Func = unique_dyn_cast<PDBSymbolFunc>(Child))<br>
  Â  Â  Â Funcs.push_back(std::move(Func));<br>
-  Â  else<br>
+  Â  else {<br>
  Â  Â  Â Other.push_back(std::move(Child));<br>
+  Â  }<br>
  Â }<br>
<br>
  Â // We don't want to have any re-allocations in the list of bases, so make<br>
<br>
Modified: llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp?rev=305229&r1=305228&r2=305229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp?rev=305229&r1=305228&r2=305229&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp (original)<br>
+++ llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp Mon Jun 12 15:46:35 2017<br>
@@ -151,21 +151,21 @@ bool PrettyClassLayoutGraphicalDumper::s<br>
 }<br>
<br>
 void PrettyClassLayoutGraphicalDumper::dump(const PDBSymbolData &Symbol) {<br>
-  assert(CurrentItem != nullptr);<br>
-<br>
-  DataMemberLayoutItem &Layout =<br>
-  Â  Â  static_cast<DataMemberLayoutItem &>(*CurrentItem);<br>
-<br>
  Â VariableDumper VarDumper(Printer);<br>
  Â VarDumper.start(Symbol, ClassOffsetZero);<br>
<br>
-  if (Layout.hasUDTLayout() && shouldRecurse()) {<br>
-  Â  uint32_t ChildOffsetZero = ClassOffsetZero + Layout.getOffsetInParent();<br>
-  Â  Printer.Indent();<br>
-  Â  PrettyClassLayoutGraphicalDumper TypeDumper(Printer, RecursionLevel + 1,<br>
-  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  ChildOffsetZero);<br>
-  Â  TypeDumper.start(Layout.getUDTLayout());<br>
-  Â  Printer.Unindent();<br>
+  if (CurrentItem != nullptr) {<br>
+  Â  DataMemberLayoutItem &Layout =<br>
+  Â  Â  Â  static_cast<DataMemberLayoutItem &>(*CurrentItem);<br>
+<br>
+  Â  if (Layout.hasUDTLayout() && shouldRecurse()) {<br>
+  Â  Â  uint32_t ChildOffsetZero = ClassOffsetZero + Layout.getOffsetInParent();<br>
+  Â  Â  Printer.Indent();<br>
+  Â  Â  PrettyClassLayoutGraphicalDumper TypeDumper(Printer, RecursionLevel + 1,<br>
+  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  ChildOffsetZero);<br>
+  Â  Â  TypeDumper.start(Layout.getUDTLayout());<br>
+  Â  Â  Printer.Unindent();<br>
+  Â  }<br>
  Â }<br>
<br>
  Â DumpedAnything = true;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div></div></blockquote></div>