[DWARF] Refactor callback usage for .debug_line error handling

[jhenderson]

Summary

As suggested by @JDevlieghere, I have switched the callback that takes a string in the debug line error handling, with one that takes an Error. This allows using the same default function for both recoverable and unrecoverable errors. Further, to more clearly define which is which, I have renamed the two callbacks.

There are also some other minor tidy-ups/simplifications in relation to this change.

[JDevlieghere]

Thanks James, this LGTM!

[jhenderson]

Fixed a build breakage (I forgot to update DwarfLinker.cpp). Also silenced a warning produced by GCC 4.8 and fixed a bug in the unit tests (the recordRecoverable function was using joinErrors on the wrong input).

[jhenderson]

Hi all. I wanted to get feedback on one of the changes I had to make in order to not get a warning from at least GCC 4.8.4 (the version I use to build with on Linux). Specifically, I've added a createError overload that only takes a char const *, for messages that require no additional formatting. Specifically, this is used by the only recoverable error. Prior to this, I was getting (in my opinion spurious) warnings regarding the format not being a string literal and no arguments being provided from the depths of Format.h (something to do with the way snprintf is used within the format object). I'm not sure what was wrong with my usage of format in formatErrorString when the variadic template list is empty, but this was the easiest way I could come up with to resolve it. Other suggestions are more than welcome!

[JDevlieghere]

In D46831#1100957, @jhenderson wrote:

Hi all. I wanted to get feedback on one of the changes I had to make in order to not get a warning from at least GCC 4.8.4 (the version I use to build with on Linux). Specifically, I've added a createError overload that only takes a char const *, for messages that require no additional formatting. Specifically, this is used by the only recoverable error. Prior to this, I was getting (in my opinion spurious) warnings regarding the format not being a string literal and no arguments being provided from the depths of Format.h (something to do with the way snprintf is used within the format object). I'm not sure what was wrong with my usage of format in formatErrorString when the variadic template list is empty, but this was the easiest way I could come up with to resolve it. Other suggestions are more than welcome!

Sounds reasonable to me.

llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFContext.h

public:
   /// Get a reference to the parsed accelerator table object.
   const AppleAcceleratorTable &getAppleObjC();
 
   /// Get a pointer to a parsed line table corresponding to a compile unit.
   /// Report any parsing issues as warnings on stderr.
   const DWARFDebugLine::LineTable *getLineTableForUnit(DWARFUnit *U);
 
   /// Get a pointer to a parsed line table corresponding to a compile unit.
-  /// Report any parsing warnings using the callback.
+  /// Report any recoverable parsing problems using the callback.
   Expected<const DWARFDebugLine::LineTable *>
   getLineTableForUnit(DWARFUnit *U,
-                      std::function<void(StringRef)> WarnCallback);
+                      std::function<void(Error)> RecoverableErrorCallback);
 
   DataExtractor getStringExtractor() const {
     return DataExtractor(DObj->getStringSection(), false, 0);
   }
   DataExtractor getLineStringExtractor() const {
     return DataExtractor(DObj->getLineStringSection(), false, 0);
   }
 

llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h

bool getFileLineInfoForAddress(uint64_t Address, const char *CompDir,
                                    DILineInfo &Result) const;
 
     void dump(raw_ostream &OS, DIDumpOptions DumpOptions) const;
     void clear();
 
     /// Parse prologue and all rows.
     Error parse(DWARFDataExtractor &DebugLineData, uint32_t *OffsetPtr,
                 const DWARFContext &Ctx, const DWARFUnit *U,
-                std::function<void(StringRef)> WarnCallback = warn,
+                std::function<void(Error)> RecoverableErrorCallback = warn,
                 raw_ostream *OS = nullptr);
 
     using RowVector = std::vector<Row>;
     using RowIter = RowVector::const_iterator;
     using SequenceVector = std::vector<Sequence>;
     using SequenceIter = SequenceVector::const_iterator;
 
     struct Prologue Prologue;
     RowVector Rows;
     SequenceVector Sequences;
 
   private:
     uint32_t findRowInSeq(const DWARFDebugLine::Sequence &Seq,
                           uint64_t Address) const;
     Optional<StringRef>
     getSourceByIndex(uint64_t FileIndex,
                      DILineInfoSpecifier::FileLineInfoKind Kind) const;
   };
 
   const LineTable *getLineTable(uint32_t Offset) const;
