NoQ updated this revision to Diff 217982.
NoQ marked 8 inline comments as done.
NoQ added a comment.
Fixits ♫ Fix the Fixits ♫
Yabba dabba doo ♫
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65182/new/
https://reviews.llvm.org/D65182
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/dead-stores.c
clang/test/Analysis/edges-new.mm
clang/test/Analysis/objc-arc.m
clang/test/Analysis/plist-output.m
clang/test/Analysis/virtualcall-fixits.cpp
Index: clang/test/Analysis/virtualcall-fixits.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/virtualcall-fixits.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN: %s 2>&1 | FileCheck -check-prefix=TEXT %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN: -analyzer-config fixits-as-remarks=true \
+// RUN: -analyzer-output=plist -o %t.plist -verify %s
+// RUN: cat %t.plist | FileCheck -check-prefix=PLIST %s
+
+struct S {
+ virtual void foo();
+ S() {
+ foo();
+ // expected-warning@-1{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
+ // expected-remark@-2{{5-5: 'S::'}}
+ }
+ ~S();
+};
+
+// TEXT: warning: Call to virtual method 'S::foo' during construction
+// TEXT-SAME: bypasses virtual dispatch
+// TEXT-NEXT: foo();
+// TEXT-NEXT: ^~~~~
+// TEXT-NEXT: S::
+// TEXT-NEXT: 1 warning generated.
+
+// PLIST: <key>fixits</key>
+// PLIST-NEXT: <array>
+// PLIST-NEXT: <dict>
+// PLIST-NEXT: <key>remove_range</key>
+// PLIST-NEXT: <array>
+// PLIST-NEXT: <dict>
+// PLIST-NEXT: <key>line</key><integer>13</integer>
+// PLIST-NEXT: <key>col</key><integer>5</integer>
+// PLIST-NEXT: <key>file</key><integer>0</integer>
+// PLIST-NEXT: </dict>
+// PLIST-NEXT: <dict>
+// PLIST-NEXT: <key>line</key><integer>13</integer>
+// PLIST-NEXT: <key>col</key><integer>4</integer>
+// PLIST-NEXT: <key>file</key><integer>0</integer>
+// PLIST-NEXT: </dict>
+// PLIST-NEXT: </array>
+// PLIST-NEXT: <key>insert_string</key>
+// PLIST-NEXT: <string>S::</string>
+// PLIST-NEXT: </dict>
+// PLIST-NEXT: </array>
Index: clang/test/Analysis/plist-output.m
===================================================================
--- clang/test/Analysis/plist-output.m
+++ clang/test/Analysis/plist-output.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t.plist
// RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/plist-output.m.plist -
void test_null_init(void) {
Index: clang/test/Analysis/objc-arc.m
===================================================================
--- clang/test/Analysis/objc-arc.m
+++ clang/test/Analysis/objc-arc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -o %t.plist %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t.plist %s
// RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/objc-arc.m.plist -
typedef signed char BOOL;
Index: clang/test/Analysis/edges-new.mm
===================================================================
--- clang/test/Analysis/edges-new.mm
+++ clang/test/Analysis/edges-new.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,deadcode.DeadStores,osx.cocoa.RetainCount,unix.Malloc,unix.MismatchedDeallocator -analyzer-output=plist -o %t -w %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,deadcode.DeadStores,osx.cocoa.RetainCount,unix.Malloc,unix.MismatchedDeallocator -analyzer-output=plist -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t -w %s
// RUN: %normalize_plist <%t | diff -ub %S/Inputs/expected-plists/edges-new.mm.plist -
//===----------------------------------------------------------------------===//
Index: clang/test/Analysis/dead-stores.c
===================================================================
--- clang/test/Analysis/dead-stores.c
+++ clang/test/Analysis/dead-stores.c
@@ -1,15 +1,20 @@
-// RUN: %clang_analyze_cc1 -Wunused-variable -analyzer-checker=core,deadcode.DeadStores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
-// RUN: %clang_analyze_cc1 -Wunused-variable -analyzer-checker=core,deadcode.DeadStores -analyzer-store=region -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_analyze_cc1 -Wunused-variable -Wno-unreachable-code \
+// RUN: -analyzer-checker=deadcode.DeadStores -fblocks -verify \
+// RUN: -analyzer-config deadcode.DeadStores:ShowFixIts=true \
+// RUN: -analyzer-config fixits-as-remarks=true \
+// RUN: -analyzer-opt-analyze-nested-blocks %s
void f1() {
int k, y; // expected-warning{{unused variable 'k'}} expected-warning{{unused variable 'y'}}
int abc=1;
long idx=abc+3*5; // expected-warning {{never read}} expected-warning{{unused variable 'idx'}}
+ // expected-remark@-1{{11-18: ''}}
}
void f2(void *b) {
char *c = (char*)b; // no-warning
char *d = b+1; // expected-warning {{never read}} expected-warning{{unused variable 'd'}}
+ // expected-remark@-1{{9-14: ''}}
printf("%s", c); // expected-warning{{implicitly declaring library function 'printf' with type 'int (const char *, ...)'}} \
// expected-note{{include the header <stdio.h> or explicitly provide a declaration for 'printf'}}
}
@@ -38,6 +43,7 @@
int x = 4; // no-warning
int *p = &x; // expected-warning{{never read}} expected-warning{{unused variable 'p'}}
+ // expected-remark@-1{{9-13: ''}}
}
@@ -401,6 +407,7 @@
void f23_pos(int argc, char **argv) {
int shouldLog = (argc > 1); // expected-warning{{Value stored to 'shouldLog' during its initialization is never read}} expected-warning{{unused variable 'shouldLog'}}
+ // expected-remark@-1{{16-28: ''}}
^{
f23_aux("I did too use it!\n");
}();
@@ -411,6 +418,7 @@
int x = (y > 2); // no-warning
^ {
int z = x + y; // expected-warning{{Value stored to 'z' during its initialization is never read}} expected-warning{{unused variable 'z'}}
+ // expected-remark@-1{{12-19: ''}}
}();
}
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -30,6 +30,7 @@
// CHECK-NEXT: ctu-dir = ""
// CHECK-NEXT: ctu-import-threshold = 100
// CHECK-NEXT: ctu-index-name = externalDefMap.txt
+// CHECK-NEXT: deadcode.DeadStores:ShowFixIts = false
// CHECK-NEXT: debug.AnalysisOrder:* = false
// CHECK-NEXT: debug.AnalysisOrder:Bind = false
// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
@@ -53,6 +54,7 @@
// CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
// CHECK-NEXT: exploration_strategy = unexplored_first_queue
// CHECK-NEXT: faux-bodies = true
+// CHECK-NEXT: fixits-as-remarks = false
// CHECK-NEXT: graph-trim-interval = 1000
// CHECK-NEXT: inline-lambdas = true
// CHECK-NEXT: ipa = dynamic-bifurcate
@@ -73,6 +75,7 @@
// CHECK-NEXT: optin.cplusplus.UninitializedObject:NotesAsWarnings = false
// CHECK-NEXT: optin.cplusplus.UninitializedObject:Pedantic = false
// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
+// CHECK-NEXT: optin.cplusplus.VirtualCall:ShowFixIts = false
// CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
// CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
// CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false
@@ -93,4 +96,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 90
+// CHECK-NEXT: num-entries = 93
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -83,11 +83,11 @@
namespace {
class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
DiagnosticsEngine &Diag;
- bool IncludePath, ShouldEmitAsError;
+ bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false;
public:
ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag)
- : Diag(Diag), IncludePath(false), ShouldEmitAsError(false) {}
+ : Diag(Diag) {}
~ClangDiagPathDiagConsumer() override {}
StringRef getName() const override { return "ClangDiags"; }
@@ -98,11 +98,9 @@
return IncludePath ? Minimal : None;
}
- void enablePaths() {
- IncludePath = true;
- }
-
+ void enablePaths() { IncludePath = true; }
void enableWerror() { ShouldEmitAsError = true; }
+ void enableFixitsAsRemarks() { FixitsAsRemarks = true; }
void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
FilesMade *filesMade) override {
@@ -111,13 +109,32 @@
? Diag.getCustomDiagID(DiagnosticsEngine::Error, "%0")
: Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0");
+ unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
for (std::vector<const PathDiagnostic*>::iterator I = Diags.begin(),
E = Diags.end(); I != E; ++I) {
const PathDiagnostic *PD = *I;
SourceLocation WarnLoc = PD->getLocation().asLocation();
- Diag.Report(WarnLoc, WarnID) << PD->getShortDescription()
- << PD->path.back()->getRanges();
+ if (!FixitsAsRemarks) {
+ Diag.Report(WarnLoc, WarnID)
+ << PD->getShortDescription() << PD->path.back()->getRanges()
+ << PD->getFixits();
+ } else {
+ Diag.Report(WarnLoc, WarnID)
+ << PD->getShortDescription() << PD->path.back()->getRanges();
+ for (const FixItHint &Hint : PD->getFixits()) {
+ SourceManager &SM = Diag.getSourceManager();
+ llvm::SmallString<128> Str;
+ llvm::raw_svector_ostream OS(Str);
+ // FIXME: Add support for InsertFromRange and BeforePreviousInsertion.
+ assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!");
+ assert(!Hint.BeforePreviousInsertions && "Not implemented yet!");
+ OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin()) << "-"
+ << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd()) << ": '"
+ << Hint.CodeToInsert << "'";
+ Diag.Report(WarnLoc, RemarkID) << OS.str();
+ }
+ }
// First, add extra notes, even if paths should not be included.
for (const auto &Piece : PD->path) {
@@ -241,6 +258,9 @@
if (Opts->AnalyzerWerror)
clangDiags->enableWerror();
+ if (Opts->ShouldEmitFixItHintsAsRemarks)
+ clangDiags->enableFixitsAsRemarks();
+
if (Opts->AnalysisDiagOpt == PD_TEXT) {
clangDiags->enablePaths();
Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -735,6 +735,27 @@
printCoverage(D, /*IndentLevel=*/2, Fids, FM, o);
+ if (!D->getFixits().empty()) {
+ o << " <key>fixits</key>\n";
+ o << " <array>\n";
+ for (const auto &Hint : D->getFixits()) {
+ assert(!Hint.isNull());
+ // FIXME: Add support for InsertFromRange and BeforePreviousInsertion.
+ assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!");
+ assert(!Hint.BeforePreviousInsertions && "Not implemented yet!");
+ o << " <dict>\n";
+ o << " <key>remove_range</key>\n";
+ EmitRange(o, SM, Lexer::getAsCharRange(Hint.RemoveRange, SM, LangOpts),
+ FM, 4);
+ o << " <key>insert_string</key>\n";
+ o << " <string>";
+ EmitString(o, Hint.CodeToInsert);
+ o << "</string>\n";
+ o << " </dict>\n";
+ }
+ o << " </array>\n";
+ }
+
// Close up the entry.
o << " </dict>\n";
}
Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -123,14 +123,15 @@
StringRef CheckName, const Decl *declWithIssue, StringRef bugtype,
StringRef verboseDesc, StringRef shortDesc, StringRef category,
PathDiagnosticLocation LocationToUnique, const Decl *DeclToUnique,
- std::unique_ptr<FilesToLineNumsMap> ExecutedLines)
+ std::unique_ptr<FilesToLineNumsMap> ExecutedLines,
+ ArrayRef<FixItHint> Fixits)
: CheckName(CheckName), DeclWithIssue(declWithIssue),
BugType(StripTrailingDots(bugtype)),
VerboseDesc(StripTrailingDots(verboseDesc)),
ShortDesc(StripTrailingDots(shortDesc)),
Category(StripTrailingDots(category)), UniqueingLoc(LocationToUnique),
UniqueingDecl(DeclToUnique), ExecutedLines(std::move(ExecutedLines)),
- path(pathImpl) {}
+ Fixits(Fixits.begin(), Fixits.end()), path(pathImpl) {}
static PathDiagnosticCallPiece *
getFirstStackedCallToHeaderFile(PathDiagnosticCallPiece *CP,
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1270,7 +1270,7 @@
R->getBugType().getName(), R->getDescription(),
R->getShortDescription(/*UseFallback=*/false), BT.getCategory(),
R->getUniqueingLocation(), R->getUniqueingDecl(),
- findExecutedLines(SM, R->getErrorNode()));
+ findExecutedLines(SM, R->getErrorNode()), R->getFixits());
}
static const Stmt *getStmtParent(const Stmt *S, const ParentMap &PM) {
@@ -3048,26 +3048,29 @@
}
void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
- const CheckerBase *Checker,
- StringRef Name, StringRef Category,
- StringRef Str, PathDiagnosticLocation Loc,
- ArrayRef<SourceRange> Ranges) {
+ const CheckerBase *Checker, StringRef Name,
+ StringRef Category, StringRef Str,
+ PathDiagnosticLocation Loc,
+ ArrayRef<SourceRange> Ranges,
+ ArrayRef<FixItHint> Fixits) {
EmitBasicReport(DeclWithIssue, Checker->getCheckName(), Name, Category, Str,
- Loc, Ranges);
+ Loc, Ranges, Fixits);
}
void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
CheckName CheckName,
StringRef name, StringRef category,
StringRef str, PathDiagnosticLocation Loc,
- ArrayRef<SourceRange> Ranges) {
+ ArrayRef<SourceRange> Ranges,
+ ArrayRef<FixItHint> Fixits) {
// 'BT' is owned by BugReporter.
BugType *BT = getBugTypeForName(CheckName, name, category);
auto R = std::make_unique<BugReport>(*BT, str, Loc);
R->setDeclWithIssue(DeclWithIssue);
- for (ArrayRef<SourceRange>::iterator I = Ranges.begin(), E = Ranges.end();
- I != E; ++I)
- R->addRange(*I);
+ for (const auto &SR : Ranges)
+ R->addRange(SR);
+ for (const auto &FH : Fixits)
+ R->addFixItHint(FH);
emitReport(std::move(R));
}
Index: clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -43,6 +43,7 @@
public:
// These are going to be null if the respective check is disabled.
mutable std::unique_ptr<BugType> BT_Pure, BT_Impure;
+ bool ShowFixIts = false;
void checkBeginFunction(CheckerContext &C) const;
void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
@@ -146,6 +147,17 @@
}
auto Report = std::make_unique<BugReport>(*BT, OS.str(), N);
+
+ if (ShowFixIts && !IsPure) {
+ // FIXME: These hints are valid only when the virtual call is made
+ // directly from the constructor/destructor. Otherwise the dispatch
+ // will work just fine from other callees, and the fix may break
+ // the otherwise correct program.
+ FixItHint Fixit = FixItHint::CreateInsertion(
+ CE->getBeginLoc(), MD->getParent()->getNameAsString() + "::");
+ Report->addFixItHint(Fixit);
+ }
+
C.emitReport(std::move(Report));
}
@@ -206,6 +218,8 @@
Chk->BT_Impure = std::make_unique<BugType>(
Mgr.getCurrentCheckName(), "Unexpected loss of virtual dispatch",
categories::CXXObjectLifecycle);
+ Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ Mgr.getCurrentCheckName(), "ShowFixIts");
}
}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -11,6 +11,7 @@
//
//===----------------------------------------------------------------------===//
+#include "clang/Lex/Lexer.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
@@ -119,11 +120,19 @@
}
namespace {
+class DeadStoresChecker : public Checker<check::ASTCodeBody> {
+public:
+ bool ShowFixIts = false;
+
+ void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+ BugReporter &BR) const;
+};
+
class DeadStoreObs : public LiveVariables::Observer {
const CFG &cfg;
ASTContext &Ctx;
BugReporter& BR;
- const CheckerBase *Checker;
+ const DeadStoresChecker *Checker;
AnalysisDeclContext* AC;
ParentMap& Parents;
llvm::SmallPtrSet<const VarDecl*, 20> Escaped;
@@ -135,7 +144,7 @@
public:
DeadStoreObs(const CFG &cfg, ASTContext &ctx, BugReporter &br,
- const CheckerBase *checker, AnalysisDeclContext *ac,
+ const DeadStoresChecker *checker, AnalysisDeclContext *ac,
ParentMap &parents,
llvm::SmallPtrSet<const VarDecl *, 20> &escaped)
: cfg(cfg), Ctx(ctx), BR(br), Checker(checker), AC(ac), Parents(parents),
@@ -202,12 +211,32 @@
llvm::raw_svector_ostream os(buf);
const char *BugType = nullptr;
+ SmallVector<FixItHint, 1> Fixits;
+
switch (dsk) {
- case DeadInit:
+ case DeadInit: {
BugType = "Dead initialization";
os << "Value stored to '" << *V
<< "' during its initialization is never read";
+
+ ASTContext &ACtx = V->getASTContext();
+ if (Checker->ShowFixIts) {
+ if (V->getInit()->HasSideEffects(ACtx,
+ /*IncludePossibleEffects=*/true)) {
+ break;
+ }
+ SourceManager &SM = ACtx.getSourceManager();
+ const LangOptions &LO = ACtx.getLangOpts();
+ SourceLocation L1 =
+ Lexer::findNextToken(
+ V->getTypeSourceInfo()->getTypeLoc().getEndLoc(),
+ SM, LO)->getEndLoc();
+ SourceLocation L2 =
+ Lexer::getLocForEndOfToken(V->getInit()->getEndLoc(), 1, SM, LO);
+ Fixits.push_back(FixItHint::CreateRemoval({L1, L2}));
+ }
break;
+ }
case DeadIncrement:
BugType = "Dead increment";
@@ -225,7 +254,7 @@
}
BR.EmitBasicReport(AC->getDecl(), Checker, BugType, "Dead store", os.str(),
- L, R);
+ L, R, Fixits);
}
void CheckVarDecl(const VarDecl *VD, const Expr *Ex, const Expr *Val,
@@ -471,35 +500,31 @@
// DeadStoresChecker
//===----------------------------------------------------------------------===//
-namespace {
-class DeadStoresChecker : public Checker<check::ASTCodeBody> {
-public:
- void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
- BugReporter &BR) const {
-
- // Don't do anything for template instantiations.
- // Proving that code in a template instantiation is "dead"
- // means proving that it is dead in all instantiations.
- // This same problem exists with -Wunreachable-code.
- if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
- if (FD->isTemplateInstantiation())
- return;
+void DeadStoresChecker::checkASTCodeBody(const Decl *D, AnalysisManager &mgr,
+ BugReporter &BR) const {
+ // Don't do anything for template instantiations.
+ // Proving that code in a template instantiation is "dead"
+ // means proving that it is dead in all instantiations.
+ // This same problem exists with -Wunreachable-code.
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+ if (FD->isTemplateInstantiation())
+ return;
- if (LiveVariables *L = mgr.getAnalysis<LiveVariables>(D)) {
- CFG &cfg = *mgr.getCFG(D);
- AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D);
- ParentMap &pmap = mgr.getParentMap(D);
- FindEscaped FS;
- cfg.VisitBlockStmts(FS);
- DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped);
- L->runOnAllBlocks(A);
- }
+ if (LiveVariables *L = mgr.getAnalysis<LiveVariables>(D)) {
+ CFG &cfg = *mgr.getCFG(D);
+ AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D);
+ ParentMap &pmap = mgr.getParentMap(D);
+ FindEscaped FS;
+ cfg.VisitBlockStmts(FS);
+ DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped);
+ L->runOnAllBlocks(A);
}
-};
}
-void ento::registerDeadStoresChecker(CheckerManager &mgr) {
- mgr.registerChecker<DeadStoresChecker>();
+void ento::registerDeadStoresChecker(CheckerManager &Mgr) {
+ auto *Chk = Mgr.registerChecker<DeadStoresChecker>();
+ Chk->ShowFixIts =
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "ShowFixIts");
}
bool ento::shouldRegisterDeadStoresChecker(const LangOptions &LO) {
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -811,13 +811,16 @@
/// Lines executed in the path.
std::unique_ptr<FilesToLineNumsMap> ExecutedLines;
+ SmallVector<FixItHint, 4> Fixits;
+
public:
PathDiagnostic() = delete;
PathDiagnostic(StringRef CheckName, const Decl *DeclWithIssue,
StringRef bugtype, StringRef verboseDesc, StringRef shortDesc,
StringRef category, PathDiagnosticLocation LocationToUnique,
const Decl *DeclToUnique,
- std::unique_ptr<FilesToLineNumsMap> ExecutedLines);
+ std::unique_ptr<FilesToLineNumsMap> ExecutedLines,
+ ArrayRef<FixItHint> Fixits);
~PathDiagnostic();
const PathPieces &path;
@@ -871,6 +874,8 @@
StringRef getBugType() const { return BugType; }
StringRef getCategory() const { return Category; }
+ ArrayRef<FixItHint> getFixits() const { return Fixits; }
+
/// Return the semantic context where an issue occurred. If the
/// issue occurs along a path, this represents the "central" area
/// where the bug manifests.
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -96,6 +96,7 @@
SmallVector<SourceRange, 4> Ranges;
const SourceRange ErrorNodeRange;
NoteList Notes;
+ SmallVector<FixItHint, 4> Fixits;
/// A (stack of) a set of symbols that are registered with this
/// report as being "interesting", and thus used to help decide which
@@ -319,7 +320,7 @@
const Stmt *getStmt() const;
- /// Add a range to a bug report.
+ /// Add a range to the bug report.
///
/// Ranges are used to highlight regions of interest in the source code.
/// They should be at the same source code line as the BugReport location.
@@ -335,6 +336,19 @@
/// Get the SourceRanges associated with the report.
virtual llvm::iterator_range<ranges_iterator> getRanges() const;
+ /// Add a fix-it hint to the bug report.
+ ///
+ /// Fix-it hints are the suggested edits to the code that would resolve
+ /// the problem explained by the bug report. Fix-it hints should be
+ /// as conservative as possible because it is not uncommon for the user
+ /// to blindly apply all fixits to their project. Note that it is very hard
+ /// to produce a good fix-it hint for most path-sensitive warnings.
+ void addFixItHint(const FixItHint &F) {
+ Fixits.push_back(F);
+ }
+
+ llvm::ArrayRef<FixItHint> getFixits() const { return Fixits; }
+
/// Add custom or predefined bug report visitors to this report.
///
/// The visitors should be used when the default trace is not sufficient.
@@ -473,12 +487,14 @@
void EmitBasicReport(const Decl *DeclWithIssue, const CheckerBase *Checker,
StringRef BugName, StringRef BugCategory,
StringRef BugStr, PathDiagnosticLocation Loc,
- ArrayRef<SourceRange> Ranges = None);
+ ArrayRef<SourceRange> Ranges = None,
+ ArrayRef<FixItHint> Fixits = None);
void EmitBasicReport(const Decl *DeclWithIssue, CheckName CheckName,
StringRef BugName, StringRef BugCategory,
StringRef BugStr, PathDiagnosticLocation Loc,
- ArrayRef<SourceRange> Ranges = None);
+ ArrayRef<SourceRange> Ranges = None,
+ ArrayRef<FixItHint> Fixits = None);
private:
llvm::StringMap<BugType *> StrBugTypes;
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -300,6 +300,14 @@
"Whether to place an event at each tracked condition.",
false)
+ANALYZER_OPTION(bool, ShouldEmitFixItHintsAsRemarks, "fixits-as-remarks",
+ "Emit fix-it hints as remarks for testing purposes",
+ false)
+
+//===----------------------------------------------------------------------===//
+// Unsigned analyzer options.
+//===----------------------------------------------------------------------===//
+
ANALYZER_OPTION(unsigned, CTUImportThreshold, "ctu-import-threshold",
"The maximal amount of translation units that is considered "
"for import when inlining functions during CTU analysis. "
@@ -308,10 +316,6 @@
"various translation units.",
100u)
-//===----------------------------------------------------------------------===//
-// Unsinged analyzer options.
-//===----------------------------------------------------------------------===//
-
ANALYZER_OPTION(
unsigned, AlwaysInlineSize, "ipa-always-inline-size",
"The size of the functions (in basic blocks), which should be considered "
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -563,6 +563,11 @@
def VirtualCallChecker : Checker<"VirtualCall">,
HelpText<"Check virtual function calls during construction/destruction">,
CheckerOptions<[
+ CmdLineOption<Boolean,
+ "ShowFixIts",
+ "Enable fix-it hints for this checker",
+ "false",
+ InAlpha>,
CmdLineOption<Boolean,
"PureOnly",
"Disables the checker. Keeps cplusplus.PureVirtualCall "
@@ -648,6 +653,13 @@
def DeadStoresChecker : Checker<"DeadStores">,
HelpText<"Check for values stored to variables that are never read "
"afterwards">,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "ShowFixIts",
+ "Enable fix-it hints for this checker",
+ "false",
+ InAlpha>
+ ]>,
Documentation<HasDocumentation>;
} // end DeadCode
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
