[clang-tidy] Untangle layering in ClangTidyDiagnosticConsumer somewhat. NFC

Summary:
Clang's hierarchy is CompilerInstance -> DiagnosticsEngine -> DiagnosticConsumer.
(Ownership is optional/shared, but this structure is fairly clear).

Currently ClangTidyDiagnosticConsumer *owns* the DiagnosticsEngine:
 - this inverts the hierarchy, which is confusing
 - this means ClangTidyDiagnosticConsumer() mutates the passed-in context, which
   is both surprising and limits flexibility
 - it's not possible to use a different DiagnosticsEngine with ClangTidy

This means a little bit more code in the places ClangTidy is used standalone,
but more flexibility in using ClangTidy with other diagnostics configurations.

Reviewers: hokein

Subscribers: xazax.hun, cfe-commits

Differential Revision: https://reviews.llvm.org/D54033

llvm-svn: 346418

Author: sam-mccall

clang-tools-extra/clang-tidy/ClangTidy.cpp

@@ -551,7 +551,9 @@ runClangTidy(clang::tidy::ClangTidyContext &Context,
   Context.setProfileStoragePrefix(StoreCheckProfile);
 
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
-
+  DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(),
+                       &DiagConsumer, /*ShouldOwnClient=*/false);
+  Context.setDiagnosticsEngine(&DE);
   Tool.setDiagnosticConsumer(&DiagConsumer);
 
   class ActionFactory : public FrontendActionFactory {

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

@@ -267,13 +267,7 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
     ClangTidyContext &Ctx, bool RemoveIncompatibleErrors)
     : Context(Ctx), RemoveIncompatibleErrors(RemoveIncompatibleErrors),
       LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
-      LastErrorWasIgnored(false) {
-  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
-  Diags = llvm::make_unique<DiagnosticsEngine>(
-      IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts, this,
-      /*ShouldOwnClient=*/false);
-  Context.DiagEngine = Diags.get();
-}
+      LastErrorWasIgnored(false) {}
 
 void ClangTidyDiagnosticConsumer::finalizeLastError() {
   if (!Errors.empty()) {
@@ -391,7 +385,7 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
 
   if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error &&
       DiagLevel != DiagnosticsEngine::Fatal &&
-      LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(),
+      LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(),
                                     Info.getLocation(), Info.getID(),
                                     Context)) {
     ++Context.Stats.ErrorsIgnoredNOLINT;
@@ -453,14 +447,14 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
       Errors.back());
   SmallString<100> Message;
   Info.FormatDiagnostic(Message);
-  FullSourceLoc Loc =
-      (Info.getLocation().isInvalid())
-          ? FullSourceLoc()
-          : FullSourceLoc(Info.getLocation(), Info.getSourceManager());
+  FullSourceLoc Loc;
+  if (Info.getLocation().isValid() && Info.hasSourceManager())
+    Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
   Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
                            Info.getFixItHints());
 
-  checkFilters(Info.getLocation());
+  if (Info.hasSourceManager())
+    checkFilters(Info.getLocation(), Info.getSourceManager());
 }
 
 bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName,
@@ -481,15 +475,15 @@ bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName,
   return false;
 }
 
-void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location) {
+void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
+                                               const SourceManager &Sources) {
   // Invalid location may mean a diagnostic in a command line, don't skip these.
   if (!Location.isValid()) {
     LastErrorRelatesToUserCode = true;
     LastErrorPassesLineFilter = true;
     return;
   }
 
-  const SourceManager &Sources = Diags->getSourceManager();
   if (!*Context.getOptions().SystemHeaders &&
       Sources.isInSystemHeader(Location))
     return;

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

@@ -102,6 +102,12 @@ class ClangTidyContext {
   /// \brief Initializes \c ClangTidyContext instance.
   ClangTidyContext(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
                    bool AllowEnablingAnalyzerAlphaCheckers = false);
+  /// Sets the DiagnosticsEngine that diag() will emit diagnostics to.
+  // FIXME: this is required initialization, and should be a constructor param.
+  // Fix the context -> diag engine -> consumer -> context initialization cycle.
+  void setDiagnosticsEngine(DiagnosticsEngine *DiagEngine) {
+    this->DiagEngine = DiagEngine;
+  }
 
   ~ClangTidyContext();
 
@@ -186,9 +192,8 @@ class ClangTidyContext {
   }
 
 private:
-  // Sets DiagEngine and Errors.
+  // Writes to Stats.
   friend class ClangTidyDiagnosticConsumer;
-  friend class ClangTidyPluginAction;
 
   DiagnosticsEngine *DiagEngine;
   std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
@@ -242,12 +247,11 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
 
   /// \brief Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter
   /// according to the diagnostic \p Location.
-  void checkFilters(SourceLocation Location);
+  void checkFilters(SourceLocation Location, const SourceManager& Sources);
   bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
 
   ClangTidyContext &Context;
   bool RemoveIncompatibleErrors;
-  std::unique_ptr<DiagnosticsEngine> Diags;
   std::vector<ClangTidyError> Errors;
   std::unique_ptr<llvm::Regex> HeaderFilter;
   bool LastErrorRelatesToUserCode;

clang-tools-extra/clang-tidy/plugin/ClangTidyPlugin.cpp

@@ -35,7 +35,7 @@ class ClangTidyPluginAction : public PluginASTAction {
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
                                                  StringRef File) override {
     // Insert the current diagnostics engine.
-    Context->DiagEngine = &Compiler.getDiagnostics();
+    Context->setDiagnosticsEngine(&Compiler.getDiagnostics());
 
     // Create the AST consumer.
     ClangTidyASTConsumerFactory Factory(*Context);

clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

@@ -83,6 +83,9 @@ runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
   ClangTidyContext Context(llvm::make_unique<DefaultOptionsProvider>(
       ClangTidyGlobalOptions(), Options));
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
+  DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions,
+                       &DiagConsumer, false);
+  Context.setDiagnosticsEngine(&DE);
 
   std::vector<std::string> Args(1, "clang-tidy");
   Args.push_back("-fsyntax-only");