NoQ updated this revision to Diff 215236.
NoQ added a comment.
Add a "not implemented yet" assertion for kinds of fixits that are not
supported as of that patch (but will definitely need to be supported in the
future).
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-re@-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-re@-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-re@-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-re@-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-re@-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-re@-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
@@ -92,4 +95,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 89
+// CHECK-NEXT: num-entries = 92
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 << Hint.RemoveRange.getBegin().printToString(SM) << " - "
+ << Hint.RemoveRange.getEnd().printToString(SM) << " - '"
+ << 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,25 @@
printCoverage(D, /*IndentLevel=*/2, Fids, FM, o);
+ if (!D->getFixits().empty()) {
+ o << " <key>fixits</key>\n";
+ o << " <array>\n";
+ for (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>" << Hint.CodeToInsert << "</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
@@ -134,14 +134,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
@@ -1268,7 +1268,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) {
@@ -3079,23 +3079,26 @@
const CheckerBase *Checker,
StringRef Name, StringRef Category,
StringRef Str, PathDiagnosticLocation Loc,
- ArrayRef<SourceRange> Ranges) {
+ 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) {
+ CheckName CheckName, StringRef name,
+ StringRef category, StringRef str,
+ PathDiagnosticLocation Loc,
+ ArrayRef<SourceRange> Ranges,
+ ArrayRef<FixItHint> Fixits) {
// 'BT' is owned by BugReporter.
BugType *BT = getBugTypeForName(CheckName, name, category);
auto R = llvm::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 (auto SR : Ranges)
+ R->addRange(SR);
+ for (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;
@@ -145,6 +146,17 @@
}
auto Report = llvm::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));
}
@@ -205,6 +217,8 @@
Chk->BT_Impure = llvm::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
@@ -813,13 +813,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;
@@ -873,6 +876,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
@@ -108,6 +108,7 @@
const SourceRange ErrorNodeRange;
ExtraTextList ExtraText;
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
@@ -316,7 +317,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.
@@ -332,6 +333,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(FixItHint F) {
+ Fixits.push_back(F);
+ }
+
+ virtual 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.
@@ -498,12 +512,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
