replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

llvm-svn: 200924
This commit is contained in:
Alexander Kornienko 2014-02-06 14:50:10 +00:00
parent 1a0aadcd77
commit 54461eb389
3 changed files with 126 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

130
clang-tools-extra/clang-tidy/ClangTidy.cpp
View File

@ -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 *ClangTidyASTConsumerFactory::CreateASTConsumer(
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> ClangTidyASTConsumerFactory::getCheckNames() {
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> ClangTidyASTConsumerFactory::getCheckNames() {
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::getCheckersControlList() {
// 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 EnableChecksRegex, StringRef DisableChecksRegex,
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);
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());
}
}
if (CheckName.empty())
Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0")) << Message.Message;
else
Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
<< Message.Message << CheckName;
}
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<ClangTidyError> &Errors, bool Fix) {
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);
}

43
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
View File

@ -37,11 +37,11 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
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(unsigned DiagnosticID) const {
}
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::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx)
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();
}

5
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
View File

@ -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