Cleanup ConstantExpr handling:

 * Correctly delete TypeHandles in AsmParser.  In addition to not leaking
   memory, this prevents a bug that could have occurred when a type got
   resolved that the constexpr was using
 * Check for errors in the AsmParser instead of hitting assertion failures
   deep in the code
 * Simplify the interface to the ConstantExpr class, removing unneccesary
   parameters to the ::get* methods.
 * Rename the 'getelementptr' version of ConstantExpr::get to
   ConstantExpr::getGetElementPtr


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@3161 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 61 insertions, 54 deletions

diff --git a/lib/AsmParser/llvmAsmParser.y b/lib/AsmParser/llvmAsmParser.y
index 96ba4c8..828c41a 100644
--- a/lib/AsmParser/llvmAsmParser.y
+++ b/lib/AsmParser/llvmAsmParser.y
@@ -8,17 +8,14 @@
 #include "ParserInternals.h"
 #include "llvm/SymbolTable.h"
 #include "llvm/Module.h"
-#include "llvm/GlobalVariable.h"
 #include "llvm/iTerminators.h"
 #include "llvm/iMemory.h"
 #include "llvm/iPHINode.h"
-#include "llvm/Argument.h"
 #include "Support/STLExtras.h"
 #include "Support/DepthFirstIterator.h"
 #include <list>
-#include <utility>            // Get definition of pair class
+#include <utility>
 #include <algorithm>
-#include <iostream>
 using std::list;
 using std::vector;
 using std::pair;
@@ -972,33 +969,53 @@ ConstVal: Types '[' ConstVector ']' { // Nonempty unsized arr
 
 // FIXME: ConstExpr::get never return null!  Do checking here in the parser.
 ConstExpr: Types CAST ConstVal {
-    $$ = ConstantExpr::get($2, $3, $1->get());
+    $$ = ConstantExpr::get(Instruction::Cast, $3, $1->get());
     if ($$ == 0) ThrowException("constant expression builder returned null!");
+    delete $1;
   }
   | Types GETELEMENTPTR '(' ConstVal IndexList ')' {
+    if (!isa<PointerType>($4->getType()))
+      ThrowException("GetElementPtr requires a pointer operand!");
+
+    const Type *IdxTy =
+      GetElementPtrInst::getIndexedType($4->getType(), *$5, true);
+    if (!IdxTy)
+      ThrowException("Index list invalid for constant getelementptr!");
+    if (PointerType::get(IdxTy) != $1->get())
+      ThrowException("Declared type of constant getelementptr is incorrect!");
+
     vector<Constant*> IdxVec;
     for (unsigned i = 0, e = $5->size(); i != e; ++i)
       if (Constant *C = dyn_cast<Constant>((*$5)[i]))
         IdxVec.push_back(C);
       else
-        ThrowException("Arguments to getelementptr must be constants!");
+        ThrowException("Indices to constant getelementptr must be constants!");
 
     delete $5;
 
-    $$ = ConstantExpr::get($2, $4, IdxVec, $1->get());
-    if ($$ == 0) ThrowException("constant expression builder returned null!");
+    $$ = ConstantExpr::getGetElementPtr($4, IdxVec);
+    delete $1;
   }
   | Types UnaryOps ConstVal {
     $$ = ConstantExpr::get($2, $3, $1->get());
-    if ($$ == 0) ThrowException("constant expression builder returned null!");
+    delete $1;
   }
   | Types BinaryOps ConstVal ',' ConstVal {
-    $$ = ConstantExpr::get($2, $3, $5, $1->get());
-    if ($$ == 0) ThrowException("constant expression builder returned null!");
+    if ($3->getType() != $5->getType())
+      ThrowException("Binary operator types must match!");
+    if ($1->get() != $3->getType())
+      ThrowException("Return type of binary constant must match arguments!");
+    $$ = ConstantExpr::get($2, $3, $5);
+    delete $1;
   }
   | Types ShiftOps ConstVal ',' ConstVal {
-    $$ = ConstantExpr::get($2, $3, $5, $1->get());
-    if ($$ == 0) ThrowException("constant expression builder returned null!");
+    if ($1->get() != $3->getType())
+      ThrowException("Return type of shift constant must match argument!");
+    if ($5->getType() != Type::UByteTy)
+      ThrowException("Shift count for shift constant must be unsigned byte!");
+    
+    $$ = ConstantExpr::get($2, $3, $5);
+    delete $1;
   }
   ;
 
diff --git a/lib/Bytecode/Writer/ConstantWriter.cpp b/lib/Bytecode/Writer/ConstantWriter.cpp
index 2091ced..5e24018 100644
--- a/lib/Bytecode/Writer/ConstantWriter.cpp
+++ b/lib/Bytecode/Writer/ConstantWriter.cpp
@@ -100,7 +100,8 @@ bool BytecodeWriter::outputConstant(const Constant *CPV) {
   // We must check for a ConstantExpr before switching by type because
   // a ConstantExpr can be of any type, and has no explicit value.
   // 
-  if (const ConstantExpr* CE = dyn_cast<ConstantExpr>(CPV)) {
+  if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(CPV)) {
+    // FIXME: Encoding of constant exprs could be much more compact!
     assert(CE->getNumOperands() > 0 && "ConstantExpr with 0 operands");
     output_vbr(CE->getNumOperands(), Out);   // flags as an expr
     output_vbr(CE->getOpcode(), Out);        // flags as an expr
@@ -113,9 +114,9 @@ bool BytecodeWriter::outputConstant(const Constant *CPV) {
       output_vbr((unsigned)Slot, Out);
     }
     return false;
-  }
-  else
+  } else {
     output_vbr((unsigned)0, Out);       // flag as not a ConstantExpr
+  }
   
   switch (CPV->getType()->getPrimitiveID()) {
   case Type::BoolTyID:    // Boolean Types
diff --git a/lib/VMCore/AsmWriter.cpp b/lib/VMCore/AsmWriter.cpp
index c1af5e4..c12af5b 100644
--- a/lib/VMCore/AsmWriter.cpp
+++ b/lib/VMCore/AsmWriter.cpp
@@ -333,7 +333,7 @@ static void WriteConstantInt(ostream &Out, const Constant *CV, bool PrintName,
       Out << "<pointer reference without context info>";
     }
 
-  } else if (const ConstantExpr *CE=dyn_cast<ConstantExpr>(CV)) {
+  } else if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(CV)) {
     Out << CE->getOpcodeName();
 
     bool isGEP = CE->getOpcode() == Instruction::GetElementPtr;
@@ -343,7 +343,7 @@ static void WriteConstantInt(ostream &Out, const Constant *CV, bool PrintName,
       printTypeInt(Out, (*OI)->getType(), TypeTable);
       WriteAsOperandInternal(Out, *OI, PrintName, TypeTable, Table);
       if (OI+1 != CE->op_end())
-        Out << ", ";    // ((isGEP && OI == CE->op_begin())? " " : ", ");
+        Out << ", ";
     }
     
     if (isGEP)
diff --git a/lib/VMCore/Constants.cpp b/lib/VMCore/Constants.cpp
index 234b647..faef147 100644
--- a/lib/VMCore/Constants.cpp
+++ b/lib/VMCore/Constants.cpp
@@ -138,21 +138,20 @@ ConstantPointerRef::ConstantPointerRef(GlobalValue *GV)
   Operands.push_back(Use(GV, this));
 }
 
