[PATCH] D40244: [dsymutil] Upstream getBundleInfo implementation

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 28 10:47:05 PST 2017


vsk added a comment.

Thanks for sending this out! Are there tests?



================
Comment at: tools/dsymutil/CFBundle.cpp:20
+/// std:unique_ptr<T>::release() does.
+template <class T> class CFReleaser {
+public:
----------------
Why not define CFReleaser as a std::unique_ptr with a custom deleter? That would drop support for `get_address`, but it looks dead anyway.


================
Comment at: tools/dsymutil/CFBundle.cpp:91
+  CFString &operator=(const CFString &rhs);
+  virtual ~CFString();
+
----------------
Why do the subclasses of CFReleaser<T> need virtual destructors?


================
Comment at: tools/dsymutil/CFBundle.h:1
+
+#include "llvm/ADT/StringRef.h"
----------------
Missing license?


Repository:
  rL LLVM

https://reviews.llvm.org/D40244





More information about the llvm-commits mailing list