Various cleanups:

- Remove tabs
- Standardize use of space around ( and ).
- Consolidate the ConstantPlaceHolder class
- Rename two methods to be more meaningful (ParseType, ParseTypes)
- Correct indentation of blocks
- Add documentation
- Convert input dependent asserts to error(...) so it throws instead.
Provide placeholder implementations of read_float and read_double that
still read in platform-specific endianess. When I figure out how to do
this without knowing the endianess of the platform, it will get implemented
correctly.


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

lib/Bytecode/Reader/Reader.cpp

@@ -29,24 +29,22 @@
 
 using namespace llvm;
 
+namespace {
+
 /// @brief A class for maintaining the slot number definition
-/// as a placeholder for the actual definition.
+/// as a placeholder for the actual definition for forward constants defs.
-template<class SuperType>
+class ConstantPlaceHolder : public ConstantExpr {
-class PlaceholderDef : public SuperType {
   unsigned ID;
-  PlaceholderDef();                       // DO NOT IMPLEMENT
+  ConstantPlaceHolder();                       // DO NOT IMPLEMENT
-  void operator=(const PlaceholderDef &); // DO NOT IMPLEMENT
+  void operator=(const ConstantPlaceHolder &); // DO NOT IMPLEMENT
 public:
-  PlaceholderDef(const Type *Ty, unsigned id) : SuperType(Ty), ID(id) {}
+  ConstantPlaceHolder(const Type *Ty, unsigned id) 
+    : ConstantExpr(Instruction::UserOp1, Constant::getNullValue(Ty), Ty),
+    ID(id) {}
   unsigned getID() { return ID; }
 };
 
-struct ConstantPlaceHolderHelper : public ConstantExpr {
+}
-  ConstantPlaceHolderHelper(const Type *Ty)
-    : ConstantExpr(Instruction::UserOp1, Constant::getNullValue(Ty), Ty) {}
-};
-
-typedef PlaceholderDef<ConstantPlaceHolderHelper>  ConstPHolder;
 
 // Provide some details on error
 inline void BytecodeReader::error(std::string err) {
@@ -156,6 +154,22 @@ inline void BytecodeReader::read_data(void *Ptr, void *End) {
   At += Amount;
 }
 
+/// Read a float value in little-endian order
+inline void BytecodeReader::read_float(float& FloatVal) {
+  /// FIXME: This is a broken implementation! It reads
+  /// it in a platform-specific endianess. Need to make
+  /// it little endian always.
+  read_data(&FloatVal, &FloatVal+1);
+}
+
+/// Read a double value in little-endian order
+inline void BytecodeReader::read_double(double& DoubleVal) {
+  /// FIXME: This is a broken implementation! It reads
+  /// it in a platform-specific endianess. Need to make
+  /// it little endian always.
+  read_data(&DoubleVal, &DoubleVal+1);
+}
+
 /// Read a block header and obtain its type and size
 inline void BytecodeReader::read_block(unsigned &Type, unsigned &Size) {
   Type = read_uint();
@@ -427,7 +441,7 @@ Constant* BytecodeReader::getConstantValue(unsigned TypeSlot, unsigned Slot) {
   } else {
     // Create a placeholder for the constant reference and
     // keep track of the fact that we have a forward ref to recycle it
-    Constant *C = new ConstPHolder(Ty, Slot);
+    Constant *C = new ConstantPlaceHolder(Ty, Slot);
     
     // Keep track of the fact that we have a forward ref to recycle it
     ConstantFwdRefs.insert(I, std::make_pair(Key, C));
@@ -442,8 +456,8 @@ Constant* BytecodeReader::getConstantValue(unsigned TypeSlot, unsigned Slot) {
 /// As values are created, they are inserted into the appropriate place
 /// with this method. The ValueTable argument must be one of ModuleValues
 /// or FunctionValues data members of this class.
-unsigned BytecodeReader::insertValue(
+unsigned BytecodeReader::insertValue(Value *Val, unsigned type, 
-    Value *Val, unsigned type, ValueTable &ValueTab) {
+                                      ValueTable &ValueTab) {
   assert((!isa<Constant>(Val) || !cast<Constant>(Val)->isNullValue()) ||
           !hasImplicitNull(type) &&
          "Cannot read null values from bytecode!");
@@ -549,7 +563,7 @@ void BytecodeReader::ParseInstruction(std::vector<unsigned> &Oprnds,
 
   const Type *InstTy = getSanitizedType(iType);
 
-  // Hae enough to inform the handler now
+  // We have enough info to inform the handler now.
   if (Handler) Handler->handleInstruction(Opcode, InstTy, Oprnds, At-SaveAt);
 
   // Declare the resulting instruction we'll build.
@@ -999,26 +1013,38 @@ void BytecodeReader::ParseCompactionTypes( unsigned NumEntries ) {
 /// Parse a compaction table.
 void BytecodeReader::ParseCompactionTable() {
 
+  // Notify handler that we're beginning a compaction table.
   if (Handler) Handler->handleCompactionTableBegin();
 
-  /// In LLVM 1.3 Type no longer derives from Value. So, 
+  // In LLVM 1.3 Type no longer derives from Value. So, 
-  /// we always write them first in the compaction table
+  // we always write them first in the compaction table
-  /// because they can't occupy a "type plane" where the
+  // because they can't occupy a "type plane" where the
-  /// Values reside.
+  // Values reside.
   if (! hasTypeDerivedFromValue) {
     unsigned NumEntries = read_vbr_uint();
     ParseCompactionTypes(NumEntries);
   }
 
+  // Compaction tables live in separate blocks so we have to loop
+  // until we've read the whole thing.
   while (moreInBlock()) {
+    // Read the number of Value* entries in the compaction table
     unsigned NumEntries = read_vbr_uint();
     unsigned Ty = 0;
     unsigned isTypeType = false;
 
+    // Decode the type from value read in. Most compaction table
+    // planes will have one or two entries in them. If that's the
+    // case then the length is encoded in the bottom two bits and
+    // the higher bits encode the type. This saves another VBR value.
     if ((NumEntries & 3) == 3) {
+      // In this case, both low-order bits are set (value 3). This
+      // is a signal that the typeid follows.
       NumEntries >>= 2;
       isTypeType = read_typeid(Ty);
     } else {
+      // In this case, the low-order bits specify the number of entries
+      // and the high order bits specify the type.
       Ty = NumEntries >> 2;
       isTypeType = sanitizeTypeId(Ty);
       NumEntries &= 3;
@@ -1029,17 +1055,25 @@ void BytecodeReader::ParseCompactionTable() {
     if (isTypeType) {
       ParseCompactionTypes(NumEntries);
     } else {
+      // Make sure we have enough room  for the plane 
       if (Ty >= CompactionValues.size())
         CompactionValues.resize(Ty+1);
 
+      // Make sure the plane is empty or we have some kind of error
       if (!CompactionValues[Ty].empty())
         error("Compaction table plane contains multiple entries!");
 
+      // Notify handler about the plane
       if (Handler) Handler->handleCompactionTablePlane(Ty, NumEntries);
 
+      // Convert the type slot to a type
       const Type *Typ = getType(Ty);
+
       // Push the implicit zero
       CompactionValues[Ty].push_back(Constant::getNullValue(Typ));
+
+      // Read in each of the entries, put them in the compaction table
+      // and notify the handler that we have a new compaction table value.
       for (unsigned i = 0; i != NumEntries; ++i) {
         unsigned ValSlot = read_vbr_uint();
         Value *V = getGlobalTableValue(Typ, ValSlot);
@@ -1048,11 +1082,15 @@ void BytecodeReader::ParseCompactionTable() {
       }
     }
   }
+  // Notify handler that the compaction table is done.
   if (Handler) Handler->handleCompactionTableEnd();
 }
     
-// Parse a single type constant.
+// Parse a single type. The typeid is read in first. If its a primitive type
-const Type *BytecodeReader::ParseTypeConstant() {
+// then nothing else needs to be read, we know how to instantiate it. If its
+// a derived type, then additional data is read to fill out the type 
+// definition.
+const Type *BytecodeReader::ParseType() {
   unsigned PrimType = 0;
   if (read_typeid(PrimType))
     error("Invalid type (type type) in type constants!");
@@ -1116,7 +1154,7 @@ const Type *BytecodeReader::ParseTypeConstant() {
   return Result;
 }
 
-// ParseTypeConstants - We have to use this weird code to handle recursive
+// ParseType - We have to use this weird code to handle recursive
 // types.  We know that recursive types will only reference the current slab of
 // values in the type plane, but they can forward reference types before they
 // have been read.  For example, Type #0 might be '{ Ty#1 }' and Type #1 might
@@ -1126,7 +1164,7 @@ const Type *BytecodeReader::ParseTypeConstant() {
 // something and when we reread the type later, we can replace the opaque type
 // with a new resolved concrete type.
 //
-void BytecodeReader::ParseTypeConstants(TypeListTy &Tab, unsigned NumEntries){
+void BytecodeReader::ParseTypes(TypeListTy &Tab, unsigned NumEntries){
   assert(Tab.size() == 0 && "should not have read type constants in before!");
 
   // Insert a bunch of opaque types to be resolved later...
@@ -1138,7 +1176,7 @@ void BytecodeReader::ParseTypeConstants(TypeListTy &Tab, unsigned NumEntries){
   // opaque types just inserted.
   //
   for (unsigned i = 0; i != NumEntries; ++i) {
-    const Type* NewTy = ParseTypeConstant();
+    const Type* NewTy = ParseType();
     const Type* OldTy = Tab[i].get();
     if (NewTy == 0) 
       error("Couldn't parse type!");
@@ -1185,7 +1223,9 @@ Constant *BytecodeReader::ParseConstantValue( unsigned TypeID) {
     
     // Construct a ConstantExpr of the appropriate kind
     if (isExprNumArgs == 1) {           // All one-operand expressions
-      assert(Opcode == Instruction::Cast);
+      if (Opcode != Instruction::Cast)
+        error("Only Cast instruction has one argument for ConstantExpr");
+
       Constant* Result = ConstantExpr::getCast(ArgVec[0], getType(TypeID));
       if (Handler) Handler->handleConstantExpression(Opcode, ArgVec, Result);
       return Result;
@@ -1209,7 +1249,8 @@ Constant *BytecodeReader::ParseConstantValue( unsigned TypeID) {
       if (Handler) Handler->handleConstantExpression(Opcode, ArgVec, Result);
       return Result;
     } else if (Opcode == Instruction::Select) {
-      assert(ArgVec.size() == 3);
+      if (ArgVec.size() != 3)
+        error("Select instruction must have three arguments.");
       Constant* Result = ConstantExpr::getSelect(ArgVec[0], ArgVec[1], 
                                                  ArgVec[2]);
       if (Handler) Handler->handleConstantExpression(Opcode, ArgVec, Result);
@@ -1263,16 +1304,16 @@ Constant *BytecodeReader::ParseConstantValue( unsigned TypeID) {
   }
 
   case Type::FloatTyID: {
-    float F;
+    float Val;
-    read_data(&F, &F+1);
+    read_float(Val);
-    Constant* Result = ConstantFP::get(Ty, F);
+    Constant* Result = ConstantFP::get(Ty, Val);
     if (Handler) Handler->handleConstantValue(Result);
     return Result;
   }
 
   case Type::DoubleTyID: {
     double Val;
-    read_data(&Val, &Val+1);
+    read_double(Val);
     Constant* Result = ConstantFP::get(Ty, Val);
     if (Handler) Handler->handleConstantValue(Result);
     return Result;
@@ -1394,7 +1435,7 @@ void BytecodeReader::ParseConstantPool(ValueTable &Tab,
   /// first in the constant pool.
   if (isFunction && !hasTypeDerivedFromValue) {
     unsigned NumEntries = read_vbr_uint();
-    ParseTypeConstants(TypeTab, NumEntries);
+    ParseTypes(TypeTab, NumEntries);
   }
 
   while (moreInBlock()) {
@@ -1406,7 +1447,7 @@ void BytecodeReader::ParseConstantPool(ValueTable &Tab,
     /// bytecode file in the "Type Type" plane (#12).
     /// In 1.3 plane 12 is now the label plane.  Handle this here.
     if (isTypeType) {
-      ParseTypeConstants(TypeTab, NumEntries);
+      ParseTypes(TypeTab, NumEntries);
     } else if (Typ == Type::VoidTyID) {
       /// Use of Type::VoidTyID is a misnomer. It actually means
       /// that the following plane is constant strings
@@ -1652,7 +1693,7 @@ void BytecodeReader::ParseGlobalTypes() {
   if (hasTypeDerivedFromValue)
     read_vbr_uint();
 
-  ParseTypeConstants(ModuleTypes, NumEntries);
+  ParseTypes(ModuleTypes, NumEntries);
 }
 
 /// Parse the Global info (types, global vars, constants)
@@ -1908,8 +1949,7 @@ void BytecodeReader::ParseModule() {
 
 /// This function completely parses a bytecode buffer given by the \p Buf
 /// and \p Length parameters.
-void BytecodeReader::ParseBytecode(
+void BytecodeReader::ParseBytecode(BufPtr Buf, unsigned Length, 
-       BufPtr Buf, unsigned Length,
                                    const std::string &ModuleID,
                                    bool processFunctions) {