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.

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

llvm-svn: 224454

clang/include/clang/Basic/Diagnostic.h

@@ -347,6 +347,7 @@ public:
                       DiagnosticOptions *DiagOpts,
                       DiagnosticConsumer *client = nullptr,
                       bool ShouldOwnClient = true);
+  ~DiagnosticsEngine();
 
   const IntrusiveRefCntPtr<DiagnosticIDs> &getDiagnosticIDs() const {
     return Diags;

clang/lib/Basic/Diagnostic.cpp

@@ -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);

clang/test/Frontend/verify-unknown-arg.c

@@ -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'