NoQ updated this revision to Diff 211595.
NoQ added a comment.
Unforget a `FileCheck`-based test file for the virtual call checker that i
already had.
In D65182#1598614 <https://reviews.llvm.org/D65182#1598614>, @Szelethus wrote:
> How does an `-analyzer-config fixits-as-warnings` option sound like for more
> readable tests?
Yeah, that makes a lot of sense! I guess i'll do fixits-as-remarks instead :D
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65182/new/
https://reviews.llvm.org/D65182
Files:
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/Inputs/expected-plists/edges-new.mm.plist
clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
clang/test/Analysis/virtualcall-fixits.cpp
Index: clang/test/Analysis/virtualcall-fixits.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/virtualcall-fixits.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: %s 2>&1 | FileCheck %s
+
+struct S {
+ virtual void foo();
+ S() {
+ foo();
+ }
+};
+
+// CHECK: warning: Call to virtual method 'S::foo' during construction bypasses
+// CHECK-SAME: virtual dispatch (qualify the call explicitly to suppress
+// CHECK-SAME: the warning)
+// CHECK-NEXT: foo();
+// CHECK-NEXT: ^~~~~
+// CHECK-NEXT: S::
+// CHECK-NEXT: 1 warning generated.
Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
@@ -2190,6 +2190,26 @@
<integer>86</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>86</integer>
+ <key>col</key><integer>11</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>86</integer>
+ <key>col</key><integer>40</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
Index: clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
@@ -403,6 +403,26 @@
<integer>119</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>119</integer>
+ <key>col</key><integer>7</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>119</integer>
+ <key>col</key><integer>25</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -471,6 +491,26 @@
<integer>139</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>139</integer>
+ <key>col</key><integer>10</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>139</integer>
+ <key>col</key><integer>53</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -539,6 +579,26 @@
<integer>144</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>144</integer>
+ <key>col</key><integer>10</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>144</integer>
+ <key>col</key><integer>45</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -607,6 +667,26 @@
<integer>145</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>145</integer>
+ <key>col</key><integer>10</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>145</integer>
+ <key>col</key><integer>44</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -675,6 +755,26 @@
<integer>146</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>146</integer>
+ <key>col</key><integer>10</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>146</integer>
+ <key>col</key><integer>48</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -1085,6 +1185,26 @@
<integer>150</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>150</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>150</integer>
+ <key>col</key><integer>64</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -1153,6 +1273,26 @@
<integer>151</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>151</integer>
+ <key>col</key><integer>18</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>151</integer>
+ <key>col</key><integer>67</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -1221,6 +1361,26 @@
<integer>152</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>152</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>152</integer>
+ <key>col</key><integer>55</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
@@ -1289,6 +1449,26 @@
<integer>153</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>153</integer>
+ <key>col</key><integer>18</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>153</integer>
+ <key>col</key><integer>58</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -11430,6 +11430,26 @@
<integer>431</integer>
</array>
</dict>
+ <key>fixits</key>
+ <array>
+ <dict>
+ <key>remove_range</key>
+ <array>
+ <dict>
+ <key>line</key><integer>431</integer>
+ <key>col</key><integer>11</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>431</integer>
+ <key>col</key><integer>40</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>insert_string</key>
+ <string></string>
+ </dict>
+ </array>
</dict>
<dict>
<key>path</key>
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -116,8 +116,9 @@
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();
+ Diag.Report(WarnLoc, WarnID)
+ << PD->getShortDescription() << PD->path.back()->getRanges()
+ << PD->getFixits();
// First, add extra notes, even if paths should not be included.
for (const auto &Piece : PD->path) {
Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -731,6 +731,22 @@
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());
+ 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
@@ -1261,7 +1261,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) {
@@ -3121,23 +3121,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
@@ -146,6 +146,11 @@
return;
auto Report = llvm::make_unique<BugReport>(BT, OS.str(), N);
+ if (!IsPure) {
+ FixItHint Fixit = FixItHint::CreateInsertion(
+ CE->getBeginLoc(), MD->getParent()->getNameAsString() + "::");
+ Report->addFixItHint(Fixit);
+ }
C.emitReport(std::move(Report));
}
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"
@@ -202,12 +203,23 @@
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";
+ SourceLocation L1 = Lexer::findNextToken(
+ V->getTypeSourceInfo()->getTypeLoc().getEndLoc(),
+ V->getASTContext().getSourceManager(),
+ V->getASTContext().getLangOpts())->getEndLoc();
+ SourceLocation L2 = Lexer::getLocForEndOfToken(
+ V->getInit()->getEndLoc(), 1, V->getASTContext().getSourceManager(),
+ V->getASTContext().getLangOpts());
+ Fixits.push_back(FixItHint::CreateRemoval({L1, L2}));
break;
+ }
case DeadIncrement:
BugType = "Dead increment";
@@ -225,7 +237,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,
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
@@ -804,13 +804,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;
@@ -864,6 +867,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
@@ -107,6 +107,8 @@
ExtraTextList ExtraText;
NoteList Notes;
+ SmallVector<FixItHint, 4> Fixits;
+
using Symbols = llvm::DenseSet<SymbolRef>;
using Regions = llvm::DenseSet<const MemRegion *>;
@@ -333,9 +335,17 @@
Ranges.push_back(R);
}
+ void addFixItHint(FixItHint F) {
+ Fixits.push_back(F);
+ }
+
/// Get the SourceRanges associated with the report.
virtual llvm::iterator_range<ranges_iterator> getRanges();
+ virtual llvm::ArrayRef<FixItHint> getFixits() {
+ return Fixits;
+ }
+
/// Add custom or predefined bug report visitors to this report.
///
/// The visitors should be used when the default trace is not sufficient.
@@ -502,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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
