dblaikie added inline comments. ================ Comment at: include/clang/Driver/Compilation.h:54 @@ +53,3 @@ + /// to consumers; it's here just to manage ownership. + std::vector<std::unique_ptr<Action>> OwningActions; + ---------------- Seems like this name isn't quite right ("OwningActions" sounds like "these are the actions that own <something>") - perhaps "AllActions"? (maybe "ActionHolder"? Not sure)
================ 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."); ---------------- 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. ================ 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(); ---------------- Up to you, but we usually just drop the "std::unique_ptr<T>" in favor of "auto" when the RHS has "make_unique". http://reviews.llvm.org/D15911 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits