[PATCH] Make DiagnosticsEngine::takeClient return std::unique_ptr<>

[David Blaikie]

Looks mostly like a sort-of improvement, with a bit more tidying up that could be done in follow-up patches (though I'd slightly prefer to drop the !! before committing this at least, and double-check the owner naming convention in existing cases to be consistent)

================
Comment at: include/clang/Basic/Diagnostic.h:370
@@ -370,3 +369,3 @@
   /// \brief Determine whether this \c DiagnosticsEngine object own its client.
-  bool ownsClient() const { return OwnsDiagClient; }
-  
+  bool ownsClient() const { return !!OwningClient; }
+
----------------
I don't think !! is idiomatic in the LLVM codebase (though I may be incorrect) & probably just (bool) or static_cast<bool> would be more consistent.

================
Comment at: include/clang/Rewrite/Frontend/FixItRewriter.h:69
@@ -68,3 +68,3 @@
   DiagnosticConsumer *Client;
-  bool OwnsClient;
+  std::unique_ptr<DiagnosticConsumer> OwningClient;
 
----------------
What's the naming convention for other T*+unique_ptr<T> combos like this? I think I introduced the first few of these (see the email thread on the "maybe owning" pointer semantics in llvm-dev/cfe-dev for a list of other cases) and I might've used "*Owner" rather than "Owning*" - it'd be nice to be consistent (so they're easy to find and cleanup later).

================
Comment at: lib/Frontend/Rewrite/FixItRewriter.cpp:45
@@ -44,4 +44,3 @@
 FixItRewriter::~FixItRewriter() {
-  Diags.takeClient();
-  Diags.setClient(Client, OwnsClient);
+  Diags.setClient(Client, !!OwningClient.release());
 }
----------------
As mentioned above (hmm, should this class use the CaptureDroppedDiagnostics? I guess not (it probably does other things) - but maybe we should have a common scoped "replace the DiagnosticConsumer" device that could be used in both places? - though that's probably overkill once we fix the ownership one way or another)

================
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:46
@@ -50,1 +45,3 @@
+  CheckDiagnostics();
+  Diags.takeClient().release();
 }
----------------
While it didn't have it before - this might deserve a comment.

http://reviews.llvm.org/D6294