jlebar added inline comments.
================
Comment at: include/clang/Driver/Action.h:36
@@ -35,1 +35,3 @@
+///
+/// Actions are usually owned by a Compilation.
class Action {
----------------
tra wrote:
> There's no API to pass ownership to Compilation explicitly, so the only way
> for an Action to be owned by Compilation is to create it with MakeAction.
>
> Perhaps "Actions created with MakeAction<>() are owned by Compilation"
>
> BTW, should we (can we?) make MakeAction<>() the only way to create actions?
Updated the comment.
================
Comment at: include/clang/Driver/Compilation.h:118
@@ +117,3 @@
+ template <typename T, typename... Args> T *MakeAction(Args &&... Arg) {
+ static_assert(std::is_convertible<T *, Action *>::value,
+ "T must be derived from Action.");
----------------
dblaikie wrote:
> Did you find this static_assert to be particularly helpful? Similar errors
> (though somewhat more verbose) would be produced during the push_back call a
> few lines down anyway.
Fair enough. I got rid of it but changed my unique_ptr into a
unique_ptr<Action> so that we'll get an error somewhere other than inside the
bowels of SmallVector.
================
Comment at: include/clang/Driver/Compilation.h:120
@@ +119,3 @@
+ "T must be derived from Action.");
+ std::unique_ptr<T> A = llvm::make_unique<T>(std::forward<Args>(Arg)...);
+ T* RawPtr = A.get();
----------------
dblaikie wrote:
> Up to you, but we usually just drop the "std::unique_ptr<T>" in favor of
> "auto" when the RHS has "make_unique".
Done, but then I realized I can just use operator new and skirt this entirely.
http://reviews.llvm.org/D15911
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
