[clang-tools-extra] r200924 - Improve clang-tidy diagnostic output and filtering.
Alexander Kornienko
alexfh at google.com
Thu Feb 6 06:50:10 PST 2014
Author: alexfh
Date: Thu Feb 6 08:50:10 2014
New Revision: 200924
URL: http://llvm.org/viewvc/llvm-project?rev=200924&view=rev
Log:
Improve clang-tidy diagnostic output and filtering.
Summary:
This patch introduces several improvements to clang-tidy diagnostic;
1. Make filtering of messages from non-user code more reliable. Output an
error when it or any of the related notes touches user code. This fixes an
assertion when an error has a location in a system header, and one of the
notes relates to user code.
2. In order for 1. to work, subscribe to the static analyzer diagnostics using
a custom PathDiagnosticConsumer.
3. Enable colors on supported terminals.
4. Output FixItHints.
Reviewers: djasper
Reviewed By: djasper
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2714
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
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=200924&r1=200923&r2=200924&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Thu Feb 6 08:50:10 2014
@@ -35,6 +35,7 @@
#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/Signals.h"
#include <algorithm>
#include <vector>
@@ -48,9 +49,9 @@ namespace clang {
namespace tidy {
namespace {
-static const char *AnalyzerCheckerNamePrefix = "clang-analyzer-";
+static const char *AnalyzerCheckNamePrefix = "clang-analyzer-";
-static StringRef StaticAnalyzerCheckers[] = {
+static StringRef StaticAnalyzerChecks[] = {
#define GET_CHECKERS
#define CHECKER(FULLNAME, CLASS, DESCFILE, HELPTEXT, GROUPINDEX, HIDDEN) \
FULLNAME,
@@ -59,6 +60,56 @@ static StringRef StaticAnalyzerCheckers[
#undef GET_CHECKERS
};
+class AnalyzerDiagnosticConsumer : public ento::PathDiagnosticConsumer {
+public:
+ AnalyzerDiagnosticConsumer(ClangTidyContext &Context) : Context(Context) {}
+
+ virtual void
+ FlushDiagnosticsImpl(std::vector<const ento::PathDiagnostic *> &Diags,
+ FilesMade *filesMade) LLVM_OVERRIDE {
+ for (std::vector<const ento::PathDiagnostic *>::iterator I = Diags.begin(),
+ E = Diags.end();
+ I != E; ++I) {
+ const ento::PathDiagnostic *PD = *I;
+ StringRef CheckName(AnalyzerCheckNamePrefix);
+ addRanges(Context.diag(CheckName, PD->getLocation().asLocation(),
+ PD->getShortDescription()),
+ PD->path.back()->getRanges());
+
+ ento::PathPieces FlatPath =
+ PD->path.flatten(/*ShouldFlattenMacros=*/true);
+ for (ento::PathPieces::const_iterator PI = FlatPath.begin(),
+ PE = FlatPath.end();
+ PI != PE; ++PI) {
+ addRanges(Context.diag(CheckName, (*PI)->getLocation().asLocation(),
+ (*PI)->getString(), DiagnosticIDs::Note),
+ (*PI)->getRanges());
+ }
+ }
+ }
+
+ virtual StringRef getName() const { return "ClangTidyDiags"; }
+
+ virtual bool supportsLogicalOpControlFlow() const LLVM_OVERRIDE {
+ return true;
+ }
+ virtual bool supportsCrossFileDiagnostics() const LLVM_OVERRIDE {
+ return true;
+ }
+
+private:
+ ClangTidyContext &Context;
+
+ // FIXME: Convert to operator<<(DiagnosticBuilder&, ArrayRef<SourceRange>).
+ static const DiagnosticBuilder &addRanges(const DiagnosticBuilder &DB,
+ ArrayRef<SourceRange> Ranges) {
+ for (ArrayRef<SourceRange>::iterator I = Ranges.begin(), E = Ranges.end();
+ I != E; ++I)
+ DB << *I;
+ return DB;
+ }
+};
+
} // namespace
ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
@@ -103,15 +154,15 @@ clang::ASTConsumer *ClangTidyASTConsumer
AnalyzerOptionsRef Options = Compiler.getAnalyzerOpts();
Options->CheckersControlList = getCheckersControlList();
Options->AnalysisStoreOpt = RegionStoreModel;
- Options->AnalysisDiagOpt = PD_TEXT;
+ Options->AnalysisDiagOpt = PD_NONE;
Options->AnalyzeNestedBlocks = true;
Options->eagerlyAssumeBinOpBifurcation = true;
- ASTConsumer *Consumers[] = {
- Finder.newASTConsumer(),
- ento::CreateAnalysisConsumer(Compiler.getPreprocessor(),
- Compiler.getFrontendOpts().OutputFile, Options,
- Compiler.getFrontendOpts().Plugins)
- };
+ ento::AnalysisASTConsumer *AnalysisConsumer = ento::CreateAnalysisConsumer(
+ Compiler.getPreprocessor(), Compiler.getFrontendOpts().OutputFile,
+ Options, Compiler.getFrontendOpts().Plugins);
+ AnalysisConsumer->AddDiagnosticConsumer(
+ new AnalyzerDiagnosticConsumer(Context));
+ ASTConsumer *Consumers[] = { Finder.newASTConsumer(), AnalysisConsumer };
return new MultiplexConsumer(Consumers);
}
@@ -129,7 +180,7 @@ std::vector<std::string> ClangTidyASTCon
for (CheckersList::const_iterator I = AnalyzerChecks.begin(),
E = AnalyzerChecks.end();
I != E; ++I)
- CheckNames.push_back(AnalyzerCheckerNamePrefix + I->first);
+ CheckNames.push_back(AnalyzerCheckNamePrefix + I->first);
std::sort(CheckNames.begin(), CheckNames.end());
return CheckNames;
@@ -138,13 +189,13 @@ std::vector<std::string> ClangTidyASTCon
ClangTidyASTConsumerFactory::CheckersList
ClangTidyASTConsumerFactory::getCheckersControlList() {
CheckersList List;
- ArrayRef<StringRef> Checkers(StaticAnalyzerCheckers);
+ ArrayRef<StringRef> Checks(StaticAnalyzerChecks);
bool AnalyzerChecksEnabled = false;
- for (unsigned i = 0; i < Checkers.size(); ++i) {
- std::string Checker((AnalyzerCheckerNamePrefix + Checkers[i]).str());
+ for (unsigned i = 0; i < Checks.size(); ++i) {
+ std::string Checker((AnalyzerCheckNamePrefix + Checks[i]).str());
AnalyzerChecksEnabled |=
- Filter.IsCheckEnabled(Checker) && !Checkers[i].startswith("debug");
+ Filter.IsCheckEnabled(Checker) && !Checks[i].startswith("debug");
}
if (AnalyzerChecksEnabled) {
@@ -155,12 +206,12 @@ ClangTidyASTConsumerFactory::getCheckers
// Always add all core checkers if any other static analyzer checks are
// enabled. This is currently necessary, as other path sensitive checks
// rely on the core checkers.
- for (unsigned i = 0; i < Checkers.size(); ++i) {
- std::string Checker((AnalyzerCheckerNamePrefix + Checkers[i]).str());
+ for (unsigned i = 0; i < Checks.size(); ++i) {
+ std::string Checker((AnalyzerCheckNamePrefix + Checks[i]).str());
- if (Checkers[i].startswith("core") ||
- (!Checkers[i].startswith("debug") && Filter.IsCheckEnabled(Checker)))
- List.push_back(std::make_pair(Checkers[i], true));
+ if (Checks[i].startswith("core") ||
+ (!Checks[i].startswith("debug") && Filter.IsCheckEnabled(Checker)))
+ List.push_back(std::make_pair(Checks[i], true));
}
}
return List;
@@ -237,29 +288,42 @@ void runClangTidy(StringRef EnableChecks
EnableChecksRegex, DisableChecksRegex, Context)));
}
+static SourceLocation getLocation(SourceManager &SourceMgr, StringRef FilePath,
+ unsigned Offset) {
+ if (FilePath.empty())
+ return SourceLocation();
+
+ const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);
+ FileID ID = SourceMgr.createFileID(File, SourceLocation(), SrcMgr::C_User);
+ return SourceMgr.getLocForStartOfFile(ID).getLocWithOffset(Offset);
+}
+
static void reportDiagnostic(const ClangTidyMessage &Message,
SourceManager &SourceMgr,
DiagnosticsEngine::Level Level,
DiagnosticsEngine &Diags,
- StringRef CheckName = "") {
- SourceLocation Loc;
- if (!Message.FilePath.empty()) {
- const FileEntry *File =
- SourceMgr.getFileManager().getFile(Message.FilePath);
- FileID ID = SourceMgr.createFileID(File, SourceLocation(), SrcMgr::C_User);
- Loc = SourceMgr.getLocForStartOfFile(ID);
- Loc = Loc.getLocWithOffset(Message.FileOffset);
- }
- if (CheckName.empty())
- Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0")) << Message.Message;
- else
- Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
- << Message.Message << CheckName;
+ tooling::Replacements *Fixes = NULL) {
+ SourceLocation Loc =
+ getLocation(SourceMgr, Message.FilePath, Message.FileOffset);
+ DiagnosticBuilder Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0"))
+ << Message.Message;
+ if (Fixes != NULL) {
+ for (tooling::Replacements::const_iterator I = Fixes->begin(),
+ E = Fixes->end();
+ I != E; ++I) {
+ SourceLocation FixLoc =
+ getLocation(SourceMgr, I->getFilePath(), I->getOffset());
+ Diag << FixItHint::CreateReplacement(
+ SourceRange(FixLoc, FixLoc.getLocWithOffset(I->getLength())),
+ I->getReplacementText());
+ }
+ }
}
void handleErrors(SmallVectorImpl<ClangTidyError> &Errors, bool Fix) {
FileManager Files((FileSystemOptions()));
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+ DiagOpts->ShowColors = llvm::sys::Process::StandardErrHasColors();
DiagnosticConsumer *DiagPrinter =
new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts);
DiagnosticsEngine Diags(IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
@@ -271,7 +335,7 @@ void handleErrors(SmallVectorImpl<ClangT
E = Errors.end();
I != E; ++I) {
reportDiagnostic(I->Message, SourceMgr, DiagnosticsEngine::Warning, Diags,
- I->CheckName);
+ &I->Fix);
for (unsigned i = 0, e = I->Notes.size(); i != e; ++i) {
reportDiagnostic(I->Notes[i], SourceMgr, DiagnosticsEngine::Note, Diags);
}
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=200924&r1=200923&r2=200924&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Thu Feb 6 08:50:10 2014
@@ -37,11 +37,11 @@ ClangTidyError::ClangTidyError(StringRef
const ClangTidyMessage &Message)
: CheckName(CheckName), Message(Message) {}
-DiagnosticBuilder ClangTidyContext::diag(StringRef CheckName,
- SourceLocation Loc,
- StringRef Description) {
+DiagnosticBuilder ClangTidyContext::diag(
+ StringRef CheckName, SourceLocation Loc, StringRef Description,
+ DiagnosticIDs::Level Level /* = DiagnosticsEngine::Warning*/) {
unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(
- DiagnosticIDs::Warning, Description);
+ Level, (Description + " [" + CheckName + "]").str());
if (CheckNamesByDiagnosticID.count(ID) == 0)
CheckNamesByDiagnosticID.insert(std::make_pair(ID, CheckName.str()));
return DiagEngine->Report(Loc, ID);
@@ -69,7 +69,7 @@ StringRef ClangTidyContext::getCheckName
}
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx)
- : Context(Ctx) {
+ : Context(Ctx), LastErrorRelatesToUserCode(false) {
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
Diags.reset(new DiagnosticsEngine(
IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts, this,
@@ -77,31 +77,38 @@ ClangTidyDiagnosticConsumer::ClangTidyDi
Context.setDiagnosticsEngine(Diags.get());
}
+void ClangTidyDiagnosticConsumer::finalizeLastError() {
+ if (!LastErrorRelatesToUserCode && !Errors.empty())
+ Errors.pop_back();
+ LastErrorRelatesToUserCode = false;
+}
+
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
- // FIXME: Demultiplex diagnostics.
- // FIXME: Ensure that we don't get notes from user code related to errors
- // from non-user code.
- // Let argument parsing-related warnings through.
- if (Diags->hasSourceManager() &&
- Diags->getSourceManager().isInSystemHeader(Info.getLocation()))
- return;
- if (DiagLevel != DiagnosticsEngine::Note) {
- Errors.push_back(
- ClangTidyError(Context.getCheckName(Info.getID()), getMessage(Info)));
- } else {
+ // FIXME: Deduplicate diagnostics.
+ if (DiagLevel == DiagnosticsEngine::Note) {
assert(!Errors.empty() &&
"A diagnostic note can only be appended to a message.");
Errors.back().Notes.push_back(getMessage(Info));
+ } else {
+ finalizeLastError();
+ Errors.push_back(
+ ClangTidyError(Context.getCheckName(Info.getID()), getMessage(Info)));
}
addFixes(Info, Errors.back());
+
+ // Let argument parsing-related warnings through.
+ if (!Diags->hasSourceManager() ||
+ !Diags->getSourceManager().isInSystemHeader(Info.getLocation())) {
+ LastErrorRelatesToUserCode = true;
+ }
}
// Flushes the internal diagnostics buffer to the ClangTidyContext.
void ClangTidyDiagnosticConsumer::finish() {
- for (unsigned i = 0, e = Errors.size(); i != e; ++i) {
+ finalizeLastError();
+ for (unsigned i = 0, e = Errors.size(); i != e; ++i)
Context.storeError(Errors[i]);
- }
Errors.clear();
}
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=200924&r1=200923&r2=200924&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Thu Feb 6 08:50:10 2014
@@ -75,7 +75,8 @@ public:
/// tablegen'd diagnostic IDs.
/// FIXME: Figure out a way to manage ID spaces.
DiagnosticBuilder diag(StringRef CheckName, SourceLocation Loc,
- StringRef Message);
+ StringRef Message,
+ DiagnosticIDs::Level Level = DiagnosticIDs::Warning);
/// \brief Sets the \c DiagnosticsEngine so that Diagnostics can be generated
/// correctly.
@@ -124,10 +125,12 @@ public:
private:
void addFixes(const Diagnostic &Info, ClangTidyError &Error);
ClangTidyMessage getMessage(const Diagnostic &Info) const;
+ void finalizeLastError();
ClangTidyContext &Context;
OwningPtr<DiagnosticsEngine> Diags;
SmallVector<ClangTidyError, 8> Errors;
+ bool LastErrorRelatesToUserCode;
};
} // end namespace tidy
More information about the cfe-commits
mailing list