[clang-tools-extra] r346418 - [clang-tidy] Untangle layering in ClangTidyDiagnosticConsumer somewhat. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 8 09:42:16 PST 2018
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");
More information about the cfe-commits
mailing list