OK here's a new patch. Additional comments below.<br><br><div class="gmail_quote">On Wed, Feb 10, 2010 at 10:57 AM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@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"><div><div><br></div><div>LangRef.html:</div><div><br></div><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+ <dt><b>Union constants</b></dt></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ <dd>Union constants are represented with notation similar to a structure with</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ a single element - that is, a single typed element surrounded</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+ by braces (<tt>{}</tt>)). For example: "<tt>{ i32 4 }</tt>". A</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ single-element constant struct can be implicitly converted to a</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ <a href="#t_union">union type</a> as long as the type of the struct</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+ element matches the type of one of the union members.</dd></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
<br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">It's a minor point, but I'd avoid the term "implicitly converted".</div></div></div></div></blockquote><div><br></div>
<div>done </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><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
<br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">Constants.cpp:</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">- assert(Idx->getType()->isInteger(32) &&</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+ assert(Idx->getType() == Type::getInt32Ty(Val->getContext()) &&</div>
<div><br></div></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">You're reverting these changes, please don't. There are a couple instances of this.</div></div></div></div></blockquote>
<div><br></div><div>Didn't mean to :) Hope that the train hasn't moved too far while I was editing this. </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><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+void ConstantUnion::replaceUsesOfWithOnConstant(Value *From, Value *To,</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ Use *U) {</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ assert(false && "Implement replaceUsesOfWithOnConstant for unions");</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+}</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+</div><div><br></div><div>Still not implemented?</div></div></div><div><br></div></div></div></blockquote><div>Not in this patch - as you say, it's too large already.</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><div></div><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+UnionType *UnionType::get(const Type *type, ...) {</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ va_list ap;</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+ std::vector<const llvm::Type*> UnionFields;</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ va_start(ap, type);</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
<br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">Please use smallvector.</div></div></div></div></blockquote><div><br></div><div>Done - although I was just copying from what Struct does. </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><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
<br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><div class="im"><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+bool UnionType::isValidElementType(const Type *ElemTy) {</div>
</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ return !ElemTy->isVoidTy() && !ElemTy->isLabelTy() &&</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+ !ElemTy->isMetadataTy() && !isa<FunctionType>(ElemTy);</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+}</div><div><br></div><div>Isn't there a better predicate somewhere?</div>
</div></div></div></div></blockquote><div><br></div><div>Apparently there is now. Done. </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><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px"><div><br></div><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+LLVMTypeRef LLVMUnionTypeInContext(LLVMContextRef C, LLVMTypeRef *ElementTypes,</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ unsigned ElementCount) {</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ std::vector<const Type*> Tys;</div>
<div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ for (LLVMTypeRef *I = ElementTypes,</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
<br></div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">indentation of unsigned and use smallvector.</div></div></div></div><div><br></div></div></div></blockquote><div>Done. </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><div></div><div><br></div><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+/// ParseUnionType</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+/// TypeRec</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+/// ::= 'union' '{' '}'</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+/// ::= 'union' '{' TypeRec (',' TypeRec)* '}'</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+bool LLParser::ParseUnionType(PATypeHolder &Result) {</div>
<div><font face="Inconsolata" size="3"><span style="font-size:13px"><br></span></font></div><div><font face="Inconsolata" size="3"><span style="font-size:13px">Unions can't be empty, so the first grammar production isn't valid.</span></font></div>
</div></div></div></blockquote><div><br></div><div>Done. </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><div><div><font face="Inconsolata" size="3"><span style="font-size:13px"><br>
</span></font></div><div><font face="Inconsolata" size="3"><span style="font-size:13px">Otherwise, it looks good, please send these updates and I will commit it for you.</span></font></div><div><font face="Inconsolata" size="3"><span style="font-size:13px"><br>
</span></font></div></div></div></div></blockquote><div>As much as it pains me to say anything that would delay this getting committed, it might make sense to apply this patch after the code freeze - since the feature isn't going to be finished in time for 2.7.</div>
<div><br></div><div>-- </div></div>-- Talin<br>