-  Expected<const LineTable *>
-  getOrParseLineTable(DWARFDataExtractor &DebugLineData, uint32_t Offset,
-                      const DWARFContext &Ctx, const DWARFUnit *U,
-                      std::function<void(StringRef)> WarnCallback = warn);
+  Expected<const LineTable *> getOrParseLineTable(
+      DWARFDataExtractor &DebugLineData, uint32_t Offset,
+      std::function<void(Error)> RecoverableErrorCallback = warn);
 
   /// Helper to allow for parsing of an entire .debug_line section in sequence.
   class SectionParser {
   public:
     using cu_range = DWARFUnitSection<DWARFCompileUnit>::iterator_range;
     using tu_range =
         iterator_range<std::deque<DWARFUnitSection<DWARFTypeUnit>>::iterator>;
     using LineToUnitMap = std::map<uint64_t, DWARFUnit *>;
 
     SectionParser(DWARFDataExtractor &Data, const DWARFContext &C, cu_range CUs,
                   tu_range TUs);
 
     /// Get the next line table from the section. Report any issues via the
     /// callbacks.
     ///
-    /// \param StringCallback - any issues that don't indicate that the line
-    /// table is invalid are reported using this function.
-    /// \param ErrorCallback - any issues that mean that the line table is
-    /// invalid are reported using this callback.
+    /// \param RecoverableErrorCallback - any issues that don't prevent further
+    /// parsing of the table will be reported through this callback.
+    /// \param UnrecoverableErrorCallback - any issues that prevent further
+    /// parsing of the table will be reported through this callback.
     /// \param OS - if not null, the parser will print information about the
     /// table as it parses it.
-    LineTable parseNext(
-        function_ref<void(StringRef)> StringCallback = warn,
-        function_ref<void(Error)> ErrorCallback = warnForError,
+    LineTable
+    parseNext(function_ref<void(Error)> RecoverableErrorCallback = warn,
+              function_ref<void(Error)> UnrecoverableErrorCallback = warn,
               raw_ostream *OS = nullptr);
 
     /// Skip the current line table and go to the following line table (if
     /// present) immediately.
     ///
     /// \param ErrorCallback - report any prologue parsing issues via this
     /// callback.
-    void skip(function_ref<void(Error)> ErrorCallback = warnForError);
+    void skip(function_ref<void(Error)> ErrorCallback = warn);
 
     /// Indicates if the parser has parsed as much as possible.
     ///
     /// \note Certain problems with the line table structure might mean that
     /// parsing stops before the end of the section is reached.
     bool done() const { return Done; }
 
     /// Get the offset the parser has reached.
     uint32_t getOffset() const { return Offset; }
 
   private:
     DWARFUnit *prepareToParse(uint32_t Offset);
     void moveToNextTable(uint32_t OldOffset, const Prologue &P);
 
     LineToUnitMap LineToUnit;
 
     DWARFDataExtractor &DebugLineData;
     const DWARFContext &Context;
     uint32_t Offset = 0;
     bool Done = false;
   };
 
-  /// Helper function for DWARFDebugLine parse functions, to report issues that
-  /// don't prevent parsing the remainder of the table as warnings.
-  ///
-  /// \param Message The message to report.
-  static void warn(StringRef Message);
-  
-  /// Helper function for DWARFDebugLine parse functions, to report issues that
-  /// prevent parsing the remainder of the table as warnings.
+  /// Helper function for DWARFDebugLine parse functions, to report issues
+  /// identified during parsing.
   ///
   /// \param Err The Error to report.
-  static void warnForError(Error Err);
+  static void warn(Error Err);
 
 private:
   struct ParsingState {
     ParsingState(struct LineTable *LT);
 
     void resetRowAndSequence();
     void appendRowToMatrix(uint32_t Offset);
 

llvm/trunk/lib/DebugInfo/DWARF/DWARFContext.cpp

auto DumpLineSection = [&](DWARFDebugLine::SectionParser Parser,
                              DIDumpOptions DumpOpts) {
     while (!Parser.done()) {
       if (DumpOffset && Parser.getOffset() != *DumpOffset) {
         Parser.skip();
         continue;
       }
       OS << "debug_line[" << format("0x%8.8x", Parser.getOffset()) << "]\n";
       if (DumpOpts.Verbose) {
-        Parser.parseNext(DWARFDebugLine::warn, DWARFDebugLine::warnForError,
-                         &OS);
+        Parser.parseNext(DWARFDebugLine::warn, DWARFDebugLine::warn, &OS);
       } else {
         DWARFDebugLine::LineTable LineTable = Parser.parseNext();
         LineTable.dump(OS, DumpOpts);
       }
     }
   };
 
   if (shouldDump(Explicit, ".debug_line", DIDT_ID_DebugLine,
return getAccelTable(AppleObjC, *DObj, DObj->getAppleObjCSection(),
                        DObj->getStringSection(), isLittleEndian());
 }
 
 const DWARFDebugLine::LineTable *
 DWARFContext::getLineTableForUnit(DWARFUnit *U) {
   Expected<const DWARFDebugLine::LineTable *> ExpectedLineTable =
       getLineTableForUnit(U, DWARFDebugLine::warn);
   if (!ExpectedLineTable) {
-    DWARFDebugLine::warnForError(ExpectedLineTable.takeError());
+    DWARFDebugLine::warn(ExpectedLineTable.takeError());
     return nullptr;
   }
   return *ExpectedLineTable;
 }
 
-Expected<const DWARFDebugLine::LineTable *>
-DWARFContext::getLineTableForUnit(DWARFUnit *U,
-                                  std::function<void(StringRef)> WarnCallback) {
+Expected<const DWARFDebugLine::LineTable *> DWARFContext::getLineTableForUnit(
+    DWARFUnit *U, std::function<void(Error)> RecoverableErrorCallback) {
   if (!Line)
     Line.reset(new DWARFDebugLine);
 
   auto UnitDIE = U->getUnitDIE();
   if (!UnitDIE)
     return nullptr;
 
   auto Offset = toSectionOffset(UnitDIE.find(DW_AT_stmt_list));
   if (!Offset)
     return nullptr; // No line table for this compile unit.
 
   uint32_t stmtOffset = *Offset + U->getLineTableOffset();
   // See if the line table is cached.
   if (const DWARFLineTable *lt = Line->getLineTable(stmtOffset))
     return lt;
 
   // Make sure the offset is good before we try to parse.
   if (stmtOffset >= U->getLineSection().Data.size())
     return nullptr;
 
   // We have to parse it first.
   DWARFDataExtractor lineData(*DObj, U->getLineSection(), isLittleEndian(),
                               U->getAddressByteSize());
   return Line->getOrParseLineTable(lineData, stmtOffset, *this, U,
-                                   WarnCallback);
+                                   RecoverableErrorCallback);
 }
 
 void DWARFContext::parseCompileUnits() {
   CUs.parse(*this, DObj->getInfoSection());
 }
 
 void DWARFContext::parseTypeUnits() {
   if (!TUs.empty())

llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

 }
 
 template <typename... Ts>
 static Error createError(char const *Fmt, const Ts &... Vals) {
   return make_error<StringError>(formatErrorString(Fmt, Vals...),
                                  inconvertibleErrorCode());
 }
 
+static Error createError(char const *Msg) {
+  return make_error<StringError>(Msg, inconvertibleErrorCode());
+}
+
 Error DWARFDebugLine::Prologue::parse(const DWARFDataExtractor &DebugLineData,
                                       uint32_t *OffsetPtr,
                                       const DWARFContext &Ctx,
                                       const DWARFUnit *U) {
   const uint64_t PrologueOffset = *OffsetPtr;
 
   clear();
   TotalLength = DebugLineData.getU32(OffsetPtr);
DWARFDebugLine::getLineTable(uint32_t Offset) const {
   LineTableConstIter Pos = LineTableMap.find(Offset);
   if (Pos != LineTableMap.end())
     return &Pos->second;
   return nullptr;
 }
 
 Expected<const DWARFDebugLine::LineTable *> DWARFDebugLine::getOrParseLineTable(
     DWARFDataExtractor &DebugLineData, uint32_t Offset, const DWARFContext &Ctx,
-    const DWARFUnit *U, std::function<void(StringRef)> WarnCallback) {
+    const DWARFUnit *U, std::function<void(Error)> RecoverableErrorCallback) {
   if (!DebugLineData.isValidOffset(Offset))
     return createError("offset 0x%8.8" PRIx32
                        " is not a valid debug line section offset",
                        Offset);
 
   std::pair<LineTableIter, bool> Pos =
       LineTableMap.insert(LineTableMapTy::value_type(Offset, LineTable()));
   LineTable *LT = &Pos.first->second;
   if (Pos.second) {
-    if (Error Err = LT->parse(DebugLineData, &Offset, Ctx, U, WarnCallback))
+    if (Error Err =
+            LT->parse(DebugLineData, &Offset, Ctx, U, RecoverableErrorCallback))
       return std::move(Err);
     return LT;
   }
   return LT;
 }
 
 Error DWARFDebugLine::LineTable::parse(
     DWARFDataExtractor &DebugLineData, uint32_t *OffsetPtr,
     const DWARFContext &Ctx, const DWARFUnit *U,
-    std::function<void(StringRef)> WarnCallback, raw_ostream *OS) {
+    std::function<void(Error)> RecoverableErrorCallback, raw_ostream *OS) {
   const uint32_t DebugLineOffset = *OffsetPtr;
 
   clear();
 
   Error PrologueErr = Prologue.parse(DebugLineData, OffsetPtr, Ctx, U);
 
   if (OS) {
     // The presence of OS signals verbose dumping.
if (Opcode == 0) {
       // Reset discriminator to 0.
       State.Row.Discriminator = 0;
     }
     if(OS)
       *OS << "\n";
   }
 
   if (!State.Sequence.Empty)
-    WarnCallback("last sequence in debug line table is not terminated!");
+    RecoverableErrorCallback(
+        createError("last sequence in debug line table is not terminated!"));
 
   // Sort all sequences so that address lookup will work faster.
   if (!Sequences.empty()) {
     llvm::sort(Sequences.begin(), Sequences.end(), Sequence::orderByLowPC);
     // Note: actually, instruction address ranges of sequences should not
     // overlap (in shared objects and executables). If they do, the address
     // lookup would still work, though, but result would be ambiguous.
     // We don't report warning in this case. For example,
if (!DebugLineData.isValidOffset(Offset))
     Done = true;
 }
 
 bool DWARFDebugLine::Prologue::totalLengthIsValid() const {
   return TotalLength == 0xffffffff || TotalLength < 0xffffff00;
 }
 
 DWARFDebugLine::LineTable DWARFDebugLine::SectionParser::parseNext(
-    function_ref<void(StringRef)> StringCallback,
-    function_ref<void(Error)> ErrorCallback, raw_ostream *OS) {
+    function_ref<void(Error)> RecoverableErrorCallback,
+    function_ref<void(Error)> UnrecoverableErrorCallback, raw_ostream *OS) {
   assert(DebugLineData.isValidOffset(Offset) &&
          "parsing should have terminated");
   DWARFUnit *U = prepareToParse(Offset);
   uint32_t OldOffset = Offset;
   LineTable LT;
-  Error Err = LT.parse(DebugLineData, &Offset, Context, U, StringCallback, OS);
-  ErrorCallback(std::move(Err));
+  if (Error Err = LT.parse(DebugLineData, &Offset, Context, U,
+                           RecoverableErrorCallback, OS))
+    UnrecoverableErrorCallback(std::move(Err));
   moveToNextTable(OldOffset, LT.Prologue);
   return LT;
 }
 
 void DWARFDebugLine::SectionParser::skip(
     function_ref<void(Error)> ErrorCallback) {
   assert(DebugLineData.isValidOffset(Offset) &&
          "parsing should have terminated");
   DWARFUnit *U = prepareToParse(Offset);
   uint32_t OldOffset = Offset;
   LineTable LT;
-  Error Err = LT.Prologue.parse(DebugLineData, &Offset, Context, U);
+  if (Error Err = LT.Prologue.parse(DebugLineData, &Offset, Context, U))
     ErrorCallback(std::move(Err));
   moveToNextTable(OldOffset, LT.Prologue);
 }
 
 DWARFUnit *DWARFDebugLine::SectionParser::prepareToParse(uint32_t Offset) {
   DWARFUnit *U = nullptr;
   auto It = LineToUnit.find(Offset);
   if (It != LineToUnit.end())
     U = It->second;
void DWARFDebugLine::SectionParser::moveToNextTable(uint32_t OldOffset,
   }
 
   Offset = OldOffset + P.TotalLength + P.sizeofTotalLength();
   if (!DebugLineData.isValidOffset(Offset)) {
     Done = true;
   }
 }
 
-void DWARFDebugLine::warn(StringRef Message) {
-  WithColor::warning() << Message << '\n';
-}
-
-void DWARFDebugLine::warnForError(Error Err) {
-  handleAllErrors(std::move(Err),
-                  [](ErrorInfoBase &Info) { warn(Info.message()); });
+void DWARFDebugLine::warn(Error Err) {
+  handleAllErrors(std::move(Err), [](ErrorInfoBase &Info) {
+    WithColor::warning() << Info.message() << '\n';
+  });
 }

llvm/trunk/tools/dsymutil/DwarfLinker.cpp

void DwarfLinker::patchLineTableForUnit(CompileUnit &Unit,
   DWARFDebugLine::LineTable LineTable;
   uint32_t StmtOffset = *StmtList;
   DWARFDataExtractor LineExtractor(
       OrigDwarf.getDWARFObj(), OrigDwarf.getDWARFObj().getLineSection(),
       OrigDwarf.isLittleEndian(), Unit.getOrigUnit().getAddressByteSize());
 
   Error Err = LineTable.parse(LineExtractor, &StmtOffset, OrigDwarf,
                               &Unit.getOrigUnit());
-  DWARFDebugLine::warnForError(std::move(Err));
+  DWARFDebugLine::warn(std::move(Err));
 
   // This vector is the output line table.
   std::vector<DWARFDebugLine::Row> NewRows;
   NewRows.reserve(LineTable.Rows.size());
 
   // Current sequence of rows being extracted, before being inserted
   // in NewRows.
   std::vector<DWARFDebugLine::Row> Seq;

llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

 using namespace dwarfgen;
 using namespace object;
 using namespace utils;
 using namespace testing;
 
 namespace {
 struct CommonFixture {
   CommonFixture()
-      : LineData("", true, 0),
-        RecordIssue(std::bind(&CommonFixture::recordIssue, this,
+      : LineData("", true, 0), Recoverable(Error::success()),
+        RecordRecoverable(std::bind(&CommonFixture::recordRecoverable, this,
                                     std::placeholders::_1)),
-        FoundError(Error::success()),
-        RecordError(std::bind(&CommonFixture::recordError, this,
+        Unrecoverable(Error::success()),
+        RecordUnrecoverable(std::bind(&CommonFixture::recordUnrecoverable, this,
                                       std::placeholders::_1)){};
 
-  ~CommonFixture() { EXPECT_FALSE(FoundError); }
+  ~CommonFixture() {
+    EXPECT_FALSE(Recoverable);
+    EXPECT_FALSE(Unrecoverable);
+  }
 
   bool setupGenerator(uint16_t Version = 4) {
     Triple T = getHostTripleForAddrSize(8);
     if (!isConfigurationSupported(T))
       return false;
     auto ExpectedGenerator = Generator::create(T, Version);
     if (ExpectedGenerator)
       Gen.reset(ExpectedGenerator->release());
DWARFDebugLine::SectionParser setupParser() {
     LT2.addByte(0xbb);
     LT2.addExtendedOpcode(1, DW_LNE_end_sequence, {});
 
     generate();
 
     return DWARFDebugLine::SectionParser(LineData, *Context, CUs, TUs);
   }
 
-  void recordIssue(StringRef Message) { IssueMessage = Message; }
-  void recordError(Error Err) {
-    FoundError = joinErrors(std::move(FoundError), std::move(Err));
+  void recordRecoverable(Error Err) {
+    Recoverable = joinErrors(std::move(Recoverable), std::move(Err));
+  }
+  void recordUnrecoverable(Error Err) {
+    Unrecoverable = joinErrors(std::move(Unrecoverable), std::move(Err));
   }
 
   void checkError(ArrayRef<StringRef> ExpectedMsgs, Error Err) {
     ASSERT_TRUE(Err.operator bool());
     size_t WhichMsg = 0;
     Error Remaining =
         handleErrors(std::move(Err), [&](const ErrorInfoBase &Actual) {
           ASSERT_LT(WhichMsg, ExpectedMsgs.size());
           // Use .str(), because googletest doesn't visualise a StringRef
           // properly.
           EXPECT_EQ(Actual.message(), ExpectedMsgs[WhichMsg++].str());
         });
     EXPECT_EQ(WhichMsg, ExpectedMsgs.size());
     EXPECT_FALSE(Remaining);
   }
 
   void checkError(StringRef ExpectedMsg, Error Err) {
     checkError(ArrayRef<StringRef>{ExpectedMsg}, std::move(Err));
   }
 
   void checkGetOrParseLineTableEmitsError(StringRef ExpectedMsg,
                                           uint64_t Offset = 0) {
     auto ExpectedLineTable = Line.getOrParseLineTable(
-        LineData, Offset, *Context, nullptr, RecordIssue);
+        LineData, Offset, *Context, nullptr, RecordRecoverable);
     EXPECT_FALSE(ExpectedLineTable);
-    EXPECT_TRUE(IssueMessage.empty());
+    EXPECT_FALSE(Recoverable);
 
     checkError(ExpectedMsg, ExpectedLineTable.takeError());
   }
 
   std::unique_ptr<Generator> Gen;
   std::unique_ptr<DWARFContext> Context;
   DWARFDataExtractor LineData;
   DWARFDebugLine Line;
-  std::string IssueMessage;
-  std::function<void(StringRef)> RecordIssue;
-  Error FoundError;
-  std::function<void(Error)> RecordError;
+  Error Recoverable;
+  std::function<void(Error)> RecordRecoverable;
+  Error Unrecoverable;
+  std::function<void(Error)> RecordUnrecoverable;
 
   SmallVector<std::unique_ptr<DWARFCompileUnit>, 2> CUs;
   std::deque<DWARFUnitSection<DWARFTypeUnit>> TUs;
 };
 
 // Fixtures must derive from "Test", but parameterised fixtures from
 // "TestWithParam". It does not seem possible to inherit from both, so we share
 // the common state in a separate class, inherited by the two fixture classes.
TEST_P(DebugLineParameterisedFixture, GetOrParseLineTableValidTable) {
   LT2.addExtendedOpcode(1, DW_LNE_end_sequence, {});
   LT2.addExtendedOpcode(9, DW_LNE_set_address, {{0x55667788, LineTable::Quad}});
   LT2.addStandardOpcode(DW_LNS_copy, {});
   LT2.addByte(0xcc);
   LT2.addExtendedOpcode(1, DW_LNE_end_sequence, {});
 
   generate();
 
-  auto ExpectedLineTable =
-      Line.getOrParseLineTable(LineData, 0, *Context, nullptr, RecordIssue);
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
   ASSERT_TRUE(ExpectedLineTable.operator bool());
-  EXPECT_TRUE(IssueMessage.empty());
+  EXPECT_FALSE(Recoverable);
   const DWARFDebugLine::LineTable *Expected = *ExpectedLineTable;
   checkDefaultPrologue(Version, Format, Expected->Prologue, 16);
   EXPECT_EQ(Expected->Sequences.size(), 1u);
 
   uint64_t SecondOffset =
       Expected->Prologue.sizeofTotalLength() + Expected->Prologue.TotalLength;
-  IssueMessage.clear();
+  Recoverable = Error::success();
   auto ExpectedLineTable2 = Line.getOrParseLineTable(
-      LineData, SecondOffset, *Context, nullptr, RecordIssue);
+      LineData, SecondOffset, *Context, nullptr, RecordRecoverable);
   ASSERT_TRUE(ExpectedLineTable2.operator bool());
-  EXPECT_TRUE(IssueMessage.empty());
+  EXPECT_FALSE(Recoverable);
   const DWARFDebugLine::LineTable *Expected2 = *ExpectedLineTable2;
   checkDefaultPrologue(Version, Format, Expected2->Prologue, 32);
   EXPECT_EQ(Expected2->Sequences.size(), 2u);
 
   EXPECT_NE(Expected, Expected2);
 
   // Check that if the same offset is requested, the exact same pointer is
   // returned.
-  IssueMessage.clear();
-  auto ExpectedLineTable3 =
-      Line.getOrParseLineTable(LineData, 0, *Context, nullptr, RecordIssue);
+  Recoverable = Error::success();
+  auto ExpectedLineTable3 = Line.getOrParseLineTable(
+      LineData, 0, *Context, nullptr, RecordRecoverable);
   ASSERT_TRUE(ExpectedLineTable3.operator bool());
-  EXPECT_TRUE(IssueMessage.empty());
+  EXPECT_FALSE(Recoverable);
   EXPECT_EQ(Expected, *ExpectedLineTable3);
 
-  IssueMessage.clear();
+  Recoverable = Error::success();
   auto ExpectedLineTable4 = Line.getOrParseLineTable(
-      LineData, SecondOffset, *Context, nullptr, RecordIssue);
+      LineData, SecondOffset, *Context, nullptr, RecordRecoverable);
   ASSERT_TRUE(ExpectedLineTable4.operator bool());
-  EXPECT_TRUE(IssueMessage.empty());
+  EXPECT_FALSE(Recoverable);
   EXPECT_EQ(Expected2, *ExpectedLineTable4);
 
   // TODO: Add tests that show that the body of the programs have been read
   // correctly.
 }
 
 TEST_F(DebugLineBasicFixture, ErrorForReservedLength) {
   if (!setupGenerator())

 }
 
 INSTANTIATE_TEST_CASE_P(
     LineTableTestParams, DebugLineParameterisedFixture,
     Values(std::make_pair(
                2, DWARF32), // Test lower-bound of v2-3 fields and DWARF32.
            std::make_pair(3, DWARF32), // Test upper-bound of v2-3 fields.
            std::make_pair(4, DWARF64), // Test v4 fields and DWARF64.
-           std::make_pair(5, DWARF32), std::make_pair(5, DWARF64)),);
+           std::make_pair(5, DWARF32), std::make_pair(5, DWARF64)), );
 
 TEST_F(DebugLineBasicFixture, ErrorForInvalidExtendedOpcodeLength) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable();
   // The Length should be 1 for an end sequence opcode.
   LT.addExtendedOpcode(2, DW_LNE_end_sequence, {});
TEST_F(DebugLineBasicFixture, CallbackUsedForUnterminatedSequence) {
   LT.addExtendedOpcode(9, DW_LNE_set_address,
                        {{0x99aabbccddeeff00, LineTable::Quad}});
   LT.addStandardOpcode(DW_LNS_copy, {});
   LT.addByte(0xbb);
   LT.addByte(0xcc);
 
   generate();
 
-  auto ExpectedLineTable =
-      Line.getOrParseLineTable(LineData, 0, *Context, nullptr, RecordIssue);
-  EXPECT_EQ(IssueMessage,
-            "last sequence in debug line table is not terminated!");
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
+  checkError("last sequence in debug line table is not terminated!",
+             std::move(Recoverable));
   ASSERT_TRUE(ExpectedLineTable.operator bool());
   EXPECT_EQ((*ExpectedLineTable)->Rows.size(), 6u);
   // The unterminated sequence is not added to the sequence list.
   EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
 }
 
 TEST_F(DebugLineBasicFixture, ParserParsesCorrectly) {
   if (!setupGenerator())
     return;
 
   DWARFDebugLine::SectionParser Parser = setupParser();
 
   EXPECT_EQ(Parser.getOffset(), 0u);
   ASSERT_FALSE(Parser.done());
 
-  DWARFDebugLine::LineTable Parsed = Parser.parseNext(RecordIssue, RecordError);
+  DWARFDebugLine::LineTable Parsed =
+      Parser.parseNext(RecordRecoverable, RecordUnrecoverable);
   checkDefaultPrologue(4, DWARF32, Parsed.Prologue, 16);
   EXPECT_EQ(Parsed.Sequences.size(), 1u);
   EXPECT_EQ(Parser.getOffset(), 62u);
   ASSERT_FALSE(Parser.done());
 
   DWARFDebugLine::LineTable Parsed2 =
-      Parser.parseNext(RecordIssue, RecordError);
+      Parser.parseNext(RecordRecoverable, RecordUnrecoverable);
   checkDefaultPrologue(4, DWARF64, Parsed2.Prologue, 16);
   EXPECT_EQ(Parsed2.Sequences.size(), 1u);
   EXPECT_EQ(Parser.getOffset(), 136u);
   EXPECT_TRUE(Parser.done());
 
-  EXPECT_TRUE(IssueMessage.empty());
-  EXPECT_FALSE(FoundError);
+  EXPECT_FALSE(Recoverable);
+  EXPECT_FALSE(Unrecoverable);
 }
 
 TEST_F(DebugLineBasicFixture, ParserSkipsCorrectly) {
   if (!setupGenerator())
     return;
 
   DWARFDebugLine::SectionParser Parser = setupParser();
 
   EXPECT_EQ(Parser.getOffset(), 0u);
   ASSERT_FALSE(Parser.done());
 
-  Parser.skip(RecordError);
+  Parser.skip(RecordUnrecoverable);
   EXPECT_EQ(Parser.getOffset(), 62u);
   ASSERT_FALSE(Parser.done());
 
-  Parser.skip(RecordError);
+  Parser.skip(RecordUnrecoverable);
   EXPECT_EQ(Parser.getOffset(), 136u);
   EXPECT_TRUE(Parser.done());
 
-  EXPECT_FALSE(FoundError);
+  EXPECT_FALSE(Unrecoverable);
 }
 
 TEST_F(DebugLineBasicFixture, ParserAlwaysDoneForEmptySection) {
   if (!setupGenerator())
     return;
 
   generate();
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
 
   EXPECT_TRUE(Parser.done());
 }
 
 TEST_F(DebugLineBasicFixture, ParserMovesToEndForBadLengthWhenParsing) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable();
   LT.setCustomPrologue({{0xffffff00, LineTable::Long}});
   Gen->addLineTable();
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.parseNext(RecordIssue, RecordError);
+  Parser.parseNext(RecordRecoverable, RecordUnrecoverable);
 
   EXPECT_EQ(Parser.getOffset(), 4u);
   EXPECT_TRUE(Parser.done());
-  EXPECT_TRUE(IssueMessage.empty());
+  EXPECT_FALSE(Recoverable);
 
   checkError("parsing line table prologue at offset 0x00000000 unsupported "
              "reserved unit length found of value 0xffffff00",
-             std::move(FoundError));
+             std::move(Unrecoverable));
 }
 
 TEST_F(DebugLineBasicFixture, ParserMovesToEndForBadLengthWhenSkipping) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable();
   LT.setCustomPrologue({{0xffffff00, LineTable::Long}});
   Gen->addLineTable();
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.skip(RecordError);
+  Parser.skip(RecordUnrecoverable);
 
   EXPECT_EQ(Parser.getOffset(), 4u);
   EXPECT_TRUE(Parser.done());
 
   checkError("parsing line table prologue at offset 0x00000000 unsupported "
              "reserved unit length found of value 0xffffff00",
-             std::move(FoundError));
+             std::move(Unrecoverable));
 }
 
 TEST_F(DebugLineBasicFixture, ParserReportsFirstErrorInEachTableWhenParsing) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable(DWARF32);
   LT.setCustomPrologue({{2, LineTable::Long}, {0, LineTable::Half}});
   LineTable &LT2 = Gen->addLineTable(DWARF32);
   LT2.setCustomPrologue({{2, LineTable::Long}, {1, LineTable::Half}});
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.parseNext(RecordIssue, RecordError);
+  Parser.parseNext(RecordRecoverable, RecordUnrecoverable);
   ASSERT_FALSE(Parser.done());
-  Parser.parseNext(RecordIssue, RecordError);
+  Parser.parseNext(RecordRecoverable, RecordUnrecoverable);
 
   EXPECT_TRUE(Parser.done());
-  EXPECT_TRUE(IssueMessage.empty());
+  EXPECT_FALSE(Recoverable);
 
   checkError({"parsing line table prologue at offset 0x00000000 found "
               "unsupported version 0x00",
               "parsing line table prologue at offset 0x00000006 found "
               "unsupported version 0x01"},
-             std::move(FoundError));
+             std::move(Unrecoverable));
 }
 
 TEST_F(DebugLineBasicFixture, ParserReportsNonPrologueProblemsWhenParsing) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable(DWARF32);
   LT.addExtendedOpcode(0x42, DW_LNE_end_sequence, {});
   LineTable &LT2 = Gen->addLineTable(DWARF32);
   LT2.addExtendedOpcode(9, DW_LNE_set_address,
                         {{0x1234567890abcdef, LineTable::Quad}});
   LT2.addStandardOpcode(DW_LNS_copy, {});
   LT2.addByte(0xbb);
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.parseNext(RecordIssue, RecordError);
-  EXPECT_TRUE(IssueMessage.empty());
+  Parser.parseNext(RecordRecoverable, RecordUnrecoverable);
+  EXPECT_FALSE(Recoverable);
   ASSERT_FALSE(Parser.done());
   checkError(
       "unexpected line op length at offset 0x00000030 expected 0x42 found 0x01",
-      std::move(FoundError));
+      std::move(Unrecoverable));
 
   // Reset the error state so that it does not confuse the next set of checks.
-  FoundError = Error::success();
-  Parser.parseNext(RecordIssue, RecordError);
+  Unrecoverable = Error::success();
+  Parser.parseNext(RecordRecoverable, RecordUnrecoverable);
 
   EXPECT_TRUE(Parser.done());
-  EXPECT_EQ(IssueMessage,
-            "last sequence in debug line table is not terminated!");
-  EXPECT_TRUE(!FoundError);
+  checkError("last sequence in debug line table is not terminated!",
+             std::move(Recoverable));
+  EXPECT_FALSE(Unrecoverable);
 }
 
 TEST_F(DebugLineBasicFixture,
        ParserReportsPrologueErrorsInEachTableWhenSkipping) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable(DWARF32);
   LT.setCustomPrologue({{2, LineTable::Long}, {0, LineTable::Half}});
   LineTable &LT2 = Gen->addLineTable(DWARF32);
   LT2.setCustomPrologue({{2, LineTable::Long}, {1, LineTable::Half}});
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.skip(RecordError);
+  Parser.skip(RecordUnrecoverable);
   ASSERT_FALSE(Parser.done());
-  Parser.skip(RecordError);
+  Parser.skip(RecordUnrecoverable);
 
   EXPECT_TRUE(Parser.done());
 
   checkError({"parsing line table prologue at offset 0x00000000 found "
               "unsupported version 0x00",
               "parsing line table prologue at offset 0x00000006 found "
               "unsupported version 0x01"},
-             std::move(FoundError));
+             std::move(Unrecoverable));
 }
 
 TEST_F(DebugLineBasicFixture, ParserIgnoresNonPrologueErrorsWhenSkipping) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable(DWARF32);
   LT.addExtendedOpcode(42, DW_LNE_end_sequence, {});
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.skip(RecordError);
+  Parser.skip(RecordUnrecoverable);
 
   EXPECT_TRUE(Parser.done());
-  EXPECT_TRUE(!FoundError);
+  EXPECT_FALSE(Unrecoverable);
 }
 
 } // end anonymous namespace