[analyzer] Add minimal support for fix-it hints.
authorArtem Dergachev <artem.dergachev@gmail.com>
Fri, 6 Sep 2019 20:55:29 +0000 (20:55 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Fri, 6 Sep 2019 20:55:29 +0000 (20:55 +0000)
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

llvm-svn: 371257

15 files changed:
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/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 [new file with mode: 0644]

index 0626d29..4d52655 100644 (file)
@@ -564,6 +564,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 "
                   "enabled. This option is only provided for backwards "
@@ -654,7 +659,12 @@ def DeadStoresChecker : Checker<"DeadStores">,
                   "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 9a0225e..d853fb7 100644 (file)
@@ -300,6 +300,14 @@ ANALYZER_OPTION(bool, ShouldTrackConditionsDebug, "track-conditions-debug",
                 "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, CTUImportThreshold, "ctu-import-threshold",
                 "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 76c282a..28e9e94 100644 (file)
@@ -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;
index 52d2584..6b5a2eb 100644 (file)
@@ -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() {
index e4889c1..6144188 100644 (file)
@@ -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 @@ LookThroughTransitiveAssignmentsAndCommaOperators(const Expr *Ex) {
 }
 
 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) {
index 903ba04..a210128 100644 (file)
@@ -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(const CallEvent &Call,
   }
 
   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(CheckerManager &Mgr) {
     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 3ce998f..f5d747c 100644 (file)
@@ -2928,6 +2928,9 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
         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::generateDiagnosticForConsumerMap(
 }
 
 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 04640eb..3ba050f 100644 (file)
@@ -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_ostream &o, StringRef Message,
   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_ostream &o,
     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_ostream &o, const PathDiagnosticEventPiece& P
   // 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_ostream &o, const PathDiagnosticCallPiece &P,
 
   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(raw_ostream &o,
        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_ostream &o, const PathDiagnosticNotePiece& P,
   // 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_ostream &o,
   // 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 0dcb9db..9f03035 100644 (file)
@@ -83,11 +83,11 @@ void ento::createTextPathDiagnosticConsumer(
 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");
-
-    for (std::vector<const PathDiagnostic*>::iterator I = Diags.begin(),
-         E = Diags.end(); I != E; ++I) {
+    unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
+
+    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();
 
index c8a24ee..6c6883d 100644 (file)
@@ -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 26377f7..cbfdabd 100644 (file)
@@ -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@-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 @@ void f5() {
   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 @@ void f23_pos(int argc, char **argv) {
   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 @@ void f24_A(int y) {
     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 dda1bfb..6bddbef 100644 (file)
@@ -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 cab6618..7127232 100644 (file)
@@ -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 3291633..553982b 100644 (file)
@@ -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) {
diff --git a/clang/test/Analysis/virtualcall-fixits.cpp b/clang/test/Analysis/virtualcall-fixits.cpp
new file mode 100644 (file)
index 0000000..ea149d5
--- /dev/null
@@ -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>