Remove unnecessary fields in top-level parser of Subzero.

src/PNaClTranslator.cpp

Index: src/PNaClTranslator.cpp
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index dfe93ff99957601bf4c5c5366dd14e335ba79d92..26afa326bbd177373c7acda900682223d9b21733 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -161,8 +161,8 @@ public:
   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 {}
 
@@ -227,24 +227,21 @@ public:
 
   /// 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.
@@ -313,7 +310,7 @@ public:
   }
 
   /// 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.
@@ -345,6 +342,7 @@ public:
   /// 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
@@ -373,15 +371,8 @@ private:
   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
@@ -2914,9 +2905,7 @@ void ModuleParser::ProcessRecord() {
       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;
   }