This revision was automatically updated to reflect the committed changes.
Closed by commit rL371257: [analyzer] Add minimal support for fix-it hints.
(authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65182?vs=218575&id=219170#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65182/new/
https://reviews.llvm.org/D65182
Files:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/dead-stores.c
cfe/trunk/test/Analysis/edges-new.mm
cfe/trunk/test/Analysis/objc-arc.m
cfe/trunk/test/Analysis/plist-output.m
cfe/trunk/test/Analysis/virtualcall-fixits.cpp
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ cfe/trunk/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: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -393,6 +393,7 @@
StringRef Tag;
std::vector<SourceRange> ranges;
+ std::vector<FixItHint> fixits;
protected:
PathDiagnosticPiece(StringRef s, Kind k, DisplayHint hint = Below);
@@ -437,9 +438,16 @@
ranges.push_back(SourceRange(B,E));
}
+ void addFixit(FixItHint F) {
+ fixits.push_back(F);
+ }
+
/// Return the SourceRanges associated with this PathDiagnosticPiece.
ArrayRef<SourceRange> getRanges() const { return ranges; }
+ /// Return the fix-it hints associated with this PathDiagnosticPiece.
+ ArrayRef<FixItHint> getFixits() const { return fixits; }
+
virtual void Profile(llvm::FoldingSetNodeID &ID) const;
void setAsLastInMainSourceFile() {
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ cfe/trunk/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
@@ -280,20 +281,17 @@
/// allows you to specify where exactly in the auto-generated path diagnostic
/// the extra note should appear.
void addNote(StringRef Msg, const PathDiagnosticLocation &Pos,
- ArrayRef<SourceRange> Ranges) {
+ ArrayRef<SourceRange> Ranges = {},
+ ArrayRef<FixItHint> Fixits = {}) {
auto P = std::make_shared<PathDiagnosticNotePiece>(Pos, Msg);
for (const auto &R : Ranges)
P->addRange(R);
- Notes.push_back(std::move(P));
- }
+ for (const auto &F : Fixits)
+ P->addFixit(F);
- // FIXME: Instead of making an override, we could have default-initialized
- // Ranges with {}, however it crashes the MSVC 2013 compiler.
- void addNote(StringRef Msg, const PathDiagnosticLocation &Pos) {
- std::vector<SourceRange> Ranges;
- addNote(Msg, Pos, Ranges);
+ Notes.push_back(std::move(P));
}
virtual const NoteList &getNotes() {
@@ -319,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.
@@ -335,6 +333,20 @@
/// Get the SourceRanges associated with the report.
virtual llvm::iterator_range<ranges_iterator> getRanges() const;
+ /// Add a fix-it hint to the warning message of 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. It usually is very hard
+ /// to produce a good fix-it hint for most path-sensitive warnings.
+ /// Fix-it hints can also be added to notes through the addNote() interface.
+ void addFixItHint(const FixItHint &F) {
+ Fixits.push_back(F);
+ }
+
+ 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 +485,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: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -564,6 +564,11 @@
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 "
"enabled. This option is only provided for backwards "
@@ -654,7 +659,12 @@
"Warns for deadstores in nested assignments."
"E.g.: if ((P = f())) where P is unused.",
"true",
- Released>
+ Released>,
+ CmdLineOption<Boolean,
+ "ShowFixIts",
+ "Enable fix-it hints for this checker",
+ "false",
+ InAlpha>
]>,
Documentation<HasDocumentation>;
Index: cfe/trunk/test/Analysis/virtualcall-fixits.cpp
===================================================================
--- cfe/trunk/test/Analysis/virtualcall-fixits.cpp
+++ cfe/trunk/test/Analysis/virtualcall-fixits.cpp
@@ -0,0 +1,45 @@
+// 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><string>S::</string>
+// PLIST-NEXT: </dict>
+// PLIST-NEXT: </array>
Index: cfe/trunk/test/Analysis/edges-new.mm
===================================================================
--- cfe/trunk/test/Analysis/edges-new.mm
+++ cfe/trunk/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: cfe/trunk/test/Analysis/plist-output.m
===================================================================
--- cfe/trunk/test/Analysis/plist-output.m
+++ cfe/trunk/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: cfe/trunk/test/Analysis/objc-arc.m
===================================================================
--- cfe/trunk/test/Analysis/objc-arc.m
+++ cfe/trunk/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: cfe/trunk/test/Analysis/analyzer-config.c
===================================================================
--- cfe/trunk/test/Analysis/analyzer-config.c
+++ cfe/trunk/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: deadcode.DeadStores:WarnForDeadNestedAssignments = true
// CHECK-NEXT: debug.AnalysisOrder:* = false
// CHECK-NEXT: debug.AnalysisOrder:Bind = false
@@ -54,6 +55,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
@@ -74,6 +76,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
@@ -94,4 +97,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 91
+// CHECK-NEXT: num-entries = 94
Index: cfe/trunk/test/Analysis/dead-stores.c
===================================================================
--- cfe/trunk/test/Analysis/dead-stores.c
+++ cfe/trunk/test/Analysis/dead-stores.c
@@ -1,17 +1,16 @@
-// RUN: %clang_analyze_cc1 -Wunused-variable -fblocks -Wno-unreachable-code \
-// RUN: -analyzer-checker=core,deadcode.DeadStores \
-// RUN: -analyzer-config deadcode.DeadStores:WarnForDeadNestedAssignments=false\
-// RUN: -analyzer-opt-analyze-nested-blocks -verify=non-nested %s
-//
-// RUN: %clang_analyze_cc1 -Wunused-variable -fblocks -Wno-unreachable-code \
-// RUN: -analyzer-checker=core,deadcode.DeadStores \
-// RUN: -analyzer-config deadcode.DeadStores:WarnForDeadNestedAssignments=false\
-// RUN: -analyzer-opt-analyze-nested-blocks -verify=non-nested \
-// RUN: -analyzer-store=region %s
-//
-// RUN: %clang_analyze_cc1 -Wunused-variable -fblocks -Wno-unreachable-code \
-// RUN: -analyzer-checker=core,deadcode.DeadStores \
-// RUN: -analyzer-opt-analyze-nested-blocks -verify=non-nested,nested %s
+// RUN: %clang_analyze_cc1 -Wunused-variable -fblocks -Wno-unreachable-code \
+// RUN: -analyzer-checker=core,deadcode.DeadStores \
+// RUN: -analyzer-config deadcode.DeadStores:ShowFixIts=true \
+// RUN: -analyzer-config fixits-as-remarks=true \
+// RUN: -analyzer-config \
+// RUN: deadcode.DeadStores:WarnForDeadNestedAssignments=false \
+// RUN: -verify=non-nested %s
+
+// RUN: %clang_analyze_cc1 -Wunused-variable -fblocks -Wno-unreachable-code \
+// RUN: -analyzer-checker=core,deadcode.DeadStores \
+// RUN: -analyzer-config deadcode.DeadStores:ShowFixIts=true \
+// RUN: -analyzer-config fixits-as-remarks=true \
+// RUN: -verify=non-nested,nested %s
void f1() {
int k, y; // non-nested-warning {{unused variable 'k'}}
@@ -19,12 +18,14 @@
int abc = 1;
long idx = abc + 3 * 5; // non-nested-warning {{never read}}
// non-nested-warning@-1 {{unused variable 'idx'}}
+ // non-nested-remark@-2 {{11-24: ''}}
}
void f2(void *b) {
char *c = (char *)b; // no-warning
char *d = b + 1; // non-nested-warning {{never read}}
// non-nested-warning@-1 {{unused variable 'd'}}
+ // non-nested-remark@-2 {{10-17: ''}}
printf("%s", c);
// non-nested-warning@-1 {{implicitly declaring library function 'printf' with type 'int (const char *, ...)'}}
// non-nested-note@-2 {{include the header <stdio.h> or explicitly provide a declaration for 'printf'}}
@@ -50,6 +51,7 @@
int x = 4; // no-warning
int *p = &x; // non-nested-warning {{never read}}
// non-nested-warning@-1 {{unused variable 'p'}}
+ // non-nested-remark@-2 {{9-13: ''}}
}
int f6() {
@@ -413,6 +415,7 @@
int shouldLog = (argc > 1);
// non-nested-warning@-1 {{Value stored to 'shouldLog' during its initialization is never read}}
// non-nested-warning@-2 {{unused variable 'shouldLog'}}
+ // non-nested-remark@-3 {{16-28: ''}}
^{
f23_aux("I did too use it!\n");
}();
@@ -425,6 +428,7 @@
int z = x + y;
// non-nested-warning@-1 {{Value stored to 'z' during its initialization is never read}}
// non-nested-warning@-2 {{unused variable 'z'}}
+ // non-nested-remark@-3 {{10-17: ''}}
}();
}
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -134,6 +134,7 @@
void EmitRanges(raw_ostream &o, const ArrayRef<SourceRange> Ranges,
unsigned indent);
void EmitMessage(raw_ostream &o, StringRef Message, unsigned indent);
+ void EmitFixits(raw_ostream &o, ArrayRef<FixItHint> fixits, unsigned indent);
void ReportControlFlow(raw_ostream &o,
const PathDiagnosticControlFlowPiece& P,
@@ -222,6 +223,33 @@
EmitString(o, Message) << '\n';
}
+void PlistPrinter::EmitFixits(raw_ostream &o, ArrayRef<FixItHint> fixits,
+ unsigned indent) {
+ if (fixits.size() == 0)
+ return;
+
+ const SourceManager &SM = PP.getSourceManager();
+ const LangOptions &LangOpts = PP.getLangOpts();
+
+ Indent(o, indent) << "<key>fixits</key>\n";
+ Indent(o, indent) << "<array>\n";
+ for (const auto &fixit : fixits) {
+ assert(!fixit.isNull());
+ // FIXME: Add support for InsertFromRange and BeforePreviousInsertion.
+ assert(!fixit.InsertFromRange.isValid() && "Not implemented yet!");
+ assert(!fixit.BeforePreviousInsertions && "Not implemented yet!");
+ Indent(o, indent) << " <dict>\n";
+ Indent(o, indent) << " <key>remove_range</key>\n";
+ EmitRange(o, SM, Lexer::getAsCharRange(fixit.RemoveRange, SM, LangOpts),
+ FM, indent + 2);
+ Indent(o, indent) << " <key>insert_string</key>";
+ EmitString(o, fixit.CodeToInsert);
+ o << "\n";
+ Indent(o, indent) << " </dict>\n";
+ }
+ Indent(o, indent) << "</array>\n";
+}
+
void PlistPrinter::ReportControlFlow(raw_ostream &o,
const PathDiagnosticControlFlowPiece& P,
unsigned indent) {
@@ -272,6 +300,9 @@
EmitString(o, s) << '\n';
}
+ assert(P.getFixits().size() == 0 &&
+ "Fixits on constrol flow pieces are not implemented yet!");
+
--indent;
Indent(o, indent) << "</dict>\n";
}
@@ -308,6 +339,9 @@
// Output the text.
EmitMessage(o, P.getString(), indent);
+ // Output the fixits.
+ EmitFixits(o, P.getFixits(), indent);
+
// Finish up.
--indent;
Indent(o, indent); o << "</dict>\n";
@@ -335,6 +369,9 @@
if (auto callExit = P.getCallExitEvent())
ReportPiece(o, *callExit, indent, depth, /*includeControlFlow*/ true);
+
+ assert(P.getFixits().size() == 0 &&
+ "Fixits on call pieces are not implemented yet!");
}
void PlistPrinter::ReportMacroSubPieces(raw_ostream &o,
@@ -347,6 +384,9 @@
I != E; ++I) {
ReportPiece(o, **I, indent, depth, /*includeControlFlow*/ false);
}
+
+ assert(P.getFixits().size() == 0 &&
+ "Fixits on constrol flow pieces are not implemented yet!");
}
void PlistPrinter::ReportMacroExpansions(raw_ostream &o, unsigned indent) {
@@ -404,6 +444,9 @@
// Output the text.
EmitMessage(o, P.getString(), indent);
+ // Output the fixits.
+ EmitFixits(o, P.getFixits(), indent);
+
// Finish up.
--indent;
Indent(o, indent); o << "</dict>\n";
@@ -432,6 +475,9 @@
// Output the text.
EmitMessage(o, P.getString(), indent);
+ assert(P.getFixits().size() == 0 &&
+ "Fixits on pop-up pieces are not implemented yet!");
+
// Finish up.
--indent;
Indent(o, indent) << "</dict>\n";
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2928,6 +2928,9 @@
Pieces.push_front(*I);
}
+ for (const auto &I : report->getFixits())
+ Pieces.back()->addFixit(I);
+
updateExecutedLinesWithDiagnosticPieces(*PD);
Consumer->HandlePathDiagnostic(std::move(PD));
}
@@ -3048,26 +3051,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: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ cfe/trunk/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,30 +120,37 @@
}
namespace {
+class DeadStoresChecker : public Checker<check::ASTCodeBody> {
+public:
+ bool ShowFixIts = false;
+ bool WarnForDeadNestedAssignments = true;
+
+ 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;
std::unique_ptr<ReachableCode> reachableCode;
const CFGBlock *currentBlock;
std::unique_ptr<llvm::DenseSet<const VarDecl *>> InEH;
- const bool WarnForDeadNestedAssignments;
enum DeadStoreKind { Standard, Enclosing, DeadIncrement, DeadInit };
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,
bool warnForDeadNestedAssignments)
: cfg(cfg), Ctx(ctx), BR(br), Checker(checker), AC(ac), Parents(parents),
- Escaped(escaped), currentBlock(nullptr),
- WarnForDeadNestedAssignments(warnForDeadNestedAssignments) {}
+ Escaped(escaped), currentBlock(nullptr) {}
~DeadStoreObs() override {}
@@ -205,12 +213,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";
@@ -222,7 +250,7 @@
// eg.: f((x = foo()))
case Enclosing:
- if (!WarnForDeadNestedAssignments)
+ if (!Checker->WarnForDeadNestedAssignments)
return;
BugType = "Dead nested assignment";
os << "Although the value stored to '" << *V
@@ -233,7 +261,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,
@@ -479,42 +507,37 @@
// DeadStoresChecker
//===----------------------------------------------------------------------===//
-namespace {
-class DeadStoresChecker : public Checker<check::ASTCodeBody> {
-public:
- bool WarnForDeadNestedAssignments = true;
-
- void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
- BugReporter &BR) const {
+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;
+ // 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,
- WarnForDeadNestedAssignments);
- 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,
+ WarnForDeadNestedAssignments);
+ L->runOnAllBlocks(A);
}
-};
}
void ento::registerDeadStoresChecker(CheckerManager &Mgr) {
- auto Chk = Mgr.registerChecker<DeadStoresChecker>();
+ auto *Chk = Mgr.registerChecker<DeadStoresChecker>();
const AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
Chk->WarnForDeadNestedAssignments =
AnOpts.getCheckerBooleanOption(Chk, "WarnForDeadNestedAssignments");
+ Chk->ShowFixIts =
+ AnOpts.getCheckerBooleanOption(Chk, "ShowFixIts");
}
bool ento::shouldRegisterDeadStoresChecker(const LangOptions &LO) {
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ cfe/trunk/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: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/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,22 +109,46 @@
? 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) {
+ auto reportPiece =
+ [&](unsigned ID, SourceLocation Loc, StringRef String,
+ ArrayRef<SourceRange> Ranges, ArrayRef<FixItHint> Fixits) {
+ if (!FixitsAsRemarks) {
+ Diag.Report(Loc, ID) << String << Ranges << Fixits;
+ } else {
+ Diag.Report(Loc, ID) << String << Ranges;
+ for (const FixItHint &Hint : Fixits) {
+ 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(Loc, RemarkID) << OS.str();
+ }
+ }
+ };
+
+ 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();
+ reportPiece(WarnID, PD->getLocation().asLocation(),
+ PD->getShortDescription(), PD->path.back()->getRanges(),
+ PD->path.back()->getFixits());
// First, add extra notes, even if paths should not be included.
for (const auto &Piece : PD->path) {
if (!isa<PathDiagnosticNotePiece>(Piece.get()))
continue;
- SourceLocation NoteLoc = Piece->getLocation().asLocation();
- Diag.Report(NoteLoc, NoteID) << Piece->getString()
- << Piece->getRanges();
+ reportPiece(NoteID, Piece->getLocation().asLocation(),
+ Piece->getString(), Piece->getRanges(), Piece->getFixits());
}
if (!IncludePath)
@@ -138,9 +160,8 @@
if (isa<PathDiagnosticNotePiece>(Piece.get()))
continue;
- SourceLocation NoteLoc = Piece->getLocation().asLocation();
- Diag.Report(NoteLoc, NoteID) << Piece->getString()
- << Piece->getRanges();
+ reportPiece(NoteID, Piece->getLocation().asLocation(),
+ Piece->getString(), Piece->getRanges(), Piece->getFixits());
}
}
}
@@ -241,6 +262,9 @@
if (Opts->AnalyzerWerror)
clangDiags->enableWerror();
+ if (Opts->ShouldEmitFixItHintsAsRemarks)
+ clangDiags->enableFixitsAsRemarks();
+
if (Opts->AnalysisDiagOpt == PD_TEXT) {
clangDiags->enablePaths();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
