Simplify the ownership model for DiagnosticClients, which was really

convoluted and a bit leaky. Now, the Diagnostic object owns its
DiagnosticClient.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@111437 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 23 insertions, 33 deletions

diff --git a/lib/Frontend/ASTUnit.cpp b/lib/Frontend/ASTUnit.cpp
index 019d25c131..28cdc52922 100644
--- a/lib/Frontend/ASTUnit.cpp
+++ b/lib/Frontend/ASTUnit.cpp
@@ -371,14 +371,19 @@ class CaptureDroppedDiagnostics {
 public:
   CaptureDroppedDiagnostics(bool RequestCapture, Diagnostic &Diags, 
                            llvm::SmallVectorImpl<StoredDiagnostic> &StoredDiags)
-    : Diags(Diags), Client(StoredDiags), PreviousClient(Diags.getClient()) 
+    : Diags(Diags), Client(StoredDiags), PreviousClient(0)
   {
-    if (RequestCapture || Diags.getClient() == 0)
+    if (RequestCapture || Diags.getClient() == 0) {
+      PreviousClient = Diags.takeClient();
       Diags.setClient(&Client);
+    }
   }
 
   ~CaptureDroppedDiagnostics() {
-    Diags.setClient(PreviousClient);
+    if (Diags.getClient() == &Client) {
+      Diags.takeClient();
+      Diags.setClient(PreviousClient);
+    }
   }
 };
 
@@ -654,15 +659,12 @@ bool ASTUnit::Parse(llvm::MemoryBuffer *OverrideMainBuffer) {
   CaptureDroppedDiagnostics Capture(CaptureDiagnostics, 
                                     getDiagnostics(),
                                     StoredDiagnostics);
-  Clang.setDiagnosticClient(getDiagnostics().getClient());
   
   // Create the target instance.
   Clang.setTarget(TargetInfo::CreateTargetInfo(Clang.getDiagnostics(),
                                                Clang.getTargetOpts()));
-  if (!Clang.hasTarget()) {
-    Clang.takeDiagnosticClient();
+  if (!Clang.hasTarget())
     return true;
-  }
   
   // Inform the target of the language options.
   //
@@ -754,8 +756,6 @@ bool ASTUnit::Parse(llvm::MemoryBuffer *OverrideMainBuffer) {
     PreprocessorOpts.ImplicitPCHInclude = PriorImplicitPCHInclude;
   }
 
-  Clang.takeDiagnosticClient();
-  
   Invocation.reset(Clang.takeInvocation());
   
   // If we were asked to cache code-completion results and don't have any
@@ -776,7 +776,6 @@ error:
   
   Clang.takeSourceManager();
   Clang.takeFileManager();
-  Clang.takeDiagnosticClient();
   Invocation.reset(Clang.takeInvocation());
   return true;
 }
