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

[Florian Hahn via llvm-branch-commits]

================
@@ -52,171 +92,144 @@ static Error parseRecord(BitstreamMetaParserHelper &Parser, unsigned Code) {
   switch (*RecordID) {
   case RECORD_META_CONTAINER_INFO: {
     if (Record.size() != 2)
-      return malformedRecord("BLOCK_META", "RECORD_META_CONTAINER_INFO");
-    Parser.ContainerVersion = Record[0];
-    Parser.ContainerType = Record[1];
+      return malformedRecord(MetaContainerInfoName);
+    Container.emplace();
+    Container->Version = Record[0];
+    Container->Type = Record[1];
+    // Error immediately if container version is outdated, so the user sees an
+    // explanation instead of a parser error.
+    if (Container->Version != CurrentContainerVersion) {
+      return ::error(
+          "Unsupported remark container version (expected: {}, read: {}). "
+          "Please upgrade/downgrade your toolchain to read this container.",
+          CurrentContainerVersion, Container->Version);
+    }
     break;
   }
   case RECORD_META_REMARK_VERSION: {
     if (Record.size() != 1)
-      return malformedRecord("BLOCK_META", "RECORD_META_REMARK_VERSION");
-    Parser.RemarkVersion = Record[0];
+      return malformedRecord(MetaRemarkVersionName);
+    RemarkVersion = Record[0];
+    // Error immediately if remark version is outdated, so the user sees an
+    // explanation instead of a parser error.
+    if (*RemarkVersion != CurrentRemarkVersion) {
+      return ::error(
+          "Unsupported remark version in container (expected: {}, read: {}). "
+          "Please upgrade/downgrade your toolchain to read this container.",
+          CurrentRemarkVersion, *RemarkVersion);
+    }
     break;
   }
   case RECORD_META_STRTAB: {
     if (Record.size() != 0)
-      return malformedRecord("BLOCK_META", "RECORD_META_STRTAB");
-    Parser.StrTabBuf = Blob;
+      return malformedRecord(MetaStrTabName);
+    StrTabBuf = Blob;
     break;
   }
   case RECORD_META_EXTERNAL_FILE: {
     if (Record.size() != 0)
-      return malformedRecord("BLOCK_META", "RECORD_META_EXTERNAL_FILE");
-    Parser.ExternalFilePath = Blob;
+      return malformedRecord(MetaExternalFileName);
+    ExternalFilePath = Blob;
     break;
   }
   default:
-    return unknownRecord("BLOCK_META", *RecordID);
+    return unknownRecord(*RecordID);
   }
   return Error::success();
 }
 
-BitstreamRemarkParserHelper::BitstreamRemarkParserHelper(
-    BitstreamCursor &Stream)
-    : Stream(Stream) {}
-
-/// Parse a record and fill in the fields in the parser.
-static Error parseRecord(BitstreamRemarkParserHelper &Parser, unsigned Code) {
-  BitstreamCursor &Stream = Parser.Stream;
-  // Note: 5 is used here because it's the max number of fields we have per
-  // record.
-  SmallVector<uint64_t, 5> Record;
-  StringRef Blob;
-  Expected<unsigned> RecordID = Stream.readRecord(Code, Record, &Blob);
-  if (!RecordID)
-    return RecordID.takeError();
+Error BitstreamRemarkParserHelper::parseRecord(unsigned Code) {
+  Record.clear();
+  Expected<unsigned> MaybeRecordID =
+      Stream.readRecord(Code, Record, &RecordBlob);
+  if (!MaybeRecordID)
+    return MaybeRecordID.takeError();
+  RecordID = *MaybeRecordID;
+  return handleRecord();
+}
 
-  switch (*RecordID) {
+Error BitstreamRemarkParserHelper::handleRecord() {
+  switch (RecordID) {
   case RECORD_REMARK_HEADER: {
     if (Record.size() != 4)
-      return malformedRecord("BLOCK_REMARK", "RECORD_REMARK_HEADER");
-    Parser.Type = Record[0];
-    Parser.RemarkNameIdx = Record[1];
-    Parser.PassNameIdx = Record[2];
-    Parser.FunctionNameIdx = Record[3];
+      return malformedRecord(RemarkHeaderName);
+    Type = Record[0];
+    RemarkNameIdx = Record[1];
+    PassNameIdx = Record[2];
+    FunctionNameIdx = Record[3];
     break;
   }
   case RECORD_REMARK_DEBUG_LOC: {
     if (Record.size() != 3)
-      return malformedRecord("BLOCK_REMARK", "RECORD_REMARK_DEBUG_LOC");
-    Parser.SourceFileNameIdx = Record[0];
-    Parser.SourceLine = Record[1];
-    Parser.SourceColumn = Record[2];
+      return malformedRecord(RemarkDebugLocName);
+    Loc.emplace();
+    Loc->SourceFileNameIdx = Record[0];
+    Loc->SourceLine = Record[1];
+    Loc->SourceColumn = Record[2];
----------------
fhahn wrote:

can we construct the obejct in `emplace`?

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