[PATCH] D79671: [MsgPack] MsgPackDocument::readFromBlob now merges

Tim Renouf via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 12 11:49:33 PDT 2020


tpr marked an inline comment as done.
tpr added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/MsgPackDocument.h:370-371
+          Merger = [](DocNode *DestNode, DocNode SrcNode, DocNode MapKey) {
+            return (DestNode->isMap() && SrcNode.isMap()) ||
+                   (DestNode->isArray() && SrcNode.isArray());
+          });
----------------
foad wrote:
> Can't the body of this default merger be just `llvm_unreachable();`?
Probably, although that is a change in behavior from before, which I'd rather not do as I think I am not the only user of it. (Also, even if we did do that, it should be "return false" so that the readFromBlob call returns false for failure.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79671/new/

https://reviews.llvm.org/D79671





More information about the llvm-commits mailing list