r371257 - [analyzer] Add minimal support for fix-it hints.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 6 13:55:29 PDT 2019
Author: dergachev
Date: Fri Sep 6 13:55:29 2019
New Revision: 371257
URL: http://llvm.org/viewvc/llvm-project?rev=371257&view=rev
Log:
[analyzer] Add minimal support for fix-it hints.
Allow attaching fixit hints to Static Analyzer BugReports.
Fixits are attached either to the bug report itself or to its notes
(path-sensitive event notes or path-insensitive extra notes).
Add support for fixits in text output (including the default text output that
goes without notes, as long as the fixit "belongs" to the warning).
Add support for fixits in the plist output mode.
Implement a fixit for the path-insensitive DeadStores checker. Only dead
initialization warning is currently covered.
Implement a fixit for the path-sensitive VirtualCall checker when the virtual
method is not pure virtual (in this case the "fix" is to suppress the warning
by qualifying the call).
Both fixits are under an off-by-default flag for now, because they
require more careful testing.
Differential Revision: https://reviews.llvm.org/D65182
Added:
cfe/trunk/test/Analysis/virtualcall-fixits.cpp
Modified:
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
Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Fri Sep 6 13:55:29 2019
@@ -564,6 +564,11 @@ def VirtualCallChecker : Checker<"Virtua
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 @@ def DeadStoresChecker : Checker<"DeadSto
"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>;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def Fri Sep 6 13:55:29 2019
@@ -300,6 +300,14 @@ ANALYZER_OPTION(bool, ShouldTrackConditi
"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 @@ ANALYZER_OPTION(unsigned, CTUImportThres
"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 "
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Fri Sep 6 13:55:29 2019
@@ -96,6 +96,7 @@ protected:
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 @@ public:
/// 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 @@ public:
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 @@ public:
/// 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 @@ public:
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;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h Fri Sep 6 13:55:29 2019
@@ -393,6 +393,7 @@ private:
StringRef Tag;
std::vector<SourceRange> ranges;
+ std::vector<FixItHint> fixits;
protected:
PathDiagnosticPiece(StringRef s, Kind k, DisplayHint hint = Below);
@@ -437,9 +438,16 @@ public:
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() {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp Fri Sep 6 13:55:29 2019
@@ -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 @@ LookThroughTransitiveAssignmentsAndComma
}
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 @@ public:
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 @@ public:
// 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 @@ public:
}
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 @@ public:
// 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) {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp Fri Sep 6 13:55:29 2019
@@ -43,6 +43,7 @@ class VirtualCallChecker
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 @@ void VirtualCallChecker::checkPreCall(co
}
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 @@ void ento::registerVirtualCallChecker(Ch
Chk->BT_Impure = std::make_unique<BugType>(
Mgr.getCurrentCheckName(), "Unexpected loss of virtual dispatch",
categories::CXXObjectLifecycle);
+ Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ Mgr.getCurrentCheckName(), "ShowFixIts");
}
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Sep 6 13:55:29 2019
@@ -2928,6 +2928,9 @@ void BugReporter::FlushReport(BugReportE
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 @@ BugReporter::generateDiagnosticForConsum
}
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));
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Fri Sep 6 13:55:29 2019
@@ -134,6 +134,7 @@ private:
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 @@ void PlistPrinter::EmitMessage(raw_ostre
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 @@ void PlistPrinter::ReportControlFlow(raw
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 @@ void PlistPrinter::ReportEvent(raw_ostre
// 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 @@ void PlistPrinter::ReportCall(raw_ostrea
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 @@ void PlistPrinter::ReportMacroSubPieces(
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 @@ void PlistPrinter::ReportNote(raw_ostrea
// 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 @@ void PlistPrinter::ReportPopUp(raw_ostre
// 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";
Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Fri Sep 6 13:55:29 2019
@@ -83,11 +83,11 @@ void ento::createTextPathDiagnosticConsu
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 @@ public:
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 @@ public:
? 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 @@ public:
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 @@ public:
if (Opts->AnalyzerWerror)
clangDiags->enableWerror();
+ if (Opts->ShouldEmitFixItHintsAsRemarks)
+ clangDiags->enableFixitsAsRemarks();
+
if (Opts->AnalysisDiagOpt == PD_TEXT) {
clangDiags->enablePaths();
Modified: cfe/trunk/test/Analysis/analyzer-config.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/analyzer-config.c (original)
+++ cfe/trunk/test/Analysis/analyzer-config.c Fri Sep 6 13:55:29 2019
@@ -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
Modified: cfe/trunk/test/Analysis/dead-stores.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dead-stores.c?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dead-stores.c (original)
+++ cfe/trunk/test/Analysis/dead-stores.c Fri Sep 6 13:55:29 2019
@@ -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 @@ void f1() {
int abc = 1;
long idx = abc + 3 * 5; // non-nested-warning {{never read}}
// non-nested-warning at -1 {{unused variable 'idx'}}
+ // non-nested-remark at -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 at -1 {{unused variable 'd'}}
+ // non-nested-remark at -2 {{10-17: ''}}
printf("%s", c);
// non-nested-warning at -1 {{implicitly declaring library function 'printf' with type 'int (const char *, ...)'}}
// non-nested-note at -2 {{include the header <stdio.h> or explicitly provide a declaration for 'printf'}}
@@ -50,6 +51,7 @@ void f5() {
int x = 4; // no-warning
int *p = &x; // non-nested-warning {{never read}}
// non-nested-warning at -1 {{unused variable 'p'}}
+ // non-nested-remark at -2 {{9-13: ''}}
}
int f6() {
@@ -413,6 +415,7 @@ void f23_pos(int argc, char **argv) {
int shouldLog = (argc > 1);
// non-nested-warning at -1 {{Value stored to 'shouldLog' during its initialization is never read}}
// non-nested-warning at -2 {{unused variable 'shouldLog'}}
+ // non-nested-remark at -3 {{16-28: ''}}
^{
f23_aux("I did too use it!\n");
}();
@@ -425,6 +428,7 @@ void f24_A(int y) {
int z = x + y;
// non-nested-warning at -1 {{Value stored to 'z' during its initialization is never read}}
// non-nested-warning at -2 {{unused variable 'z'}}
+ // non-nested-remark at -3 {{10-17: ''}}
}();
}
Modified: cfe/trunk/test/Analysis/edges-new.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/edges-new.mm?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/edges-new.mm (original)
+++ cfe/trunk/test/Analysis/edges-new.mm Fri Sep 6 13:55:29 2019
@@ -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 -
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/test/Analysis/objc-arc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-arc.m?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/objc-arc.m (original)
+++ cfe/trunk/test/Analysis/objc-arc.m Fri Sep 6 13:55:29 2019
@@ -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;
Modified: cfe/trunk/test/Analysis/plist-output.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plist-output.m?rev=371257&r1=371256&r2=371257&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/plist-output.m (original)
+++ cfe/trunk/test/Analysis/plist-output.m Fri Sep 6 13:55:29 2019
@@ -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) {
Added: cfe/trunk/test/Analysis/virtualcall-fixits.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/virtualcall-fixits.cpp?rev=371257&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/virtualcall-fixits.cpp (added)
+++ cfe/trunk/test/Analysis/virtualcall-fixits.cpp Fri Sep 6 13:55:29 2019
@@ -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 at -1{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
+ // expected-remark at -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>
More information about the cfe-commits
mailing list