Additional clean ups on errors in bitcode parsing.

include/llvm/Bitcode/NaCl/NaClObjDumpStream.h

Index: include/llvm/Bitcode/NaCl/NaClObjDumpStream.h
diff --git a/include/llvm/Bitcode/NaCl/NaClObjDumpStream.h b/include/llvm/Bitcode/NaCl/NaClObjDumpStream.h
index 8ff6460b6c67f70521e668def6f89e21b6819c1e..6356efd664e5819d7b54b44bddfc0a9160b7ba84 100644
--- a/include/llvm/Bitcode/NaCl/NaClObjDumpStream.h
+++ b/include/llvm/Bitcode/NaCl/NaClObjDumpStream.h
@@ -672,9 +672,9 @@ class RecordTextFormatter : public TextFormatter {
   RecordTextFormatter(const RecordTextFormatter&) = delete;
   RecordTextFormatter &operator=(const RecordTextFormatter&) = delete;
 public:
-  /// The address write width used to print the number of
-  /// bytes in the record bit address, when printing records.
-  static const unsigned AddressWriteWidth = 8;
+  /// The address write width used to print a bit address, when
+  /// printing records.
+  static const unsigned AddressWriteWidth = 10;
 
   explicit RecordTextFormatter(ObjDumpStream *ObjDump);
 
@@ -708,6 +708,9 @@ private:
   StartClusteringDirective StartCluster;
   // End clustering tokens.
   FinishClusteringDirective FinishCluster;
+
+  // Generates an address label with padding to match AddressWriteWidth;
+  std::string getBitAddress(uint64_t Bit);
 };
 
 /// Implements a stream to print out bitcode records, assembly code,
@@ -806,37 +809,47 @@ public:
   /// then returns the comments stream. In general, warnings will be
   /// printed after the next record, unless a call to Flush is made.
   raw_ostream &Warning() {
-    return Warning(LastKnownBit);
+    return WarningAt(LastKnownBit);
   }
 
   /// Prints "Warning(Bit/8:Bit%8): " onto the comments stream, and
   /// then returns the comments stream. In general, warnings will be
   /// printed after the next record, unless a call to Flush is made.
-  raw_ostream &Warning(uint64_t Bit) {
+  raw_ostream &WarningAt(uint64_t Bit) {
     LastKnownBit = Bit;
-    return PrintMessagePrefix("Warning", Bit);
+    return naclbitc::ErrorAt(Comments(), naclbitc::Warning, Bit);
   }
 
-  /// Prints "Error(Bit/8:Bit%8): " onto the comments stream, records
-  /// that an error has occurred, and then returns the comments
-  /// stream. In general errors will be printed after the next record,
-  /// unless a call to Flush is made.
+  /// Prints "Error(Bit/8:Bit%8): " onto the comments stream using the
+  /// last know bit position of the input. Then, it records that an
+  /// error has occurred and returns the comments stream. In general
+  /// errors will be printed after the next record, unless a call to
+  /// Flush is made.
   raw_ostream &Error() {
-    return Error(LastKnownBit);
+    return ErrorAt(LastKnownBit);
+  }
+
+  /// Prints "Error(Bit/8:Bit%8): " onto the comments stream at the
+  /// given Bit position. Then, it records that an error has occurred
+  /// and returns the comments stream. In general errors will be
+  /// printed after the next record, unless a call to Flush is made.
+  raw_ostream &ErrorAt(uint64_t Bit) {
+    return ErrorAt(naclbitc::Error, Bit);
   }
 
-  /// Prints "Error(Bit/8:Bit%8): " onto the comments stream, records
-  /// that an error has occurred, and then returns the comments
-  /// stream. In general errors will be printed after the next record,
-  /// unless a call to Flush is made.
-  raw_ostream &Error(uint64_t Bit);
+  /// Prints "Level(Bit/8:Bit%8): " onto the comments stream at the
+  /// given bit position and severity Level.  Then, it records that an
+  /// error has occurred and then returns the comments stream. In
+  /// general errors will be printed after the next record, unless a
+  /// call to Flush is made.
+  raw_ostream &ErrorAt(naclbitc::ErrorLevel Level, uint64_t Bit);
 
   /// Write a fatal error message to the dump stream, and then
   /// stop the executable. If any assembly, comments, or errors have
   /// been buffered, they will be printed first.
   LLVM_ATTRIBUTE_NORETURN
   void Fatal(const std::string &Message) {

[jvoung (off chromium), 2015/03/07 00:04:35]

Is this method still used?

[Karl, 2015/03/09 18:10:54]

Removed.

-    Fatal(LastKnownBit, Message);
+    FatalAt(LastKnownBit, Message);
   }
 
   /// Write a fatal error message to the dump stream, and then
@@ -844,15 +857,15 @@ public:
   /// been buffered, they will be printed first. Associates fatal error
   /// Message with the given Bit.
   LLVM_ATTRIBUTE_NORETURN
-  void Fatal(uint64_t Bit, const std::string &Message);
+  void FatalAt(uint64_t Bit, const std::string &Message);
 
   /// Write a fatal error message to the dump stream, and then
   /// stop the executable. If any assembly, comments, or errors have
   /// been buffered, they will be printed first, along with the given record.
   LLVM_ATTRIBUTE_NORETURN
-  void Fatal(uint64_t Bit,
-             const llvm::NaClBitcodeRecordData &Record,
-             const std::string &Message);
+  void FatalAt(uint64_t Bit,
+               const llvm::NaClBitcodeRecordData &Record,
+               const std::string &Message);
 
   /// Dumps a record (at the given bit), along with all buffered assembly,
   /// comments, and errors, into the objdump stream.
@@ -920,8 +933,9 @@ public:
   }
 
   /// Changes the internal state, to assume one is processing a record
-  /// at the given bit. Used by Error() and Fatal(Message) to fill in
-  /// the corresponding bit to associate with the error message.
+  /// at the given bit. Used by Warning(), Error(), and Fatal(Message)
+  /// to fill in the corresponding bit to associate with the error

[jvoung (off chromium), 2015/03/06 22:30:27]

On second read, I'm not sure you need to call out all the callers/users of this.
It seems reasonable for this just to be "Changes the internal state, to assume
one is processing a record at the given bit.". Then you don't need to update the
comments based on new usage.

WDYT?

[Karl, 2015/03/06 22:54:20]

Sounds good. Removing sentence.

+  /// message.
   void SetRecordBitAddress(uint64_t Bit) {
     LastKnownBit = Bit;
   }
@@ -965,12 +979,6 @@ private:
     AssemblyBuffer.clear();
     MessageBuffer.clear();
   }
-
-  // Returns the message stream with 'Label(Bit/8:Bit%8): '.
-  raw_ostream &PrintMessagePrefix(const char *Label, uint64_t Bit) {
-    return Comments() << Label << "(" << NaClBitstreamReader::getBitAddress(Bit)
-                      << "): ";
-  }
 };
 
 }