[cfe-commits] r111437 - in /cfe/trunk: include/clang/Basic/Diagnostic.h include/clang/Frontend/ASTUnit.h include/clang/Frontend/CompilerInstance.h lib/Frontend/ASTUnit.cpp lib/Frontend/CompilerInstance.cpp lib/Frontend/VerifyDiagnosticsClient.cpp lib/Rewrite/FixItRewriter.cpp tools/driver/cc1_main.cpp tools/driver/cc1as_main.cpp tools/driver/driver.cpp

Douglas Gregor dgregor at apple.com
Wed Aug 18 15:29:44 PDT 2010


Author: dgregor
Date: Wed Aug 18 17:29:43 2010
New Revision: 111437

URL: http://llvm.org/viewvc/llvm-project?rev=111437&view=rev
Log:
Simplify the ownership model for DiagnosticClients, which was really
convoluted and a bit leaky. Now, the Diagnostic object owns its
DiagnosticClient.

Modified:
    cfe/trunk/include/clang/Basic/Diagnostic.h
    cfe/trunk/include/clang/Frontend/ASTUnit.h
    cfe/trunk/include/clang/Frontend/CompilerInstance.h
    cfe/trunk/lib/Frontend/ASTUnit.cpp
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/lib/Frontend/VerifyDiagnosticsClient.cpp
    cfe/trunk/lib/Rewrite/FixItRewriter.cpp
    cfe/trunk/tools/driver/cc1_main.cpp
    cfe/trunk/tools/driver/cc1as_main.cpp
    cfe/trunk/tools/driver/driver.cpp

Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Wed Aug 18 17:29:43 2010
@@ -16,6 +16,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/OwningPtr.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/type_traits.h"
 #include <string>
@@ -202,8 +203,8 @@
   unsigned TemplateBacktraceLimit; // Cap on depth of template backtrace stack,
                                    // 0 -> no limit.
   ExtensionHandling ExtBehavior; // Map extensions onto warnings or errors?
-  DiagnosticClient *Client;
-
+  llvm::OwningPtr<DiagnosticClient> Client;
+  
   /// DiagMappings - Mapping information for diagnostics.  Mapping info is
   /// packed into four bits per diagnostic.  The low three bits are the mapping
   /// (an instance of diag::Mapping), or zero if unset.  The high bit is set
@@ -285,8 +286,12 @@
   //  Diagnostic characterization methods, used by a client to customize how
   //
 
-  DiagnosticClient *getClient() { return Client; }
-  const DiagnosticClient *getClient() const { return Client; }
+  DiagnosticClient *getClient() { return Client.get(); }
+  const DiagnosticClient *getClient() const { return Client.get(); }
+  
+  /// \brief Return the current diagnostic client along with ownership of that
+  /// client.
+  DiagnosticClient *takeClient() { return Client.take(); }
 
   /// pushMappings - Copies the current DiagMappings and pushes the new copy
   /// onto the top of the stack.
@@ -298,7 +303,10 @@
   /// stack.
   bool popMappings();
 
-  void setClient(DiagnosticClient* client) { Client = client; }
+  /// \brief Set the diagnostic client associated with this diagnostic object.
+  ///
+  /// The diagnostic object takes ownership of \c client.
+  void setClient(DiagnosticClient* client) { Client.reset(client); }
 
   /// setErrorLimit - Specify a limit for the number of errors we should
   /// emit before giving up.  Zero disables the limit.

Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Wed Aug 18 17:29:43 2010
@@ -59,6 +59,7 @@
 public:
   typedef std::map<FileID, std::vector<PreprocessedEntity *> > 
     PreprocessedEntitiesByFileMap;
+  
 private:
   llvm::IntrusiveRefCntPtr<Diagnostic> Diagnostics;
   llvm::OwningPtr<FileManager>      FileMgr;

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Wed Aug 18 17:29:43 2010
@@ -68,9 +68,6 @@
   /// The diagnostics engine instance.
   llvm::IntrusiveRefCntPtr<Diagnostic> Diagnostics;
 
