[llvm-branch-commits] [Remarks] BitstreamRemarkParser: Refactor error handling (PR #156511)

[Jon Roelofs via llvm-branch-commits]

================
@@ -13,81 +13,171 @@
 #ifndef LLVM_LIB_REMARKS_BITSTREAM_REMARK_PARSER_H
 #define LLVM_LIB_REMARKS_BITSTREAM_REMARK_PARSER_H
 
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Bitstream/BitstreamReader.h"
 #include "llvm/Remarks/BitstreamRemarkContainer.h"
+#include "llvm/Remarks/Remark.h"
 #include "llvm/Remarks/RemarkFormat.h"
 #include "llvm/Remarks/RemarkParser.h"
+#include "llvm/Remarks/RemarkStringTable.h"
 #include "llvm/Support/Error.h"
-#include <array>
+#include "llvm/Support/FormatVariadic.h"
 #include <cstdint>
 #include <memory>
 #include <optional>
 
 namespace llvm {
 namespace remarks {
 
-struct Remark;
+class BitstreamBlockParserHelperBase {
+protected:
+  BitstreamCursor &Stream;
+
+  unsigned BlockID;
+  StringRef BlockName;
+
+public:
+  BitstreamBlockParserHelperBase(BitstreamCursor &Stream, unsigned BlockID,
+                                 StringRef BlockName)
+      : Stream(Stream), BlockID(BlockID), BlockName(BlockName) {}
+
+  template <typename... Ts> Error error(char const *Fmt, const Ts &...Vals) {
+    std::string Buffer;
+    raw_string_ostream OS(Buffer);
+    OS << "Error while parsing " << BlockName << " block: ";
+    OS << formatv(Fmt, Vals...);
+    return make_error<StringError>(
+        std::move(Buffer),
+        std::make_error_code(std::errc::illegal_byte_sequence));
+  }
+
+  Error expectBlock();
+
+protected:
+  Error enterBlock();
+
+  Error unknownRecord(unsigned AbbrevID);
+  Error unexpectedRecord(StringRef RecordName);
+  Error malformedRecord(StringRef RecordName);
+  Error unexpectedBlock(unsigned Code);
+};
+
+template <typename Derived>
+class BitstreamBlockParserHelper : public BitstreamBlockParserHelperBase {
+protected:
+  using BitstreamBlockParserHelperBase::BitstreamBlockParserHelperBase;
+  Derived &derived() { return *static_cast<Derived *>(this); }
+
+  /// Parse a record and fill in the fields in the parser.
+  /// The subclass can statically override this method.
+  Error parseRecord(unsigned Code) { return unexpectedRecord(Code); }
+
+  /// Parse a subblock and fill in the fields in the parser.
+  /// The subclass can statically override this method.
+  Error parseSubBlock(unsigned Code) { return unexpectedBlock(Code); }
+
+public:
+  /// Enter, parse, and leave this bitstream block. This expects the
+  /// BitstreamCursor to be right after the SubBlock entry (i.e. after calling
+  /// expectBlock).
+  Error parseBlock() {
+    if (Error E = enterBlock())
+      return E;
+
+    // Stop when there is nothing to read anymore or when we encounter an
+    // END_BLOCK.
+    while (true) {
+      Expected<BitstreamEntry> Next = Stream.advance();
+      if (!Next)
+        return Next.takeError();
+      switch (Next->Kind) {
+      case BitstreamEntry::SubBlock:
+        if (Error E = derived().parseSubBlock(Next->ID))
+          return E;
+        continue;
+      case BitstreamEntry::EndBlock:
+        return Error::success();
+      case BitstreamEntry::Record:
+        if (Error E = derived().parseRecord(Next->ID))
+          return E;
+        continue;
+      case BitstreamEntry::Error:
+        return error("Unexpected end of bitstream.");
+      }
+    }
----------------
jroelofs wrote:

is the switch fully covered? if so: llvm_unreachable

https://github.com/llvm/llvm-project/pull/156511