[PATCH] D48175: [BinaryFormat] Add MsgPackTypes

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: include/llvm/BinaryFormat/MsgPackTypes.h:32-35
+/// Template to choose between a std::unique_ptr and std::shared_ptr
+template <class T, bool U>
+using Ptr =
+    typename std::conditional<U, std::unique_ptr<T>, std::shared_ptr<T>>::type;
----------------
scott.linder wrote:
> dblaikie wrote:
> > What's the need for this?
> > 
> > I suspect it's probably best to have the APIs always return unique_ptrs - which are implicitly convertible to shared_ptrs if the users of the API need that.
> The reason for this is that the methods to stream in MessagePack/YAML have to know which type to construct the document tree with. A single unique_ptr is implicitly convertible, but with a tree of them the conversions are not useful if you are trying to share parts of the tree. Unfortunately this does complicate the YAML trait specialization a bit.
Is there a significant win to using unique_ptr rather than using shared_ptr always, then?

& what's the sharing situation? Perhaps we could discuss that & see if there's a reasonable single ownership model that'd work.


https://reviews.llvm.org/D48175