Additional clean ups on errors in bitcode parsing.

include/llvm/Bitcode/NaCl/NaClObjDumpStream.h

 //===- NaClObjDumpStream.h --------------------------------------*- C++ -*-===//
 //     Models an objdump stream (bitcode records/assembly code).
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
@@ skipping 654 matching lines @@
 
 /// Models that an abbreviation index is not specified when dumping a
 /// bitcode record.
 static int32_t ABBREV_INDEX_NOT_SPECIFIED = -1;
 
 /// The formatter used for dumping records in ObjDumpStream.
 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);
 
   ~RecordTextFormatter() override {}
 
   /// Writes out the given record of values as an instruction.
   void WriteValues(uint64_t Bit,
                    const llvm::NaClBitcodeValues &Values,
                    int32_t AbbrevIndex = ABBREV_INDEX_NOT_SPECIFIED);
 
@@ skipping 13 matching lines @@
   // A comma between elements in a record.
   TokenTextDirective Comma;
   // A space inside a record.
   SpaceTextDirective Space;
   // End the line.
   EndlineTextDirective Endline;
   // Start clustering tokens.
   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,
 /// comments, and errors. The general format is to print records and
 /// assembly side by side. Comments and errors (associated with the
 /// record and/or assembly code) follow each printed record.
 ///
 /// Alignment of records, assembly, comments, and errors is done by
 /// buffering assembly, comments, and errors until a write or flush
 /// occurs.  Then, the various streams are stitched together to
@@ skipping 78 matching lines @@
   /// Returns stream to buffer messages that will be printed during the
   // next write call.
   raw_ostream &Comments() {
     return MessageStream;
   }
 
   /// 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() {
-    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, 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 "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 "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
   /// stop the executable. If any assembly, comments, or errors have
   /// 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.
   void Write(uint64_t Bit,
              const llvm::NaClBitcodeRecordData &Record,
              int32_t AbbrevIndex = ABBREV_INDEX_NOT_SPECIFIED) {
     LastKnownBit = Bit;
     NaClBitcodeValues Values(Record);
     RecordFormatter.WriteValues(Bit, Values, AbbrevIndex);
     Flush();
@@ skipping 47 matching lines @@
   unsigned GetRecordWidth() const {
     return RecordWidth;
   }
 
   /// Changes the column separator character to the given value.
   void SetColumnSeparator(char Separator) {
     ColumnSeparator = Separator;
   }
 
   /// 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;
   }
 
 private:
   // The stream to dump to.
   raw_ostream &Stream;
   // True if records should be dumped to the dump stream.
   bool DumpRecords;
   // True if assembly text should be dumped to the dump stream.
@@ skipping 23 matching lines @@
   raw_string_ostream RecordStream;
   /// The text formatter for generating records.
   RecordTextFormatter RecordFormatter;
 
   // Resets assembly and buffers.
   void ResetBuffers() {
     RecordBuffer.clear();
     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)
-                      << "): ";
-  }
 };
 
 }
 }
 
 #endif