r224454 - Destroy the diagnostic client first in ~DiagnosticEngine

[David Blaikie]

On Wed, Dec 17, 2014 at 12:23 PM, Reid Kleckner <reid at kleckner.net> wrote:
>
> Author: rnk
> Date: Wed Dec 17 14:23:11 2014
> New Revision: 224454
>
> URL: http://llvm.org/viewvc/llvm-project?rev=224454&view=rev
> Log:
> Destroy the diagnostic client first in ~DiagnosticEngine
>
> Add a comment and a test to ~DiagnosticEngine about the ordering
> requirements on the teardown of DiagnosticConsumer. This could also be
> accomplished by rearranging the fields of ~DiagnosticEngine, but I felt
> that this was a better, more explicit solution.
>

Your call - though I'd personally vote for the member reordering (& just
put the comment there, explaining what constraint on the order was
required).


> This fixes PR21911, an issue that occurred after the unique_ptr
> migration in r222193.
>

Thanks for the fix, in any case!


>
> Added:
>     cfe/trunk/test/Frontend/verify-unknown-arg.c
> Modified:
>     cfe/trunk/include/clang/Basic/Diagnostic.h
>     cfe/trunk/lib/Basic/Diagnostic.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=224454&r1=224453&r2=224454&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
> +++ cfe/trunk/include/clang/Basic/Diagnostic.h Wed Dec 17 14:23:11 2014
> @@ -347,6 +347,7 @@ public:
>                        DiagnosticOptions *DiagOpts,
>                        DiagnosticConsumer *client = nullptr,
>                        bool ShouldOwnClient = true);
> +  ~DiagnosticsEngine();
>
>    const IntrusiveRefCntPtr<DiagnosticIDs> &getDiagnosticIDs() const {
>      return Diags;
>
> Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=224454&r1=224453&r2=224454&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
> +++ cfe/trunk/lib/Basic/Diagnostic.cpp Wed Dec 17 14:23:11 2014
> @@ -61,6 +61,12 @@ DiagnosticsEngine::DiagnosticsEngine(
>    Reset();
>  }
>
> +DiagnosticsEngine::~DiagnosticsEngine() {
> +  // If we own the diagnostic client, destroy it first so that it can
> access the
> +  // engine from its destructor.
> +  setClient(nullptr);
> +}
> +
>  void DiagnosticsEngine::setClient(DiagnosticConsumer *client,
>                                    bool ShouldOwnClient) {
>    Owner.reset(ShouldOwnClient ? client : nullptr);
>
> Added: cfe/trunk/test/Frontend/verify-unknown-arg.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/verify-unknown-arg.c?rev=224454&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Frontend/verify-unknown-arg.c (added)
> +++ cfe/trunk/test/Frontend/verify-unknown-arg.c Wed Dec 17 14:23:11 2014
> @@ -0,0 +1,6 @@
> +// RUN: not %clang_cc1 -asdf -verify %s 2>&1 | FileCheck %s
> +
> +// expected-no-diagnostics
> +
> +//      CHECK: error: 'error' diagnostics seen but not expected:
> +// CHECK-NEXT: (frontend): unknown argument: '-asdf'
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141219/78482ffe/attachment.html>