-ConstantExpr::ConstantExpr(unsigned opCode, Constant *C,  const Type *Ty)
-  : Constant(Ty), iType(opCode) {
+ConstantExpr::ConstantExpr(unsigned Opcode, Constant *C, const Type *Ty)
+  : Constant(Ty), iType(Opcode) {
   Operands.push_back(Use(C, this));
 }
 
-ConstantExpr::ConstantExpr(unsigned opCode, Constant* C1,
-                           Constant* C2, const Type *Ty)
-  : Constant(Ty), iType(opCode) {
+ConstantExpr::ConstantExpr(unsigned Opcode, Constant *C1, Constant *C2)
+  : Constant(C1->getType()), iType(Opcode) {
   Operands.push_back(Use(C1, this));
   Operands.push_back(Use(C2, this));
 }
 
-ConstantExpr::ConstantExpr(unsigned opCode, Constant* C,
-                          const std::vector<Constant*> &IdxList, const Type *Ty)
-  : Constant(Ty), iType(opCode) {
+ConstantExpr::ConstantExpr(Constant *C, const std::vector<Constant*> &IdxList,
+                           const Type *DestTy)
+  : Constant(DestTy), iType(Instruction::GetElementPtr) {
   Operands.reserve(1+IdxList.size());
   Operands.push_back(Use(C, this));
   for (unsigned i = 0, E = IdxList.size(); i != E; ++i)
@@ -386,7 +385,6 @@ ConstantPointerRef *ConstantPointerRef::get(GlobalValue *GV) {
 }
 
 //---- ConstantExpr::get() implementations...
-// Return NULL on invalid expressions.
 //
 typedef pair<unsigned, vector<Constant*> > ExprMapKeyType;
 static ValueMap<const ExprMapKeyType, ConstantExpr> ExprConstants;
@@ -415,60 +413,51 @@ ConstantExpr *ConstantExpr::get(unsigned Opcode, Constant *C, const Type *Ty) {
   return Result;
 }
 
-ConstantExpr *ConstantExpr::get(unsigned Opcode, Constant *C1, Constant *C2,
-                                const Type *Ty) {
-
+ConstantExpr *ConstantExpr::get(unsigned Opcode, Constant *C1, Constant *C2) {
   // Look up the constant in the table first to ensure uniqueness
   vector<Constant*> argVec(1, C1); argVec.push_back(C2);
   const ExprMapKeyType &Key = make_pair(Opcode, argVec);
-  ConstantExpr *Result = ExprConstants.get(Ty, Key);
+  ConstantExpr *Result = ExprConstants.get(C1->getType(), Key);
   if (Result) return Result;
   
   // Its not in the table so create a new one and put it in the table.
   // Check the operands for consistency first
   assert((Opcode >= Instruction::FirstBinaryOp &&
           Opcode < Instruction::NumBinaryOps) &&
-         "Invalid opcode  in binary constant expression");
+         "Invalid opcode in binary constant expression");
 
-  assert(Ty == C1->getType() && Ty == C2->getType() &&
-         "Operand types in binary constant expression should match result");
+  assert(C1->getType() == C2->getType() &&
+         "Operand types in binary constant expression should match");
   
-  Result = new ConstantExpr(Opcode, C1, C2, Ty);
-  ExprConstants.add(Ty, Key, Result);
+  Result = new ConstantExpr(Opcode, C1, C2);
+  ExprConstants.add(C1->getType(), Key, Result);
   return Result;
 }
 
-ConstantExpr *ConstantExpr::get(unsigned Opcode, Constant *C,
-                                const std::vector<Constant*> &IdxList,
-                                const Type *Ty) {
+ConstantExpr *ConstantExpr::getGetElementPtr(Constant *C,
+                                        const std::vector<Constant*> &IdxList) {
+  const Type *Ty = C->getType();
 
   // Look up the constant in the table first to ensure uniqueness
   vector<Constant*> argVec(1, C);
   argVec.insert(argVec.end(), IdxList.begin(), IdxList.end());
   
-  const ExprMapKeyType &Key = make_pair(Opcode, argVec);
+  const ExprMapKeyType &Key = make_pair(Instruction::GetElementPtr, argVec);
   ConstantExpr *Result = ExprConstants.get(Ty, Key);
   if (Result) return Result;
-  
+
   // Its not in the table so create a new one and put it in the table.
   // Check the operands for consistency first
-  // Must be a getElementPtr.  Check for valid getElementPtr expression.
   // 
-  assert(Opcode == Instruction::GetElementPtr &&
-         "Operator other than GetElementPtr used with an index list");
-
   assert(isa<PointerType>(Ty) &&
          "Non-pointer type for constant GelElementPtr expression");
 
+  // Check that the indices list is valid...
   std::vector<Value*> ValIdxList(IdxList.begin(), IdxList.end());
-  const Type *fldType = GetElementPtrInst::getIndexedType(C->getType(),
-                                                          ValIdxList, true);
-  assert(fldType && "Invalid index list for constant GelElementPtr expression");
-
-  assert(cast<PointerType>(Ty)->getElementType() == fldType &&
-         "Type for constant GelElementPtr expression doesn't match field type");
+  const Type *DestTy = GetElementPtrInst::getIndexedType(Ty, ValIdxList, true);
+  assert(DestTy && "Invalid index list for constant GelElementPtr expression");
   
-  Result = new ConstantExpr(Opcode, C, IdxList, Ty);
+  Result = new ConstantExpr(C, IdxList, PointerType::get(DestTy));
   ExprConstants.add(Ty, Key, Result);
   return Result;
 }
@@ -480,8 +469,8 @@ void ConstantExpr::destroyConstant() {
   destroyConstantImpl();
 }
 
-const char *ConstantExpr::getOpcodeName(unsigned Opcode) {
-  return Instruction::getOpcodeName(Opcode);
+const char *ConstantExpr::getOpcodeName() const {
+  return Instruction::getOpcodeName(getOpcode());
 }