From: Douglas Gregor Date: Wed, 18 Aug 2010 22:29:43 +0000 (+0000) Subject: Simplify the ownership model for DiagnosticClients, which was really X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bdbb004f38978da0c4a75af3294d1c7b5ff84af1;p=clang 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 --- diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index 6eafa03b97..37d26947c3 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -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 @@ -202,8 +203,8 @@ private: unsigned TemplateBacktraceLimit; // Cap on depth of template backtrace stack, // 0 -> no limit. ExtensionHandling ExtBehavior; // Map extensions onto warnings or errors? - DiagnosticClient *Client; - + llvm::OwningPtr 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 @@ public: // 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 @@ public: /// 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. diff --git a/include/clang/Frontend/ASTUnit.h b/include/clang/Frontend/ASTUnit.h index de10611314..c05fee1fc8 100644 --- a/include/clang/Frontend/ASTUnit.h +++ b/include/clang/Frontend/ASTUnit.h @@ -59,6 +59,7 @@ class ASTUnit { public: typedef std::map > PreprocessedEntitiesByFileMap; + private: llvm::IntrusiveRefCntPtr Diagnostics; llvm::OwningPtr FileMgr; diff --git a/include/clang/Frontend/CompilerInstance.h b/include/clang/Frontend/CompilerInstance.h index 2a0a7a7747..e09061dbbd 100644 --- a/include/clang/Frontend/CompilerInstance.h +++ b/include/clang/Frontend/CompilerInstance.h @@ -68,9 +68,6 @@ class CompilerInstance { /// The diagnostics engine instance. llvm::IntrusiveRefCntPtr Diagnostics; - /// The diagnostics client instance. - llvm::OwningPtr DiagClient; - /// The target being compiled for. llvm::OwningPtr Target; @@ -266,18 +263,11 @@ public: 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 /// { 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 &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 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 @@ -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. diff --git a/lib/Rewrite/FixItRewriter.cpp b/lib/Rewrite/FixItRewriter.cpp index 3fdec07459..582096978d 100644 --- a/lib/Rewrite/FixItRewriter.cpp +++ b/lib/Rewrite/FixItRewriter.cpp @@ -32,11 +32,12 @@ FixItRewriter::FixItRewriter(Diagnostic &Diags, SourceManager &SourceMgr, 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 @@ void FixItRewriter::Diag(FullSourceLoc Loc, unsigned DiagID) { // 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); } diff --git a/tools/driver/cc1_main.cpp b/tools/driver/cc1_main.cpp index bfdcefbb24..2cab8da24b 100644 --- a/tools/driver/cc1_main.cpp +++ b/tools/driver/cc1_main.cpp @@ -121,8 +121,8 @@ int cc1_main(const char **ArgBegin, const char **ArgEnd, // 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 @@ int cc1_main(const char **ArgBegin, const char **ArgEnd, // 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 @@ int cc1_main(const char **ArgBegin, const char **ArgEnd, llvm::install_fatal_error_handler(LLVMErrorHandler, static_cast(&Clang->getDiagnostics())); - DiagsBuffer.FlushDiagnostics(Clang->getDiagnostics()); + DiagsBuffer->FlushDiagnostics(Clang->getDiagnostics()); // Execute the frontend actions. bool Success = ExecuteCompilerInvocation(Clang.get()); diff --git a/tools/driver/cc1as_main.cpp b/tools/driver/cc1as_main.cpp index 4620b605fb..b8f2b68788 100644 --- a/tools/driver/cc1as_main.cpp +++ b/tools/driver/cc1as_main.cpp @@ -321,9 +321,10 @@ int cc1as_main(const char **ArgBegin, const char **ArgEnd, 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. diff --git a/tools/driver/driver.cpp b/tools/driver/driver.cpp index 0e99314e24..c058ece0dc 100644 --- a/tools/driver/driver.cpp +++ b/tools/driver/driver.cpp @@ -285,10 +285,10 @@ int main(int argc_, const char **argv_) { 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;