Remove unnecessary fields in top-level parser of Subzero.

src/PNaClTranslator.cpp

 //===- subzero/src/PNaClTranslator.cpp - ICE from bitcode -----------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the PNaCl bitcode file to Ice, to machine code
@@ skipping 143 matching lines @@
 class TopLevelParser : public NaClBitcodeParser {
   TopLevelParser(const TopLevelParser &) = delete;
   TopLevelParser &operator=(const TopLevelParser &) = delete;
 
 public:
   typedef std::vector<Ice::FunctionDeclaration *> FunctionDeclarationListType;
 
   TopLevelParser(Ice::Translator &Translator, NaClBitcodeHeader &Header,
                  NaClBitstreamCursor &Cursor, bool &ErrorStatus)
       : NaClBitcodeParser(Cursor), Translator(Translator), Header(Header),
-        ErrorStatus(ErrorStatus), NumErrors(0), NumFunctionIds(0),
-        NumFunctionBlocks(0), BlockParser(nullptr) {}
+        ErrorStatus(ErrorStatus), NumErrors(0), NextDefiningFunctionID(0),
+        BlockParser(nullptr) {}
 
   ~TopLevelParser() override {}
 
   Ice::Translator &getTranslator() { return Translator; }
 
   void setBlockParser(BlockParserBaseClass *NewBlockParser) {
     BlockParser = NewBlockParser;
   }
 
   // Generates error with given Message. Always returns true.
@@ skipping 44 matching lines @@
   const Ice::FuncSigType &getFuncSigTypeByID(unsigned ID) {
     const ExtendedType *Ty = getTypeByIDAsKind(ID, ExtendedType::FuncSig);
     if (Ty == nullptr)
       // Return error recovery value.
       return UndefinedFuncSigType;
     return cast<FuncSigExtendedType>(Ty)->getSignature();
   }
 
   /// Sets the next function ID to the given LLVM function.
   void setNextFunctionID(Ice::FunctionDeclaration *Fcn) {
-    ++NumFunctionIds;
     FunctionDeclarationList.push_back(Fcn);
   }
 
-  /// Defines the next function ID as one that has an implementation
-  /// (i.e a corresponding function block in the bitcode).
-  void setNextValueIDAsImplementedFunction() {
-    DefiningFunctionDeclarationsList.push_back(FunctionDeclarationList.size());
-  }
-
   /// Returns the value id that should be associated with the the
   /// current function block. Increments internal counters during call
   /// so that it will be in correct position for next function block.
   unsigned getNextFunctionBlockValueID() {
-    if (NumFunctionBlocks >= DefiningFunctionDeclarationsList.size())
+    unsigned NumDeclaredFunctions = FunctionDeclarationList.size();

[Jim Stichnoth, 2015/01/26 23:12:54]

It's probably a good idea to change some of these "unsigned" declarations to
something like size_t (or worse, std::vector::size_type).

[Karl, 2015/01/28 18:59:18]

For this field, and the local here, I will fix it to size_t. However, I am
hesitant to change more deeply since the base class NaClBitcodeParser assumes
ID's (in general) are unsigned (not size_t_. Will consider a separate CL to
consider more general clean ups on size_t and unsigned.

+    while (NextDefiningFunctionID < NumDeclaredFunctions &&
+           FunctionDeclarationList[NextDefiningFunctionID]->isProto())
+      ++NextDefiningFunctionID;
+    if (NextDefiningFunctionID >= NumDeclaredFunctions)
       report_fatal_error(
           "More function blocks than defined function addresses");
-    return DefiningFunctionDeclarationsList[NumFunctionBlocks++];
+    return NextDefiningFunctionID++;
   }
 
   /// Returns the function associated with ID.
   Ice::FunctionDeclaration *getFunctionByID(unsigned ID) {
     if (ID < FunctionDeclarationList.size())
       return FunctionDeclarationList[ID];
     return reportGetFunctionByIDError(ID);
   }
 
   /// Returns the list of function declarations.
@@ skipping 48 matching lines @@
       SuppressMangling = false;
     }
     const Ice::RelocOffsetT Offset = 0;
     C = getTranslator().getContext()->getConstantSym(Offset, Name,
                                                      SuppressMangling);
     ValueIDConstants[ID] = C;
     return C;
   }
 
   /// Returns the number of function declarations in the bitcode file.
