sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, xazax.hun.
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.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54033
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/plugin/ClangTidyPlugin.cpp
unittests/clang-tidy/ClangTidyTest.h
Index: unittests/clang-tidy/ClangTidyTest.h
===================================================================
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -83,6 +83,9 @@
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");
Index: clang-tidy/plugin/ClangTidyPlugin.cpp
===================================================================
--- clang-tidy/plugin/ClangTidyPlugin.cpp
+++ clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -35,7 +35,7 @@
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);
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -102,6 +102,12 @@
/// \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 @@
}
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 @@
/// \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;
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -267,13 +267,7 @@
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 @@
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 @@
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 @@
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;
Index: clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -549,7 +549,9 @@
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 {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
