[PATCH] D48175: [BinaryFormat] Add MsgPackTypes

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 30 13:36:44 PDT 2018


dblaikie added a comment.

In https://reviews.llvm.org/D48175#1219832, @scott.linder wrote:

> The use I have in mind is actually an external component which depends on LLVM. We currently implement reading MessagePack metadata there, but when we land the changes to generate it in LLVM it would be nice to avoid duplicating the code there.
>
> The rationale for a shared_ptr tree is that our API hands out opaque handles to sub-trees in the metadata which have their own lifetime independent of the root. With shared_ptr this just naturally falls out, but with unique_ptr it requires some book-keeping.
>
> I do not see a scenario where the shared_ptr will be needed in LLVM, so if you prefer I am happy to update the patch to use unique_ptr.


Difficult call, but seems OK to me since you'll mostly be the only folks using the data structure anyway.

Could you include a representative unit test of this ownership situation? (can be fairly simple, but demonstrate that the root may be built, a subtree taken, then the tree destroyed and the subtree still used).


https://reviews.llvm.org/D48175





More information about the llvm-commits mailing list