[PATCH] D15911: Move ownership of Action objects into Compilation.

[David Blaikie via cfe-commits]

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