-  unsigned getNumFunctionIDs() const { return NumFunctionIds; }
+  unsigned getNumFunctionIDs() const { return FunctionDeclarationList.size(); }
 
   /// Returns the number of global declarations (i.e. IDs) defined in
   /// the bitcode file.
   unsigned getNumGlobalIDs() const {
     return FunctionDeclarationList.size() + VariableDeclarations.size();
   }
 
   /// Creates Count global variable declarations.
   void CreateGlobalVariables(size_t Count) {
     assert(VariableDeclarations.empty());
@@ skipping 11 matching lines @@
   /// Returns the global variable declaration with the given index.
   Ice::VariableDeclaration *getGlobalVariableByID(unsigned Index) {
     if (Index < VariableDeclarations.size())
       return VariableDeclarations[Index];
     return reportGetGlobalVariableByIDError(Index);
   }
 
   /// Returns the global declaration (variable or function) with the
   /// given Index.
   Ice::GlobalDeclaration *getGlobalDeclarationByID(size_t Index) {
+    size_t NumFunctionIds = FunctionDeclarationList.size();
     if (Index < NumFunctionIds)
       return getFunctionByID(Index);
     else
       return getGlobalVariableByID(Index - NumFunctionIds);
   }
 
   /// Returns the list of parsed global variable declarations.
   const Ice::Translator::VariableDeclarationListType &getGlobalVariables() {
     return VariableDeclarations;
   }
 
 private:
   // The translator associated with the parser.
   Ice::Translator &Translator;
   // The bitcode header.
   NaClBitcodeHeader &Header;
   // The exit status that should be set to true if an error occurs.
   bool &ErrorStatus;
   // The number of errors reported.
   unsigned NumErrors;
   // The types associated with each type ID.
   std::vector<ExtendedType> TypeIDValues;
   // The set of functions.
   FunctionDeclarationListType FunctionDeclarationList;
   // The set of global variables.
   Ice::Translator::VariableDeclarationListType VariableDeclarations;
   // Relocatable constants associated with global declarations.
   std::vector<Ice::Constant *> ValueIDConstants;
-  // The number of function declarations (i.e. IDs).
-  unsigned NumFunctionIds;
-  // The number of function blocks (processed so far).
-  unsigned NumFunctionBlocks;
-  // The list of function declaration IDs (in the order found) that
-  // aren't just proto declarations.
-  // TODO(kschimpf): Instead of using this list, just use
-  // FunctionDeclarationList, and the isProto member function.
-  std::vector<unsigned> DefiningFunctionDeclarationsList;
+  // The ID of the next possible defined function ID.
+  unsigned NextDefiningFunctionID;

[jvoung (off chromium), 2015/01/26 22:58:02]

Might be more clear to put this next FunctionDeclarationList and describe the
relationship.

FunctionDeclarationList is filled first. It's the set of functions (either
defined or isproto).

Then function definitions are encountered/parsed and NextDefiningFunctionID
incremented to track the next actually-defined function.

[Karl, 2015/01/28 18:59:18]

Done.

   // Error recovery value to use when getFuncSigTypeByID fails.
   Ice::FuncSigType UndefinedFuncSigType;
   // The block parser currently being applied. Used for error
   // reporting.
   BlockParserBaseClass *BlockParser;
 
   bool ParseBlock(unsigned BlockID) override;
 
   // Gets extended type associated with the given index, assuming the
   // extended type is of the WantedKind. Generates error message if
@@ skipping 2510 matching lines @@
       Error(StrBuf.str());
       return;
     }
     GlobalValue::LinkageTypes Linkage;
     if (!naclbitc::DecodeLinkage(Values[3], Linkage)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Function address has unknown linkage. Found " << Values[3];
       Error(StrBuf.str());
       return;
     }

[jvoung (off chromium), 2015/01/26 22:58:02]

Might be more clear to have a "bool IsProto = Values[2] == 1;" then pass that to
the create().

[Karl, 2015/01/28 18:59:18]

Done.

     Ice::FunctionDeclaration *Func = Ice::FunctionDeclaration::create(
-        Signature, CallingConv, Linkage, Values[2] == 0);
-    if (Values[2] == 0)
-      Context->setNextValueIDAsImplementedFunction();
+        Signature, CallingConv, Linkage, Values[2] == 1);
     Context->setNextFunctionID(Func);
     return;
   }
   default:
     BlockParserBaseClass::ProcessRecord();
     return;
   }
 }
 
 bool TopLevelParser::ParseBlock(unsigned BlockID) {
@@ skipping 59 matching lines @@
 
   if (TopLevelBlocks != 1) {
     errs() << IRFilename
            << ": Contains more than one module. Found: " << TopLevelBlocks
            << "\n";
     ErrorStatus = true;
   }
 }
 
 } // end of namespace Ice