Index: include/clang/Basic/Diagnostic.h =================================================================== --- include/clang/Basic/Diagnostic.h +++ include/clang/Basic/Diagnostic.h @@ -187,7 +187,7 @@ IntrusiveRefCntPtr Diags; IntrusiveRefCntPtr DiagOpts; DiagnosticConsumer *Client; - bool OwnsDiagClient; + std::unique_ptr OwningClient; SourceManager *SourceMgr; /// \brief Mapping information for diagnostics. @@ -347,7 +347,6 @@ DiagnosticOptions *DiagOpts, DiagnosticConsumer *client = nullptr, bool ShouldOwnClient = true); - ~DiagnosticsEngine(); const IntrusiveRefCntPtr &getDiagnosticIDs() const { return Diags; @@ -368,13 +367,12 @@ const DiagnosticConsumer *getClient() const { return Client; } /// \brief Determine whether this \c DiagnosticsEngine object own its client. - bool ownsClient() const { return OwnsDiagClient; } - + bool ownsClient() const { return !!OwningClient; } + /// \brief Return the current diagnostic client along with ownership of that /// client. - DiagnosticConsumer *takeClient() { - OwnsDiagClient = false; - return Client; + std::unique_ptr takeClient() { + return std::move(OwningClient); } bool hasSourceManager() const { return SourceMgr != nullptr; } Index: include/clang/Frontend/VerifyDiagnosticConsumer.h =================================================================== --- include/clang/Frontend/VerifyDiagnosticConsumer.h +++ include/clang/Frontend/VerifyDiagnosticConsumer.h @@ -212,7 +212,7 @@ private: DiagnosticsEngine &Diags; DiagnosticConsumer *PrimaryClient; - bool OwnsPrimaryClient; + std::unique_ptr OwningPrimaryClient; std::unique_ptr Buffer; const Preprocessor *CurrentPreprocessor; const LangOptions *LangOpts; Index: include/clang/Rewrite/Frontend/FixItRewriter.h =================================================================== --- include/clang/Rewrite/Frontend/FixItRewriter.h +++ include/clang/Rewrite/Frontend/FixItRewriter.h @@ -66,7 +66,7 @@ /// \brief The diagnostic client that performs the actual formatting /// of error messages. DiagnosticConsumer *Client; - bool OwnsClient; + std::unique_ptr OwningClient; /// \brief Turn an input path into an output path. NULL implies overwriting /// the original. Index: lib/Basic/Diagnostic.cpp =================================================================== --- lib/Basic/Diagnostic.cpp +++ lib/Basic/Diagnostic.cpp @@ -33,13 +33,11 @@ Output.append(Str.begin(), Str.end()); } - DiagnosticsEngine::DiagnosticsEngine( - const IntrusiveRefCntPtr &diags, - DiagnosticOptions *DiagOpts, - DiagnosticConsumer *client, bool ShouldOwnClient) - : Diags(diags), DiagOpts(DiagOpts), Client(client), - OwnsDiagClient(ShouldOwnClient), SourceMgr(nullptr) { + const IntrusiveRefCntPtr &diags, DiagnosticOptions *DiagOpts, + DiagnosticConsumer *client, bool ShouldOwnClient) + : Diags(diags), DiagOpts(DiagOpts), Client(nullptr), SourceMgr(nullptr) { + setClient(client, ShouldOwnClient); ArgToStringFn = DummyArgToStringFn; ArgToStringCookie = nullptr; @@ -63,18 +61,10 @@ Reset(); } -DiagnosticsEngine::~DiagnosticsEngine() { - if (OwnsDiagClient) - delete Client; -} - void DiagnosticsEngine::setClient(DiagnosticConsumer *client, bool ShouldOwnClient) { - if (OwnsDiagClient && Client) - delete Client; - + OwningClient.reset(ShouldOwnClient ? client : nullptr); Client = client; - OwnsDiagClient = ShouldOwnClient; } void DiagnosticsEngine::pushMappings(SourceLocation Loc) { Index: lib/Frontend/ASTUnit.cpp =================================================================== --- lib/Frontend/ASTUnit.cpp +++ lib/Frontend/ASTUnit.cpp @@ -589,6 +589,7 @@ DiagnosticsEngine &Diags; StoredDiagnosticConsumer Client; DiagnosticConsumer *PreviousClient; + std::unique_ptr OwningPreviousClient; public: CaptureDroppedDiagnostics(bool RequestCapture, DiagnosticsEngine &Diags, @@ -596,16 +597,15 @@ : Diags(Diags), Client(StoredDiags), PreviousClient(nullptr) { if (RequestCapture || Diags.getClient() == nullptr) { - PreviousClient = Diags.takeClient(); - Diags.setClient(&Client); + OwningPreviousClient = Diags.takeClient(); + PreviousClient = Diags.getClient(); + Diags.setClient(&Client, false); } } ~CaptureDroppedDiagnostics() { - if (Diags.getClient() == &Client) { - Diags.takeClient(); - Diags.setClient(PreviousClient); - } + if (Diags.getClient() == &Client) + Diags.setClient(PreviousClient, !!OwningPreviousClient.release()); } }; Index: lib/Frontend/CompilerInstance.cpp =================================================================== --- lib/Frontend/CompilerInstance.cpp +++ lib/Frontend/CompilerInstance.cpp @@ -159,9 +159,8 @@ if (CodeGenOpts) Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags); assert(Diags.ownsClient()); - Diags.setClient(new ChainedDiagnosticConsumer( - std::unique_ptr(Diags.takeClient()), - std::move(Logger))); + Diags.setClient( + new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger))); } static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts, @@ -172,8 +171,7 @@ if (Diags.ownsClient()) { Diags.setClient(new ChainedDiagnosticConsumer( - std::unique_ptr(Diags.takeClient()), - std::move(SerializedConsumer))); + Diags.takeClient(), std::move(SerializedConsumer))); } else { Diags.setClient(new ChainedDiagnosticConsumer( Diags.getClient(), std::move(SerializedConsumer))); Index: lib/Frontend/Rewrite/FixItRewriter.cpp =================================================================== --- lib/Frontend/Rewrite/FixItRewriter.cpp +++ lib/Frontend/Rewrite/FixItRewriter.cpp @@ -36,14 +36,13 @@ FixItOpts(FixItOpts), NumFailures(0), PrevDiagSilenced(false) { - OwnsClient = Diags.ownsClient(); - Client = Diags.takeClient(); - Diags.setClient(this); + OwningClient = Diags.takeClient(); + Client = Diags.getClient(); + Diags.setClient(this, false); } FixItRewriter::~FixItRewriter() { - Diags.takeClient(); - Diags.setClient(Client, OwnsClient); + Diags.setClient(Client, !!OwningClient.release()); } bool FixItRewriter::WriteFixedFile(FileID ID, raw_ostream &OS) { @@ -188,12 +187,10 @@ // 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.setClient(Client, false); Diags.Clear(); Diags.Report(Loc, DiagID); - Diags.takeClient(); - Diags.setClient(this); + Diags.setClient(this, false); } FixItOptions::~FixItOptions() {} Index: lib/Frontend/VerifyDiagnosticConsumer.cpp =================================================================== --- lib/Frontend/VerifyDiagnosticConsumer.cpp +++ lib/Frontend/VerifyDiagnosticConsumer.cpp @@ -29,12 +29,11 @@ VerifyDiagnosticConsumer::VerifyDiagnosticConsumer(DiagnosticsEngine &Diags_) : Diags(Diags_), - PrimaryClient(Diags.getClient()), OwnsPrimaryClient(Diags.ownsClient()), + PrimaryClient(Diags.getClient()), OwningPrimaryClient(Diags.takeClient()), Buffer(new TextDiagnosticBuffer()), CurrentPreprocessor(nullptr), LangOpts(nullptr), SrcManager(nullptr), ActiveSourceFiles(0), Status(HasNoDirectives) { - Diags.takeClient(); if (Diags.hasSourceManager()) setSourceManager(Diags.getSourceManager()); } @@ -43,10 +42,8 @@ assert(!ActiveSourceFiles && "Incomplete parsing of source files!"); assert(!CurrentPreprocessor && "CurrentPreprocessor should be invalid!"); SrcManager = nullptr; - CheckDiagnostics(); - Diags.takeClient(); - if (OwnsPrimaryClient) - delete PrimaryClient; + CheckDiagnostics(); + Diags.takeClient().release(); } #ifndef NDEBUG @@ -802,8 +799,8 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() { // Ensure any diagnostics go to the primary client. - bool OwnsCurClient = Diags.ownsClient(); - DiagnosticConsumer *CurClient = Diags.takeClient(); + std::unique_ptr OwningCurClient = Diags.takeClient(); + DiagnosticConsumer *CurClient = Diags.getClient(); Diags.setClient(PrimaryClient, false); #ifndef NDEBUG @@ -865,8 +862,7 @@ Buffer->note_end(), "note")); } - Diags.takeClient(); - Diags.setClient(CurClient, OwnsCurClient); + Diags.setClient(CurClient, !!OwningCurClient.release()); // Reset the buffer, we have processed all the diagnostics in it. Buffer.reset(new TextDiagnosticBuffer()); Index: tools/driver/driver.cpp =================================================================== --- tools/driver/driver.cpp +++ tools/driver/driver.cpp @@ -451,8 +451,7 @@ clang::serialized_diags::create(DiagOpts->DiagnosticSerializationFile, &*DiagOpts, /*MergeChildRecords=*/true); Diags.setClient(new ChainedDiagnosticConsumer( - std::unique_ptr(Diags.takeClient()), - std::move(SerializedConsumer))); + Diags.takeClient(), std::move(SerializedConsumer))); } ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false); Index: unittests/Sema/ExternalSemaSourceTest.cpp =================================================================== --- unittests/Sema/ExternalSemaSourceTest.cpp +++ unittests/Sema/ExternalSemaSourceTest.cpp @@ -154,7 +154,7 @@ DiagnosticsEngine &Diagnostics = CI.getDiagnostics(); DiagnosticConsumer *Client = Diagnostics.getClient(); if (Diagnostics.ownsClient()) - OwnedClient.reset(Diagnostics.takeClient()); + OwnedClient = Diagnostics.takeClient(); for (size_t I = 0, E = Watchers.size(); I < E; ++I) Client = Watchers[I]->Chain(Client); Diagnostics.setClient(Client, false);