<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 26, 2014 at 11:07 PM, Frédéric Riss <span dir="ltr"><<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On 26 Sep 2014, at 23:39, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:</div><br><div><blockquote type="cite" style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br>On Sep 26, 2014, at 6:49 AM, Frédéric Riss <<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>> wrote:<br><br><blockquote type="cite"><br>On 26 Sep 2014, at 14:34, Frederic Riss <<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>> wrote:<br><br>Author: friss<br>Date: Fri Sep 26 07:34:06 2014<br>New Revision: 218514<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=218514&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=218514&view=rev</a><br>Log:<br>Revert "Store TypeUnits in a SmallVector<DWARFUnitSection> instead of a single DWARFUnitSection."<br><br>This reverts commit r218513.<br><br>Buildbots using libstdc++ issue an error when trying to copy<br>SmallVector<std::unique_ptr<>>. Revert the commit until we have a fix.<br></blockquote><br>This wasn’t only libstdc++ in fact, MSVC also chocked and I have one libcxx bot that also failed.<br><br>David, if you look at the commit, you’ll see that I didn’t replace the SmallVectors we talked about by std::vectors. I tried it, but it failed in a very similar way to the builders. I thought this was some libcxx or SmallVector issue and I was going to report it with a reproducer:<br><br>#include <vector><br>#include <memory><br>#include <llvm/ADT/SmallVector.h><br><br>typedef llvm::SmallVector<std::unique_ptr<int>,1> vec_type;<br>std::vector<vec_type> vecvec;<br><br>void foo() {<br><span style="white-space:pre-wrap"> </span>vecvec.push_back(vec_type());<br>}<br><br>Is this code somehow wrong?<br>This fails to compile for me because the move semantics are lost somewhere and the implicitly deleted unique_ptr copy constructor is called. Note that other permutations of vector and SmallVector seem to work.<br><br>If I add explicit default and move constructors to DWARFUnitSection, then I am able to replace the SmallVectors by vectors as we discussed, but I don’t understand why this is necessary. Maybe this would also fix the issue seen on the builders, but in either case I can’t see why the compiler generated constructors don’t work.<span> </span><br><br>For the record, here are some builder logs:<br><a href="http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/21968/steps/compile/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/21968/steps/compile/logs/stdio</a><br></blockquote><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">I looked at this one. It looks to me like the compiler hasn’t generated</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">a move constructor for `DWARFUnitSection`. I knew MSVC had issues like</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">this, but maybe GCC has trouble to. We can’t really trust compilers to</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">auto-generate C++11 constructors properly.</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"></div></blockquote><div><br></div></div></div><div>By ‘compilers’ you mean clang too ? because there was one clang++ failure in the logs I provided (on freebsd). I suppose it can be an old version that had the same kind of issues.</div></div></div></blockquote><div><br></div><div>Not sure what happened there - I could only guess at attributing that to an old standard library that didn't have an rvalue overload of vector::push_back</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><br><blockquote type="cite"><div><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">Looking at the code, you probably just need something like this:</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important"> DwarfUnitSection(DwarfUnitSection &&X)</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important"> : SmallVector<std::unique_ptr<UnitType>>(std::move(X)) {}</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"></div></blockquote><div><br></div></span><div>Yeah, and also add an empty default constructor. I just wanted to make sure that the code in itself wasn’t flawed in a way that I didn’t understand.</div><div><br></div><div>Thanks for looking into it!</div><div>Fred</div><div><div class="h5"><br><blockquote type="cite"><div><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">If MSVC supported it, you could do this instead:</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important"> DwarfUnitSection(DwarfUnitSection &&) = default;</span><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote type="cite" style="font-family:Menlo-Regular;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><a href="http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/8474/steps/build_Lld/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/8474/steps/build_Lld/logs/stdio</a><br><a href="http://lab.llvm.org:8011/builders/lld-x86_64-win7/builds/13282/steps/build_Lld/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/lld-x86_64-win7/builds/13282/steps/build_Lld/logs/stdio</a><br><br>Fred<br><br><blockquote type="cite">Modified:<br> llvm/trunk/lib/DebugInfo/DWARFContext.cpp<br> llvm/trunk/lib/DebugInfo/DWARFContext.h<br><br>Modified: llvm/trunk/lib/DebugInfo/DWARFContext.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.cpp?rev=218514&r1=218513&r2=218514&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.cpp?rev=218514&r1=218513&r2=218514&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/DebugInfo/DWARFContext.cpp (original)<br>+++ llvm/trunk/lib/DebugInfo/DWARFContext.cpp Fri Sep 26 07:34:06 2014<br>@@ -86,17 +86,15 @@ void DWARFContext::dump(raw_ostream &OS,<br><br> if ((DumpType == DIDT_All || DumpType == DIDT_Types) && getNumTypeUnits()) {<br> OS << "\n.debug_types contents:\n";<br>- for (const auto &TUS : type_unit_sections())<br>- for (const auto &TU : TUS)<br>- TU->dump(OS);<br>+ for (const auto &TU : type_units())<br>+ TU->dump(OS);<br> }<br><br> if ((DumpType == DIDT_All || DumpType == DIDT_TypesDwo) &&<br> getNumDWOTypeUnits()) {<br> OS << "\n.debug_types.dwo contents:\n";<br>- for (const auto &DWOTUS : dwo_type_unit_sections())<br>- for (const auto &DWOTU : DWOTUS)<br>- DWOTU->dump(OS);<br>+ for (const auto &DWOTU : dwo_type_units())<br>+ DWOTU->dump(OS);<br> }<br><br> if (DumpType == DIDT_All || DumpType == DIDT_Loc) {<br>@@ -339,17 +337,15 @@ void DWARFContext::parseTypeUnits() {<br> uint32_t offset = 0;<br> const DataExtractor &DIData =<br> DataExtractor(I.second.Data, isLittleEndian(), 0);<br>- TUs.push_back(DWARFUnitSection<DWARFTypeUnit>());<br>- auto &TUS = TUs.back();<br> while (DIData.isValidOffset(offset)) {<br> std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,<br> getDebugAbbrev(), I.second.Data, getRangeSection(),<br> getStringSection(), StringRef(), getAddrSection(),<br>- &I.second.Relocs, isLittleEndian(), TUS));<br>+ &I.second.Relocs, isLittleEndian(), TUs));<br> if (!TU->extract(DIData, &offset))<br> break;<br>- TUS.push_back(std::move(TU));<br>- offset = TUS.back()->getNextUnitOffset();<br>+ TUs.push_back(std::move(TU));<br>+ offset = TUs.back()->getNextUnitOffset();<br> }<br> }<br>}<br>@@ -380,17 +376,15 @@ void DWARFContext::parseDWOTypeUnits() {<br> uint32_t offset = 0;<br> const DataExtractor &DIData =<br> DataExtractor(I.second.Data, isLittleEndian(), 0);<br>- DWOTUs.push_back(DWARFUnitSection<DWARFTypeUnit>());<br>- auto &TUS = DWOTUs.back();<br> while (DIData.isValidOffset(offset)) {<br> std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,<br> getDebugAbbrevDWO(), I.second.Data, getRangeDWOSection(),<br> getStringDWOSection(), getStringOffsetDWOSection(), getAddrSection(),<br>- &I.second.Relocs, isLittleEndian(), TUS));<br>+ &I.second.Relocs, isLittleEndian(), DWOTUs));<br> if (!TU->extract(DIData, &offset))<br> break;<br>- TUS.push_back(std::move(TU));<br>- offset = TUS.back()->getNextUnitOffset();<br>+ DWOTUs.push_back(std::move(TU));<br>+ offset = DWOTUs.back()->getNextUnitOffset();<br> }<br> }<br>}<br><br>Modified: llvm/trunk/lib/DebugInfo/DWARFContext.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.h?rev=218514&r1=218513&r2=218514&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.h?rev=218514&r1=218513&r2=218514&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/DebugInfo/DWARFContext.h (original)<br>+++ llvm/trunk/lib/DebugInfo/DWARFContext.h Fri Sep 26 07:34:06 2014<br>@@ -30,7 +30,7 @@ namespace llvm {<br>class DWARFContext : public DIContext {<br><br> DWARFUnitSection<DWARFCompileUnit> CUs;<br>- SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> TUs;<br>+ DWARFUnitSection<DWARFTypeUnit> TUs;<br> std::unique_ptr<DWARFDebugAbbrev> Abbrev;<br> std::unique_ptr<DWARFDebugLoc> Loc;<br> std::unique_ptr<DWARFDebugAranges> Aranges;<br>@@ -38,7 +38,7 @@ class DWARFContext : public DIContext {<br> std::unique_ptr<DWARFDebugFrame> DebugFrame;<br><br> DWARFUnitSection<DWARFCompileUnit> DWOCUs;<br>- SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> DWOTUs;<br>+ DWARFUnitSection<DWARFTypeUnit> DWOTUs;<br> std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;<br> std::unique_ptr<DWARFDebugLocDWO> LocDWO;<br><br>@@ -77,7 +77,6 @@ public:<br><br> typedef DWARFUnitSection<DWARFCompileUnit>::iterator_range cu_iterator_range;<br> typedef DWARFUnitSection<DWARFTypeUnit>::iterator_range tu_iterator_range;<br>- typedef iterator_range<SmallVectorImpl<DWARFUnitSection<DWARFTypeUnit>>::iterator> tu_section_iterator_range;<br><br> /// Get compile units in this context.<br> cu_iterator_range compile_units() {<br>@@ -86,9 +85,9 @@ public:<br> }<br><br> /// Get type units in this context.<br>- tu_section_iterator_range type_unit_sections() {<br>+ tu_iterator_range type_units() {<br> parseTypeUnits();<br>- return tu_section_iterator_range(TUs.begin(), TUs.end());<br>+ return tu_iterator_range(TUs.begin(), TUs.end());<br> }<br><br> /// Get compile units in the DWO context.<br>@@ -98,9 +97,9 @@ public:<br> }<br><br> /// Get type units in the DWO context.<br>- tu_section_iterator_range dwo_type_unit_sections() {<br>+ tu_iterator_range dwo_type_units() {<br> parseDWOTypeUnits();<br>- return tu_section_iterator_range(DWOTUs.begin(), DWOTUs.end());<br>+ return tu_iterator_range(DWOTUs.begin(), DWOTUs.end());<br> }<br><br> /// Get the number of compile units in this context.<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></blockquote></div></div></div><br></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>