-  /// The diagnostics client instance.
-  llvm::OwningPtr<DiagnosticClient> DiagClient;
-
   /// The target being compiled for.
   llvm::OwningPtr<TargetInfo> Target;
 
@@ -266,18 +263,11 @@
   void setDiagnostics(Diagnostic *Value);
 
   DiagnosticClient &getDiagnosticClient() const {
-    assert(DiagClient && "Compiler instance has no diagnostic client!");
-    return *DiagClient;
+    assert(Diagnostics && Diagnostics->getClient() && 
+           "Compiler instance has no diagnostic client!");
+    return *Diagnostics->getClient();
   }
 
-  /// takeDiagnosticClient - Remove the current diagnostics client and give
-  /// ownership to the caller.
-  DiagnosticClient *takeDiagnosticClient() { return DiagClient.take(); }
-
-  /// setDiagnosticClient - Replace the current diagnostics client; the compiler
-  /// instance takes ownership of \arg Value.
-  void setDiagnosticClient(DiagnosticClient *Value);
-
   /// }
   /// @name Target Info
   /// {

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Wed Aug 18 17:29:43 2010
@@ -371,14 +371,19 @@
 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 @@
   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 @@
     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 @@
   
   Clang.takeSourceManager();
   Clang.takeFileManager();
-  Clang.takeDiagnosticClient();
   Invocation.reset(Clang.takeInvocation());
   return true;
 }
@@ -1123,13 +1122,11 @@
   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 @@
   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 @@
   
   Act->Execute();
   Act->EndSourceFile();
-  Clang.takeDiagnosticClient();
   Clang.takeInvocation();
   
   if (Diagnostics->hasErrorOccurred()) {
@@ -1321,11 +1316,14 @@
                                       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 @@
   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 @@
   Clang.takeFileManager();
   Clang.takeSourceManager();
   Clang.takeInvocation();
-  Clang.takeDiagnosticClient();
   Clang.takeCodeCompletionConsumer();
   CCInvocation.getLangOpts().SpellChecking = SpellChecking;
 }
@@ -1796,7 +1793,8 @@
   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();
 }

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Aug 18 17:29:43 2010
@@ -56,10 +56,6 @@
   Diagnostics = Value;
 }
 
-void CompilerInstance::setDiagnosticClient(DiagnosticClient *Value) {
-  DiagClient.reset(Value);
-}
-
 void CompilerInstance::setTarget(TargetInfo *Value) {
   Target.reset(Value);
 }
@@ -131,14 +127,11 @@
   // 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 @@
       // 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);
 

Modified: cfe/trunk/lib/Frontend/VerifyDiagnosticsClient.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/VerifyDiagnosticsClient.cpp?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/VerifyDiagnosticsClient.cpp (original)
+++ cfe/trunk/lib/Frontend/VerifyDiagnosticsClient.cpp Wed Aug 18 17:29:43 2010
@@ -484,7 +484,7 @@
   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 @@
                                "note", false));
   }
 
+  Diags.takeClient();
   Diags.setClient(CurClient);
 
   // Reset the buffer, we have processed all the diagnostics in it.

Modified: cfe/trunk/lib/Rewrite/FixItRewriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Rewrite/FixItRewriter.cpp?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/lib/Rewrite/FixItRewriter.cpp (original)
+++ cfe/trunk/lib/Rewrite/FixItRewriter.cpp Wed Aug 18 17:29:43 2010
@@ -32,11 +32,12 @@
     Rewrite(SourceMgr, LangOpts),
     FixItOpts(FixItOpts),
     NumFailures(0) {
-  Client = Diags.getClient();
+  Client = Diags.takeClient();
   Diags.setClient(this);
 }
 
 FixItRewriter::~FixItRewriter() {
+  Diags.takeClient();
   Diags.setClient(Client);
 }
 
