From 41c247a677f0dafaa13fc05a028275334d5df779 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Mon, 17 Nov 2014 23:46:02 +0000 Subject: [PATCH] Make DiagnosticsEngine::takeClient return std::unique_ptr<> Summary: Make DiagnosticsEngine::takeClient return std::unique_ptr<>. Updated callers to store conditional ownership using a pair of pointer and unique_ptr instead of a pointer + bool. Updated code that temporarily registers clients to use the non-owning registration (+ removed extra calls to takeClient). Reviewers: dblaikie Reviewed By: dblaikie Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D6294 llvm-svn: 222193 --- clang/include/clang/Basic/Diagnostic.h | 12 ++++-------- .../clang/Frontend/VerifyDiagnosticConsumer.h | 2 +- clang/include/clang/Rewrite/Frontend/FixItRewriter.h | 2 +- clang/lib/Basic/Diagnostic.cpp | 20 +++++--------------- clang/lib/Frontend/ASTUnit.cpp | 12 ++++++------ clang/lib/Frontend/CompilerInstance.cpp | 8 +++----- clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 15 ++++++--------- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp | 16 ++++++---------- clang/tools/driver/driver.cpp | 3 +-- clang/unittests/Sema/ExternalSemaSourceTest.cpp | 2 +- 10 files changed, 34 insertions(+), 58 deletions(-) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 556219c..31a29fa 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -187,7 +187,7 @@ private: IntrusiveRefCntPtr Diags; IntrusiveRefCntPtr DiagOpts; DiagnosticConsumer *Client; - bool OwnsDiagClient; + std::unique_ptr Owner; SourceManager *SourceMgr; /// \brief Mapping information for diagnostics. @@ -347,7 +347,6 @@ public: DiagnosticOptions *DiagOpts, DiagnosticConsumer *client = nullptr, bool ShouldOwnClient = true); - ~DiagnosticsEngine(); const IntrusiveRefCntPtr &getDiagnosticIDs() const { return Diags; @@ -368,14 +367,11 @@ public: 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 Owner != nullptr; } + /// \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(Owner); } bool hasSourceManager() const { return SourceMgr != nullptr; } SourceManager &getSourceManager() const { diff --git a/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h b/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h index 9d94e78..80e140b 100644 --- a/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h +++ b/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h @@ -212,7 +212,7 @@ public: private: DiagnosticsEngine &Diags; DiagnosticConsumer *PrimaryClient; - bool OwnsPrimaryClient; + std::unique_ptr PrimaryClientOwner; std::unique_ptr Buffer; const Preprocessor *CurrentPreprocessor; const LangOptions *LangOpts; diff --git a/clang/include/clang/Rewrite/Frontend/FixItRewriter.h b/clang/include/clang/Rewrite/Frontend/FixItRewriter.h index f91093b..5994172 100644 --- a/clang/include/clang/Rewrite/Frontend/FixItRewriter.h +++ b/clang/include/clang/Rewrite/Frontend/FixItRewriter.h @@ -66,7 +66,7 @@ class FixItRewriter : public DiagnosticConsumer { /// \brief The diagnostic client that performs the actual formatting /// of error messages. DiagnosticConsumer *Client; - bool OwnsClient; + std::unique_ptr Owner; /// \brief Turn an input path into an output path. NULL implies overwriting /// the original. diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index 59a75df..5c066ef 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -33,13 +33,11 @@ static void DummyArgToStringFn(DiagnosticsEngine::ArgumentKind AK, intptr_t QT, 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 @@ DiagnosticsEngine::DiagnosticsEngine( Reset(); } -DiagnosticsEngine::~DiagnosticsEngine() { - if (OwnsDiagClient) - delete Client; -} - void DiagnosticsEngine::setClient(DiagnosticConsumer *client, bool ShouldOwnClient) { - if (OwnsDiagClient && Client) - delete Client; - + Owner.reset(ShouldOwnClient ? client : nullptr); Client = client; - OwnsDiagClient = ShouldOwnClient; } void DiagnosticsEngine::pushMappings(SourceLocation Loc) { diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index c7d3d3f..a3998fa 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -589,6 +589,7 @@ class CaptureDroppedDiagnostics { DiagnosticsEngine &Diags; StoredDiagnosticConsumer Client; DiagnosticConsumer *PreviousClient; + std::unique_ptr OwningPreviousClient; public: CaptureDroppedDiagnostics(bool RequestCapture, DiagnosticsEngine &Diags, @@ -596,16 +597,15 @@ public: : 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()); } }; diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index ac9de48..b059965 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -159,9 +159,8 @@ static void SetUpDiagnosticLog(DiagnosticOptions *DiagOpts, 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 @@ static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts, 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))); diff --git a/clang/lib/Frontend/Rewrite/FixItRewriter.cpp b/clang/lib/Frontend/Rewrite/FixItRewriter.cpp index f177623..a3e14f9 100644 --- a/clang/lib/Frontend/Rewrite/FixItRewriter.cpp +++ b/clang/lib/Frontend/Rewrite/FixItRewriter.cpp @@ -36,14 +36,13 @@ FixItRewriter::FixItRewriter(DiagnosticsEngine &Diags, SourceManager &SourceMgr, FixItOpts(FixItOpts), NumFailures(0), PrevDiagSilenced(false) { - OwnsClient = Diags.ownsClient(); - Client = Diags.takeClient(); - Diags.setClient(this); + Owner = Diags.takeClient(); + Client = Diags.getClient(); + Diags.setClient(this, false); } FixItRewriter::~FixItRewriter() { - Diags.takeClient(); - Diags.setClient(Client, OwnsClient); + Diags.setClient(Client, Owner.release() != nullptr); } bool FixItRewriter::WriteFixedFile(FileID ID, raw_ostream &OS) { @@ -188,12 +187,10 @@ void FixItRewriter::Diag(SourceLocation 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.setClient(Client, false); Diags.Clear(); Diags.Report(Loc, DiagID); - Diags.takeClient(); - Diags.setClient(this); + Diags.setClient(this, false); } FixItOptions::~FixItOptions() {} diff --git a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp index 531deb0..3ff6b18 100644 --- a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp +++ b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp @@ -29,12 +29,11 @@ typedef VerifyDiagnosticConsumer::ExpectedData ExpectedData; VerifyDiagnosticConsumer::VerifyDiagnosticConsumer(DiagnosticsEngine &Diags_) : Diags(Diags_), - PrimaryClient(Diags.getClient()), OwnsPrimaryClient(Diags.ownsClient()), + PrimaryClient(Diags.getClient()), PrimaryClientOwner(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 @@ VerifyDiagnosticConsumer::~VerifyDiagnosticConsumer() { 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::UpdateParsedFileStatus(SourceManager &SM, void VerifyDiagnosticConsumer::CheckDiagnostics() { // Ensure any diagnostics go to the primary client. - bool OwnsCurClient = Diags.ownsClient(); - DiagnosticConsumer *CurClient = Diags.takeClient(); + DiagnosticConsumer *CurClient = Diags.getClient(); + std::unique_ptr Owner = Diags.takeClient(); Diags.setClient(PrimaryClient, false); #ifndef NDEBUG @@ -865,8 +862,7 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() { Buffer->note_end(), "note")); } - Diags.takeClient(); - Diags.setClient(CurClient, OwnsCurClient); + Diags.setClient(CurClient, Owner.release() != nullptr); // Reset the buffer, we have processed all the diagnostics in it. Buffer.reset(new TextDiagnosticBuffer()); diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 57db026..e93fa80 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -451,8 +451,7 @@ int main(int argc_, const char **argv_) { 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); diff --git a/clang/unittests/Sema/ExternalSemaSourceTest.cpp b/clang/unittests/Sema/ExternalSemaSourceTest.cpp index 4291b76..3a93fc7 100644 --- a/clang/unittests/Sema/ExternalSemaSourceTest.cpp +++ b/clang/unittests/Sema/ExternalSemaSourceTest.cpp @@ -154,7 +154,7 @@ protected: 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); -- 2.7.4