[llvm] [MsgPack] Return an Error instead of bool from readFromBlob (PR #74357)

[Emma Pilkington via llvm-commits]

================
@@ -63,27 +63,28 @@ void AMDGPUPALMetadata::readFromIR(Module &M) {
   }
 }
 
-// Set PAL metadata from a binary blob from the applicable .note record.
-// Returns false if bad format.  Blob must remain valid for the lifetime of the
-// Metadata.
-bool AMDGPUPALMetadata::setFromBlob(unsigned Type, StringRef Blob) {
+// Set PAL metadata from a binary blob from the applicable .note record. Blob
+// must remain valid for the lifetime of the Metadata.
+void AMDGPUPALMetadata::setFromBlob(unsigned Type, StringRef Blob) {
   BlobType = Type;
-  if (Type == ELF::NT_AMD_PAL_METADATA)
-    return setFromLegacyBlob(Blob);
-  return setFromMsgPackBlob(Blob);
+  if (Type == ELF::NT_AMD_PAL_METADATA) {
+    setFromLegacyBlob(Blob);
+  } else {
+    setFromMsgPackBlob(Blob);
+  }
 }
 
 // Set PAL metadata from legacy (array of key=value pairs) blob.
-bool AMDGPUPALMetadata::setFromLegacyBlob(StringRef Blob) {
+void AMDGPUPALMetadata::setFromLegacyBlob(StringRef Blob) {
   auto Data = reinterpret_cast<const uint32_t *>(Blob.data());
   for (unsigned I = 0; I != Blob.size() / sizeof(uint32_t) / 2; ++I)
     setRegister(Data[I * 2], Data[I * 2 + 1]);
-  return true;
 }
 
 // Set PAL metadata from msgpack blob.
-bool AMDGPUPALMetadata::setFromMsgPackBlob(StringRef Blob) {
-  return MsgPackDoc.readFromBlob(Blob, /*Multi=*/false);
+void AMDGPUPALMetadata::setFromMsgPackBlob(StringRef Blob) {
+  if (!Blob.empty())
+    handleAllErrors(MsgPackDoc.readFromBlob(Blob, /*Multi=*/false));
----------------
epilk wrote:

I guess it's an assertion that the Blob is valid. There should never be an error so long as the frontend* generates valid metadata. I don't think its possible to fail more gracefully here either, our options are either 1) discard and overwrite the metadata, or 2) don't make any amendments to it. Previously we were doing 1) implicitly, which seems wrong to me, but 2) also seems wrong.

\* What frontend am I talking about? That's not rhetorical, I don't know! Clang doesn't seem to be aware of "!amdgpu.pal.metadata", but this comment references a frontend: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/Utils/AMDGPUPALMetadata.cpp#L29 . This metadata is used by lit tests.

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