Author: sammccall Date: Thu Nov 8 09:42:16 2018 New Revision: 346418 URL: http://llvm.org/viewvc/llvm-project?rev=346418&view=rev Log: [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 Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=346418&r1=346417&r2=346418&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Thu Nov 8 09:42:16 2018 @@ -551,7 +551,9 @@ runClangTidy(clang::tidy::ClangTidyConte 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 { Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=346418&r1=346417&r2=346418&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Thu Nov 8 09:42:16 2018 @@ -267,13 +267,7 @@ ClangTidyDiagnosticConsumer::ClangTidyDi 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::Handle 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::Handle 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,7 +475,8 @@ bool ClangTidyDiagnosticConsumer::passes 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; @@ -489,7 +484,6 @@ void ClangTidyDiagnosticConsumer::checkF return; } - const SourceManager &Sources = Diags->getSourceManager(); if (!*Context.getOptions().SystemHeaders && Sources.isInSystemHeader(Location)) return; Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h?rev=346418&r1=346417&r2=346418&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h (original) +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Thu Nov 8 09:42:16 2018 @@ -102,6 +102,12 @@ public: /// \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 @@ public: } 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 @@ private: /// \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; Modified: clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp?rev=346418&r1=346417&r2=346418&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp Thu Nov 8 09:42:16 2018 @@ -35,7 +35,7 @@ public: 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); Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=346418&r1=346417&r2=346418&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original) +++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Thu Nov 8 09:42:16 2018 @@ -83,6 +83,9 @@ runCheckOnCode(StringRef Code, std::vect 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"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
