[LLVMdev] [patch] Union Types - work in progress

[Chris Lattner]

On Feb 9, 2010, at 4:28 PM, Talin wrote:

> ping...

Hi Talin, sorry for the delay.  FWIW, it's usually best to trickle pieces of a feature in and build it up over time, otherwise your patch just gets larger and larger.

LangRef.html:

+  <dt><b>Union constants</b></dt>
+  <dd>Union constants are represented with notation similar to a structure with
+      a single element - that is, a single typed element surrounded
+      by braces (<tt>{}</tt>)).  For example: "<tt>{ i32 4 }</tt>".  A
+      single-element constant struct can be implicitly converted to a
+      <a href="#t_union">union type</a> as long as the type of the struct
+      element matches the type of one of the union members.</dd>
+

It's a minor point, but I'd avoid the term "implicitly converted".

Constants.cpp:

-  assert(Idx->getType()->isInteger(32) &&
+  assert(Idx->getType() == Type::getInt32Ty(Val->getContext()) &&

You're reverting these changes, please don't.  There are a couple instances of this.

+void ConstantUnion::replaceUsesOfWithOnConstant(Value *From, Value *To,
+                                                 Use *U) {
+  assert(false && "Implement replaceUsesOfWithOnConstant for unions");
+}
+

Still not implemented?

+UnionType *UnionType::get(const Type *type, ...) {
+  va_list ap;
+  std::vector<const llvm::Type*> UnionFields;
+  va_start(ap, type);

Please use smallvector.

+bool UnionType::isValidElementType(const Type *ElemTy) {
+  return !ElemTy->isVoidTy() && !ElemTy->isLabelTy() &&
+         !ElemTy->isMetadataTy() && !isa<FunctionType>(ElemTy);
+}

Isn't there a better predicate somewhere?

+LLVMTypeRef LLVMUnionTypeInContext(LLVMContextRef C, LLVMTypeRef *ElementTypes,
+                           unsigned ElementCount) {
+  std::vector<const Type*> Tys;
+  for (LLVMTypeRef *I = ElementTypes,
+

indentation of unsigned and use smallvector.


+/// ParseUnionType
+///   TypeRec
+///     ::= 'union' '{' '}'
+///     ::= 'union' '{' TypeRec (',' TypeRec)* '}'
+bool LLParser::ParseUnionType(PATypeHolder &Result) {

Unions can't be empty, so the first grammar production isn't valid.

Otherwise, it looks good, please send these updates and I will commit it for you.

-Chris

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100210/a94c1c32/attachment.html>