@@ -144,9 +145,11 @@
   // When producing this diagnostic, we temporarily bypass ourselves,
   // clear out any current diagnostic, and let the downstream client
   // format the diagnostic.
+  Diags.takeClient();
   Diags.setClient(Client);
   Diags.Clear();
   Diags.Report(Loc, DiagID);
+  Diags.takeClient();
   Diags.setClient(this);
 }
 

Modified: cfe/trunk/tools/driver/cc1_main.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1_main.cpp?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/tools/driver/cc1_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1_main.cpp Wed Aug 18 17:29:43 2010
@@ -121,8 +121,8 @@
 
   // Run clang -cc1 test.
   if (ArgBegin != ArgEnd && llvm::StringRef(ArgBegin[0]) == "-cc1test") {
-    TextDiagnosticPrinter DiagClient(llvm::errs(), DiagnosticOptions());
-    Diagnostic Diags(&DiagClient);
+    Diagnostic Diags(new TextDiagnosticPrinter(llvm::errs(), 
+                                               DiagnosticOptions()));
     return cc1_test(Diags, ArgBegin + 1, ArgEnd);
   }
 
@@ -133,8 +133,8 @@
 
   // Buffer diagnostics from argument parsing so that we can output them using a
   // well formed diagnostic object.
-  TextDiagnosticBuffer DiagsBuffer;
-  Diagnostic Diags(&DiagsBuffer);
+  TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
+  Diagnostic Diags(DiagsBuffer);
   CompilerInvocation::CreateFromArgs(Clang->getInvocation(), ArgBegin, ArgEnd,
                                      Diags);
 
@@ -154,7 +154,7 @@
   llvm::install_fatal_error_handler(LLVMErrorHandler,
                                   static_cast<void*>(&Clang->getDiagnostics()));
 
-  DiagsBuffer.FlushDiagnostics(Clang->getDiagnostics());
+  DiagsBuffer->FlushDiagnostics(Clang->getDiagnostics());
 
   // Execute the frontend actions.
   bool Success = ExecuteCompilerInvocation(Clang.get());

Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Wed Aug 18 17:29:43 2010
@@ -321,9 +321,10 @@
   InitializeAllAsmParsers();
 
   // Construct our diagnostic client.
-  TextDiagnosticPrinter DiagClient(errs(), DiagnosticOptions());
-  DiagClient.setPrefix("clang -cc1as");
-  Diagnostic Diags(&DiagClient);
+  TextDiagnosticPrinter *DiagClient
+    = new TextDiagnosticPrinter(errs(), DiagnosticOptions());
+  DiagClient->setPrefix("clang -cc1as");
+  Diagnostic Diags(DiagClient);
 
   // Set an error handler, so that any LLVM backend diagnostics go through our
   // error handler.

Modified: cfe/trunk/tools/driver/driver.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=111437&r1=111436&r2=111437&view=diff
==============================================================================
--- cfe/trunk/tools/driver/driver.cpp (original)
+++ cfe/trunk/tools/driver/driver.cpp Wed Aug 18 17:29:43 2010
@@ -285,10 +285,10 @@
 
   llvm::sys::Path Path = GetExecutablePath(argv[0], CanonicalPrefixes);
 
-  TextDiagnosticPrinter DiagClient(llvm::errs(), DiagnosticOptions());
-  DiagClient.setPrefix(Path.getBasename());
-
-  Diagnostic Diags(&DiagClient);
+  TextDiagnosticPrinter *DiagClient
+    = new TextDiagnosticPrinter(llvm::errs(), DiagnosticOptions());
+  DiagClient->setPrefix(Path.getBasename());
+  Diagnostic Diags(DiagClient);
 
 #ifdef CLANG_IS_PRODUCTION
   const bool IsProduction = true;





More information about the cfe-commits mailing list