@@ -1123,13 +1122,11 @@ llvm::MemoryBuffer *ASTUnit::getMainBufferWithPrecompiledPreamble(
   CaptureDroppedDiagnostics Capture(CaptureDiagnostics, 
                                     getDiagnostics(),
                                     StoredDiagnostics);
-  Clang.setDiagnosticClient(getDiagnostics().getClient());
   
   // Create the target instance.
   Clang.setTarget(TargetInfo::CreateTargetInfo(Clang.getDiagnostics(),
                                                Clang.getTargetOpts()));
   if (!Clang.hasTarget()) {
-    Clang.takeDiagnosticClient();
     llvm::sys::Path(FrontendOpts.OutputFile).eraseFromDisk();
     Preamble.clear();
     if (CreatedPreambleBuffer)
@@ -1168,7 +1165,6 @@ llvm::MemoryBuffer *ASTUnit::getMainBufferWithPrecompiledPreamble(
   Act.reset(new PrecompilePreambleAction(*this));
   if (!Act->BeginSourceFile(Clang, Clang.getFrontendOpts().Inputs[0].second,
                             Clang.getFrontendOpts().Inputs[0].first)) {
-    Clang.takeDiagnosticClient();
     Clang.takeInvocation();
     llvm::sys::Path(FrontendOpts.OutputFile).eraseFromDisk();
     Preamble.clear();
@@ -1183,7 +1179,6 @@ llvm::MemoryBuffer *ASTUnit::getMainBufferWithPrecompiledPreamble(
   
   Act->Execute();
   Act->EndSourceFile();
-  Clang.takeDiagnosticClient();
   Clang.takeInvocation();
   
   if (Diagnostics->hasErrorOccurred()) {
@@ -1321,11 +1316,14 @@ ASTUnit *ASTUnit::LoadFromCommandLine(const char **ArgBegin,
                                       bool PrecompilePreamble,
                                       bool CompleteTranslationUnit,
                                       bool CacheCodeCompletionResults) {
+  bool CreatedDiagnosticsObject = false;
+  
   if (!Diags.getPtr()) {
     // No diagnostics engine was provided, so create our own diagnostics object
     // with the default options.
     DiagnosticOptions DiagOpts;
     Diags = CompilerInstance::createDiagnostics(DiagOpts, 0, 0);
+    CreatedDiagnosticsObject = true;
   }
   
   llvm::SmallVector<const char *, 16> Args;
@@ -1678,14 +1676,14 @@ void ASTUnit::CodeComplete(llvm::StringRef File, unsigned Line, unsigned Column,
   CaptureDroppedDiagnostics Capture(true, 
                                     Clang.getDiagnostics(),
                                     StoredDiagnostics);
-  Clang.setDiagnosticClient(Diag.getClient());
   
   // Create the target instance.
   Clang.setTarget(TargetInfo::CreateTargetInfo(Clang.getDiagnostics(),
                                                Clang.getTargetOpts()));
   if (!Clang.hasTarget()) {
-    Clang.takeDiagnosticClient();
     Clang.takeInvocation();
+    CCInvocation.getLangOpts().SpellChecking = SpellChecking;
+    return;
   }
   
   // Inform the target of the language options.
@@ -1773,7 +1771,6 @@ void ASTUnit::CodeComplete(llvm::StringRef File, unsigned Line, unsigned Column,
   Clang.takeFileManager();
   Clang.takeSourceManager();
   Clang.takeInvocation();
-  Clang.takeDiagnosticClient();
   Clang.takeCodeCompletionConsumer();
   CCInvocation.getLangOpts().SpellChecking = SpellChecking;
 }
@@ -1796,7 +1793,8 @@ bool ASTUnit::Save(llvm::StringRef File) {
   Writer.WritePCH(getSema(), 0, 0);
   
   // Write the generated bitstream to "Out".
-  Out.write((char *)&Buffer.front(), Buffer.size());  
+  if (!Buffer.empty())
+    Out.write((char *)&Buffer.front(), Buffer.size());  
   Out.close();
   return Out.has_error();
 }
diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index aaa67804ce..5db612542f 100644
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -56,10 +56,6 @@ void CompilerInstance::setDiagnostics(Diagnostic *Value) {
   Diagnostics = Value;
 }
 
-void CompilerInstance::setDiagnosticClient(DiagnosticClient *Value) {
-  DiagClient.reset(Value);
-}
-
 void CompilerInstance::setTarget(TargetInfo *Value) {
   Target.reset(Value);
 }
@@ -131,14 +127,11 @@ static void SetUpBuildDumpLog(const DiagnosticOptions &DiagOpts,
   // Chain in a diagnostic client which will log the diagnostics.
   DiagnosticClient *Logger =
     new TextDiagnosticPrinter(*OS.take(), DiagOpts, /*OwnsOutputStream=*/true);
-  Diags.setClient(new ChainedDiagnosticClient(Diags.getClient(), Logger));
+  Diags.setClient(new ChainedDiagnosticClient(Diags.takeClient(), Logger));
 }
 
 void CompilerInstance::createDiagnostics(int Argc, char **Argv) {
   Diagnostics = createDiagnostics(getDiagnosticOpts(), Argc, Argv);
-
-  if (Diagnostics)
-    DiagClient.reset(Diagnostics->getClient());
 }
 
 llvm::IntrusiveRefCntPtr<Diagnostic> 
@@ -155,22 +148,20 @@ CompilerInstance::createDiagnostics(const DiagnosticOptions &Opts,
       // bit of a problem. So, just create a text diagnostic printer
       // to complain about this problem, and pretend that the user
       // didn't try to use binary output.
-      DiagClient.reset(new TextDiagnosticPrinter(llvm::errs(), Opts));
-      Diags->setClient(DiagClient.take());
+      Diags->setClient(new TextDiagnosticPrinter(llvm::errs(), Opts));
       Diags->Report(diag::err_fe_stderr_binary);
       return Diags;
     } else {
-      DiagClient.reset(new BinaryDiagnosticSerializer(llvm::errs()));
+      Diags->setClient(new BinaryDiagnosticSerializer(llvm::errs()));
     }
   } else {
-    DiagClient.reset(new TextDiagnosticPrinter(llvm::errs(), Opts));
+    Diags->setClient(new TextDiagnosticPrinter(llvm::errs(), Opts));
   }
 
   // Chain in -verify checker, if requested.
   if (Opts.VerifyDiagnostics)
-    DiagClient.reset(new VerifyDiagnosticsClient(*Diags, DiagClient.take()));
+    Diags->setClient(new VerifyDiagnosticsClient(*Diags, Diags->takeClient()));
 
-  Diags->setClient(DiagClient.take());
   if (!Opts.DumpBuildInformation.empty())
     SetUpBuildDumpLog(Opts, Argc, Argv, *Diags);
 
diff --git a/lib/Frontend/VerifyDiagnosticsClient.cpp b/lib/Frontend/VerifyDiagnosticsClient.cpp
index ae36481444..c7c84c1a5d 100644
--- a/lib/Frontend/VerifyDiagnosticsClient.cpp
+++ b/lib/Frontend/VerifyDiagnosticsClient.cpp
@@ -484,7 +484,7 @@ void VerifyDiagnosticsClient::CheckDiagnostics() {
   ExpectedData ED;
 
   // Ensure any diagnostics go to the primary client.
-  DiagnosticClient *CurClient = Diags.getClient();
+  DiagnosticClient *CurClient = Diags.takeClient();
   Diags.setClient(PrimaryClient.get());
 
   // If we have a preprocessor, scan the source for expected diagnostic
@@ -507,6 +507,7 @@ void VerifyDiagnosticsClient::CheckDiagnostics() {
                                "note", false));
   }
 
+  Diags.takeClient();
   Diags.setClient(CurClient);
 
   // Reset the buffer, we have processed all the diagnostics in it.