<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 4, 2015 at 10:31 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:reid@kleckner.net" target="_blank">reid@kleckner.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rnk<br>
Date: Wed Mar 4 12:31:10 2015<br>
New Revision: 231269<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=231269&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=231269&view=rev</a><br>
Log:<br>
Revert "unique_ptrify ValID::ConstantStructElts"<br>
<br>
This reverts r231200 and r231204. The second one added an explicit move<br>
ctor for MSVC.<br>
<br>
This change broke the clang-cl self-host due to weirdness in MSVC's<br>
implementation of std::map::insert. Somehow we lost our rvalue ref-ness<br>
when going through variadic placement new:<br>
<br>
template <class _Objty, class... _Types><br>
void construct(_Objty *_Ptr,<br>
_Types &&... _Args) { // construct _Objty(_Types...) at _Ptr<br>
::new ((void *)_Ptr) _Objty(_STD forward<_Types>(_Args)...);<br>
}<br>
<br>
For some reason, Clang decided to call the deleted std::pair copy<br>
constructor at this point. Needs further investigation, once I can<br>
build.<br></blockquote><div><br>Investigated this some more - the issue seemed to be a few layers up. The actual insert function chosen seemed to be the "const value_type&" overload, not the P&& perfect forwarding template - the latter is SFINAE'd on "is_convertible" and, as per the standard. Strangely, rvalue of "std::pair<foo, int>" is not implicitly convertible to std::pair<const foo, int> - I don't know why. GCC uses is_constructible, rather than is_convertible but I think that's wrong (you could end up calling explicit ctors which you shouldn't get from 'insert', you should only get that from 'emplace')<br><br>And of course GCC 4.7 doesn't have std::map::emplace... - so, kind of stuck between a rock & a hard place.<br><br>Richard - any thoughts on any of this, or at least: why is std::pair<move_only, int> not convertible to std::pair<const move_only, int> ?<br><br>I guess I can make this change less intrusive - make ValID copyable and just assert that it's not the case where it owns memory (ensure the unique_ptr is null).<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
llvm/trunk/lib/AsmParser/LLParser.cpp<br>
llvm/trunk/lib/AsmParser/LLParser.h<br>
<br>
Modified: llvm/trunk/lib/AsmParser/LLParser.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=231269&r1=231268&r2=231269&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=231269&r1=231268&r2=231269&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/AsmParser/LLParser.cpp (original)<br>
+++ llvm/trunk/lib/AsmParser/LLParser.cpp Wed Mar 4 12:31:10 2015<br>
@@ -2365,10 +2365,9 @@ bool LLParser::ParseValID(ValID &ID, Per<br>
ParseToken(lltok::rbrace, "expected end of struct constant"))<br>
return true;<br>
<br>
- ID.ConstantStructElts.reset(new Constant *[Elts.size()]);<br>
+ ID.ConstantStructElts = new Constant*[Elts.size()];<br>
ID.UIntVal = Elts.size();<br>
- memcpy(ID.ConstantStructElts.get(), Elts.data(),<br>
- Elts.size() * sizeof(Elts[0]));<br>
+ memcpy(ID.ConstantStructElts, Elts.data(), Elts.size()*sizeof(Elts[0]));<br>
ID.Kind = ValID::t_ConstantStruct;<br>
return false;<br>
}<br>
@@ -2387,9 +2386,8 @@ bool LLParser::ParseValID(ValID &ID, Per<br>
return true;<br>
<br>
if (isPackedStruct) {<br>
- ID.ConstantStructElts.reset(new Constant *[Elts.size()]);<br>
- memcpy(ID.ConstantStructElts.get(), Elts.data(),<br>
- Elts.size() * sizeof(Elts[0]));<br>
+ ID.ConstantStructElts = new Constant*[Elts.size()];<br>
+ memcpy(ID.ConstantStructElts, Elts.data(), Elts.size()*sizeof(Elts[0]));<br>
ID.UIntVal = Elts.size();<br>
ID.Kind = ValID::t_PackedConstantStruct;<br>
return false;<br>
@@ -3949,8 +3947,8 @@ bool LLParser::ConvertValIDToValue(Type<br>
return Error(ID.Loc, "element " + Twine(i) +<br>
" of struct initializer doesn't match struct element type");<br>
<br>
- V = ConstantStruct::get(<br>
- ST, makeArrayRef(ID.ConstantStructElts.get(), ID.UIntVal));<br>
+ V = ConstantStruct::get(ST, makeArrayRef(ID.ConstantStructElts,<br>
+ ID.UIntVal));<br>
} else<br>
return Error(ID.Loc, "constant expression type mismatch");<br>
return false;<br>
<br>
Modified: llvm/trunk/lib/AsmParser/LLParser.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.h?rev=231269&r1=231268&r2=231269&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.h?rev=231269&r1=231268&r2=231269&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/AsmParser/LLParser.h (original)<br>
+++ llvm/trunk/lib/AsmParser/LLParser.h Wed Mar 4 12:31:10 2015<br>
@@ -62,17 +62,13 @@ namespace llvm {<br>
APSInt APSIntVal;<br>
APFloat APFloatVal;<br>
Constant *ConstantVal;<br>
- std::unique_ptr<Constant*[]> ConstantStructElts;<br>
+ Constant **ConstantStructElts;<br>
<br>
ValID() : Kind(t_LocalID), APFloatVal(0.0) {}<br>
- // Workaround for MSVC not synthesizing implicit move members.<br>
- ValID(ValID &&RHS)<br>
- : Kind(std::move(RHS.Kind)), Loc(std::move(RHS.Loc)),<br>
- UIntVal(std::move(RHS.UIntVal)), StrVal(std::move(RHS.StrVal)),<br>
- StrVal2(std::move(RHS.StrVal2)), APSIntVal(std::move(RHS.APSIntVal)),<br>
- APFloatVal(std::move(RHS.APFloatVal)),<br>
- ConstantVal(std::move(RHS.ConstantVal)),<br>
- ConstantStructElts(std::move(RHS.ConstantStructElts)) {}<br>
+ ~ValID() {<br>
+ if (Kind == t_ConstantStruct || Kind == t_PackedConstantStruct)<br>
+ delete [] ConstantStructElts;<br>
+ }<br>
<br>
bool operator<(const ValID &RHS) const {<br>
if (Kind == t_LocalID || Kind == t_GlobalID)<br>
<br>
<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>
</blockquote></div><br></div></div>