[PATCH] Refactor bitcode reader to simplify control.

[Karl Schimpf]

Fixes based on feedback by Filipe.


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:289
@@ -251,1 +288,3 @@
+  /// \param M the module to build.
 
+  /// \param ShouldMaterializeAll true when the module should be materialized
----------------
filcab wrote:
> Why the empty line?
Removed.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:789
@@ -717,3 +788,3 @@
 namespace {
-  /// @brief A class for maintaining the slot number definition
+  /// \brief A class for maintaining the slot number definition
   /// as a placeholder for the actual definition for forward constants defs.
----------------
filcab wrote:
> Omit \brief.
Done.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:803
@@ -731,3 +802,3 @@
 
-    /// @brief Methods to support type inquiry through isa, cast, and dyn_cast.
+    /// \brief Methods to support type inquiry through isa, cast, and dyn_cast.
     static bool classof(const Value *V) {
----------------
filcab wrote:
> Omit \brief.
Done.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:3254
@@ -3125,2 +3253,3 @@
     case BitstreamEntry::EndBlock:
+      setParseState(AtTopLevel);
       return std::error_code();
----------------
filcab wrote:
> We're already at top level, no? (line 3234)
> I might be missing something, but it looks like we're at top level, and saw an EndBlock. Shouldn't this be an error?
I agree that we should be at top level.

I also agree that it appears weird that we allow extra (unmathced) EndBlocks. This has been allowed by the bitcode reader/writer for years. I just wasn't willing to make the leap that I should remove this.

However, I tried removing it (and making it an error), and no tests failed. Hence, Converting this to an error.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:4676
@@ -4517,2 +4675,3 @@
+    return EC;
 
   // Iterate over the module, deserializing any functions that are still on
----------------
Moved the iterating of functions to before the call to finishParse. This deals with the problems I was having with tests in test/Bitcode/invalid.test (i.e. Inputs/invalid-fwdref-type-mismatch-2.bc and Inputs/invalid-load-ptr-type.bc). These two files had multiple errors (the one they intended which was inside a function body, and the one probably not intended - extraneous stuff at the end of the bitcode file).

This removes the need for the command line flag UseOldLazyBitcodeParse, and I have deleted it.

http://reviews.llvm.org/D8786

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/