[analyzer] Add fix-it hint support.

[NoQ]

Summary

That's a pretty wonky experiment, even if a fairly easy one. I'm trying to add support for fix-it hints to the bug reporter. In the current shape the patch does the following:

• Allow attaching fixit hints to Static Analyzer BugReports.
• Add support for fixits in text output (including the default text output that goes without notes, as the fixit "belongs" to the warning).
• Add support for fixits in the plist output mode (not sure if i'm fully supporting all kinds of fixits).
• Implement a fixit for the path-insensitive DeadStores checker (only one of the cases, and i'm still not sure it's a good fixit, but it was an obvious first thing to experiment with).
• 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).

$ clang -cc1 -analyze -analyzer-checker=core,cplusplus,optin.cplusplus test.cpp
test.cpp:4:5: warning: Call to virtual method 'S::foo' during construction
    foo();
    ^~~~~
    S::
1 warning generated.

More TODOs:

• We don't have a good way to test removal fixits (such as the one in the dead stores checker). Testing plist files is not a good way to test them (though we definitely need a few such tests). We can't test them via text output because clang itself generally doesn't display removal fixits in text output (it's kinda obvious if you look at how it prints them). In clang-tidy they use a more sophisticated facility for these tests, testing the fixed file instead, but it's deep within their custom testing scripts. We might need something similar.
• HTML output still doesn't support fixits. Not sure if they are useful in there because fixits are generally not very useful when there's no button to apply them. But there should be no harm in displaying them anyway, so i hope i'll have time to take a look.
• Need more tests with macros and such stuff.

[NoQ]

Also if we ever implement any fixits for path-sensitive reports that actually make sense, i suspect that it might as well make sense to attach fixits to individual path notes. For examples, if we report a double-free, we may attach a removal of the second free to the warning and a removal of the first free to the note that highlights the first free.

[Szelethus]

Woah, nice! How does an -analyzer-config fixits-as-warnings option sound like for more readable tests?

01 Obj obj6 = /**/; // expected-warning{{Value stored to 'obj6' during its initialization is never read}}
02                  // expected-warning@-1{{FixIt: Remove range (<filename>:1:1, <filename>:1:16)}}

[NoQ]

Unforget a FileCheck-based test file for the virtual call checker that i already had.

In 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

[baloghadamsoftware]

Hmm, I was thinking on the same for some time but I wonder how many checkers could find the correct fixits? Maybe the removal fixits of double frees or double file closes, but I am afraid that for most of our path-sensitive checks there are no obvious fixits. Even clang-tidy cannot provide a fixit for most of its findings. However, generally I like the idea, even for the few checkers it can be applied to.

[NoQ]

In D65182#1604280, @baloghadamsoftware wrote:

Hmm, I was thinking on the same for some time but I wonder how many checkers could find the correct fixits? Maybe the removal fixits of double frees or double file closes, but I am afraid that for most of our path-sensitive checks there are no obvious fixits. Even clang-tidy cannot provide a fixit for most of its findings. However, generally I like the idea, even for the few checkers it can be applied to.

I'd be pretty surprised if any path-sensitive checker would ever have a really good fixit, which is why i never was super excited about this idea when people were asking for it on the mailing lists. This is definitely mostly for syntactic checkers (think MallocSizeof or some of our Objective-C checkers). But at least it's one less artificial roadblock for people choosing where to put their checker :/

[xazax.hun]

The direction looks good to me. I wonder if it is worth to add some warnings somewhere that it is really tricky to come up with reasonable fixits for most path sensitive checks.

[NoQ]

• Hide all current fixits behind a checker option, have them off by default. They're not tested enough yet.
• Suppress the dead stores fixit when the initializer has a side effect.
• Implement fixits-as-remarks for testing purposes. This is better than nothing, but i'd rather have something similar to clang-tidy where the test applies fixits and tests the resulting code.
• Add documentation for BugReport::addFixItHint().

I plan to commit this in the current shape, hence no longer "WIP".

[Charusso]

I really like that patch, as no human would rewrite 600 dyn_cast and getAs to cast and castAs. Thanks you!

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

561

Show -> Enable?

652

Same as above.

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

308

Unsinged -> Unsigned

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

103

FixIt with capital i.

[NoQ]

Fix un-singed options.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

561

recursion, n., see "recursion"

I think it's valuable when the object and the documentation describe the same idea in different words.

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

308

:D

unsinged, adj., "not popular among singers"

Fxd.

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

103

It looks as if people write FixIt when they mean "fix-it" as an adjective (eg., "fix-it hint", "fix-it location") but Fixit when they mean "a fixit" as a noun.

[Charusso]

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

103

The name of the stuff is fix-it as properly named in the title of the review and fixit is a total different noun. I do not mind, if we pick fixit, but the tooling will remain named as fix-it.

[NoQ]

Rebase!

[Szelethus]

Hmm, why the need for checker options? Why not have them by default? If fixits are an experimental feature, maybe we should have a global enable-fixits config. But I don't insist :)

Now that we proposed that clang-tidy should also use the BugReporter, maybe we should let some clang-tidy folks chip in on this.

In any case, a direction is set and looks great.

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

870

Hmm, will this return an immutable container? If not, can we make it so?

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

743

Shouldn't we assert this?

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

84

Oh my god. This must be a runner up for "Worst class name in the Clang Static Analyzer", and might even all the trophies home. I wanted to make a joke about honorable mentions, but nothing compares :^)

clang/test/Analysis/objc-arc.m

1

Just thinking aloud, but maybe it'd be time to create a RUN: line formatter for test files, WDYT?

[NoQ]

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

870

ArrayRef is like a view/slice (like StringRef), it's lightweight and you can't mutate the original container through it and it abstracts out various implementation details of the underlying vector.

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

84

I'm pretty sure [[ https://clang.llvm.org/doxygen/classclang_1_1CXX17ElidedCopyConstructorInitializerConstructionContext.html | CXX17ElidedCopyConstructorInitializerConstructionContext ]] deserves at least some recognition.

clang/test/Analysis/objc-arc.m

1

Will this formatter be smart enough to update line numbers in expected-plists/objc-arc.m.plist? (:

[NoQ]

Add a "not implemented yet" assertion for kinds of fixits that are not supported as of that patch (but will definitely need to be supported in the future).

[Szelethus]

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

870

Consider my inline void then :)

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

84

That's at least descriptive, but I agree.

clang/test/Analysis/objc-arc.m

1

Why not put that on the wish list, eh? ;)

[NoQ]

In D65182#1629192, @Szelethus wrote:

Hmm, why the need for checker options? Why not have them by default? If fixits are an experimental feature, maybe we should have a global enable-fixits config. But I don't insist :)

I did this because fixits have a much higher quality standard that warnings (because people often apply them blindly). This means that fixit support is likely to be lagging behind in meeting the quality standards and therefore it's important to allow some (but not necessarily all) on-by-default checkers have alpha fixits.

[Szelethus]

In D65182#1630540, @NoQ wrote:

In D65182#1629192, @Szelethus wrote:

Hmm, why the need for checker options? Why not have them by default? If fixits are an experimental feature, maybe we should have a global enable-fixits config. But I don't insist :)

I did this because fixits have a much higher quality standard that warnings (because people often apply them blindly). This means that fixit support is likely to be lagging behind in meeting the quality standards and therefore it's important to allow some (but not necessarily all) on-by-default checkers have alpha fixits.

I'm sold.

[a_sidorin]

Hi Artem!
The patch looks very promising for CSA, thanks for working on this!

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

359

As I see, FixItHint is a pretty expensive class to copy. Should we consider const ref/move?

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

3142

const auto&/auto&&?

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

741

const auto&?

[gribozavr]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

363

Why is it virtual? In fact, why is BugReporter subclassable at all?

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

751

Escaping?

clang/test/Analysis/dead-stores.c

11

If the tests ignore the line number anyway, why even print it?

[NoQ]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

363

This clearly doesn't need to be virtual. I copy-pasted it from nearby functions without applying any critical thinking.

However, BugReport(er) most likely needs to be sub-classable because D66473#inline-596021, so i promise to clean those up in the process :)

[NoQ]

Fixits ♫ Fix the Fixits ♫
Yabba dabba doo ♫

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

751

Oh #@&%! Thanks.

clang/test/Analysis/dead-stores.c

11

I anyway hope that this facility is temporary(c).

[gribozavr]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

363

No need to qualify with "llvm::".

523

I'm not sure if this is the right model for fixits. Fixits are usually associated with a message that explains what the fixit does. Only in the unusual case where Clang or ClangTidy is very confident that the fixit is correct, it is attached to the warning. Most commonly, fixits are attached to notes.

Also, for IDE support, it would be really nice if we could provide short descriptions of edits themselves (e.g., "replace 'virtual' with 'override") that can be displayed to the user instead of the diff when possible -- right now we don't and tools using ClangTidy have to use a subpar UI because of that. For example, when we show UI for fixing a warning, displaying the complete diff is too much; a concise description would be a lot better.

[gribozavr]

clang/test/Analysis/dead-stores.c

11

What's the eventual replacement?

[NoQ]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

523

Fixits are usually associated with a message that explains what the fixit does. Only in the unusual case where Clang or ClangTidy is very confident that the fixit is correct, it is attached to the warning.

Wait, you guys already have fixits attached to notes? Then, yeah, i need to support this.

Also, for IDE support, it would be really nice if we could provide short descriptions of edits themselves (e.g., "replace 'virtual' with 'override") that can be displayed to the user instead of the diff when possible -- right now we don't and tools using ClangTidy have to use a subpar UI because of that.

Mmm, interesting. I've seen this IDE of ours autogenerate such messages as "replace '$code_in_removed_range' with '$inserted_code'" (while also combining multiple parts of the fixit into a single replacement under the hood) and it looked quite bearable most of the time and i silently assumed that everybody does the same thing.

I agree that a high-level description of a fixit is nice to have. But given that you're attaching fixits to notes, isn't the text of the note text itself sufficient? E.g.:

• warning: variable may be used uninitialized here
• note: initialize 'x' here to suppress the warning
  • fixit: add ' = 0' after 'x'

[NoQ]

clang/test/Analysis/dead-stores.c

11

I hope we eventually learn how to apply fixits automatically, and then FileCheck fixed files.

[NoQ]

Make fix-it hints attachable to any PathDiagnosticPiece, rather than to PathDiagnostic as a whole. This basically allows attaching a fixit to any note in the report. For now the plist consumer only supports attaching fixits to the warning itself, to path event pieces, and to extra note pieces; it's completely unclear what's the benefit of attaching fixits to other kinds of pieces.

Rebase after D66733.
Fix escaping again.
Address an inline comment.

[gribozavr]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

523

Wait, you guys already have fixits attached to notes? Then, yeah, i need to support this.

Yes, fixits are attached to notes in Clang and ClangTidy.

Mmm, interesting. I've seen this IDE of ours autogenerate such messages as "replace '$code_in_removed_range' with '$inserted_code'" (while also combining multiple parts of the fixit into a single replacement under the hood) and it looked quite bearable most of the time and i silently assumed that everybody does the same thing.

We are doing the same thing, however, it only works well when the edit text is meaningful. For example, a hypothetical "replace const with constexpr" edit looks readable. However, "insert '(*' and insert ')'" does not read well, "dereference the pointer" would be better.

I agree that a high-level description of a fixit is nice to have. But given that you're attaching fixits to notes, isn't the text of the note text itself sufficient? E.g.:

Often yes, however not always. Also, fixits can be attached to warnings themselves. For example:

warning: #includes are not sorted according to the style guide
attached fixit: <remove all #includes>, <insert the same #includes in different order>

[gribozavr]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

359

Are these fix-its notionally attached to the primary diagnostic itself?

[NoQ]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

359

Yup. I'll update the comment.

[NoQ]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

523

I see, thanks! It sounds like we might want to extend the FixItHint class itself to include an optional note.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

CmdLineOption<Boolean,
                   InAlpha>
   ]>,
   Documentation<HasAlphaDocumentation>;
 
 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 "
                   "compatibility.",
                   "false",
                   InAlpha>
   ]>,
   Dependencies<[PureVirtualCallChecker]>,

 // Deadcode checkers.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = DeadCode in {
 
 def DeadStoresChecker : Checker<"DeadStores">,
   HelpText<"Check for values stored to variables that are never read "
            "afterwards">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "ShowFixIts",
+                  "Enable fix-it hints for this checker",
+                  "false",
+                  InAlpha>
+  ]>,
   Documentation<HasDocumentation>;
 
 } // end DeadCode
 
 let ParentPackage = DeadCodeAlpha in {
 
 def UnreachableCodeChecker : Checker<"UnreachableCode">,
   HelpText<"Check unreachable code">,

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

ANALYZER_OPTION(bool, ShouldTrackConditions, "track-conditions",
                 "Whether to track conditions that are a control dependency of "
                 "an already tracked variable.",
                 false)
 
 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. "
                 "Lowering this threshold can alleviate the memory burder of "
                 "analysis with many interdependent definitions located in "
                 "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 "
     "to be small enough to always inline.",
     3)
 
 ANALYZER_OPTION(
     unsigned, GraphTrimInterval, "graph-trim-interval",

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

protected:
   PathDiagnosticLocation UniqueingLocation;
   const Decl *UniqueingDecl;
 
   const ExplodedNode *ErrorNode = nullptr;
   SmallVector<SourceRange, 4> Ranges;
   ExtraTextList ExtraText;
   NoteList Notes;
 
+  SmallVector<FixItHint, 4> Fixits;
+
   using Symbols = llvm::DenseSet<SymbolRef>;
   using Regions = llvm::DenseSet<const MemRegion *>;
 
   /// A (stack of) a set of symbols that are registered with this
   /// report as being "interesting", and thus used to help decide which
   /// diagnostics to include when constructing the final path diagnostic.
   /// The stack is largely used by BugReporter when generating PathDiagnostics
   /// for multiple PathDiagnosticConsumers.
public:
 
   /// Get the declaration containing the uniqueing location.
   const Decl *getUniqueingDecl() const {
     return UniqueingDecl;
   }
 
   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.
   /// By default, the source range of the statement corresponding to the error
   /// node will be used; add a single invalid range to specify absence of
   /// ranges.
   void addRange(SourceRange R) {
     assert((R.isValid() || Ranges.empty()) && "Invalid range can only be used "
                            "to specify that the report does not have a range.");
     Ranges.push_back(R);
   }
 
+  /// Add a fix-it hint to the bug report.
+  ///
+  /// Fix-it hints are the suggested edits to the code that would resolve
+  /// the problem explained by the bug report. Fix-it hints should be
+  /// as conservative as possible because it is not uncommon for the user
+  /// to blindly apply all fixits to their project. Note that it is very hard
+  /// to produce a good fix-it hint for most path-sensitive warnings.
+  void addFixItHint(FixItHint F) {
+    Fixits.push_back(F);
+  }
+
   /// 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.
   /// For example, they allow constructing a more elaborate trace.
   /// \sa registerConditionVisitor(), registerTrackNullOrUndefValue(),
   /// registerFindLastStore(), registerNilReceiverVisitor(), and
   /// registerVarDeclsLastStore().
   void addVisitor(std::unique_ptr<BugReporterVisitor> visitor);
 
   /// Remove all visitors attached to this bug report.
   void clearVisitors();
 
   /// Iterators through the custom diagnostic visitors.
   visitor_iterator visitor_begin() { return Callbacks.begin(); }
   visitor_iterator visitor_end() { return Callbacks.end(); }
 
public:
   /// The reports are usually generated by the checkers. Further, they are
   /// folded based on the profile value, which is done to coalesce similar
   /// reports.
   void emitReport(std::unique_ptr<BugReport> R);
 
   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;
 
   /// Returns a BugType that is associated with the given name and
   /// category.
   BugType *getBugTypeForName(CheckName CheckName, StringRef name,
                              StringRef category);

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

class PathDiagnostic : public llvm::FoldingSetNode {
   /// Important bug uniqueing location.
   /// The location info is useful to differentiate between bugs.
   PathDiagnosticLocation UniqueingLoc;
   const Decl *UniqueingDecl;
 
   /// 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;
 
   /// Return the path currently used by builders for constructing the
   /// PathDiagnostic.
   PathPieces &getActivePath() {
     if (pathStack.empty())
public:
   StringRef getShortDescription() const {
     return ShortDesc.empty() ? VerboseDesc : ShortDesc;
   }
 
   StringRef getCheckName() const { return CheckName; }
   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.
   const Decl *getDeclWithIssue() const { return DeclWithIssue; }
 
   using meta_iterator = std::deque<std::string>::const_iterator;
 
   meta_iterator meta_begin() const { return OtherDesc.begin(); }

clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

 //==- DeadStoresChecker.cpp - Check for stores to dead variables -*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines a DeadStores, a flow-sensitive checker that looks for
 //  stores to variables that are no longer live.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
if (BO->getOpcode() == BO_Comma) {
       continue;
     }
     break;
   }
   return Ex;
 }
 
 namespace {
+class DeadStoresChecker : public Checker<check::ASTCodeBody> {
+public:
+  bool ShowFixIts = false;
+
+  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+                        BugReporter &BR) const;
+};
+
 class DeadStoreObs : public LiveVariables::Observer {
   const CFG &cfg;
   ASTContext &Ctx;
   BugReporter& BR;
-  const CheckerBase *Checker;
+  const DeadStoresChecker *Checker;
   AnalysisDeclContext* AC;
   ParentMap& Parents;
   llvm::SmallPtrSet<const VarDecl*, 20> Escaped;
   std::unique_ptr<ReachableCode> reachableCode;
   const CFGBlock *currentBlock;
   std::unique_ptr<llvm::DenseSet<const VarDecl *>> InEH;
 
   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)
       : cfg(cfg), Ctx(ctx), BR(br), Checker(checker), AC(ac), Parents(parents),
         Escaped(escaped), currentBlock(nullptr) {}
 
   ~DeadStoreObs() override {}
 
   bool isLive(const LiveVariables::LivenessValues &Live, const VarDecl *D) {
void Report(const VarDecl *V, DeadStoreKind dsk,
 
     if (isSuppressed(R))
       return;
 
     SmallString<64> buf;
     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";
         LLVM_FALLTHROUGH;
       case Standard:
         if (!BugType) BugType = "Dead assignment";
         os << "Value stored to '" << *V << "' is never read";
         break;
 
       case Enclosing:
         // Don't report issues in this case, e.g.: "if (x = foo())",
         // where 'x' is unused later.  We have yet to see a case where
         // this is a real bug.
         return;
     }
 
     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,
                     DeadStoreKind dsk,
                     const LiveVariables::LivenessValues &Live) {
 
     if (!VD->hasLocalStorage())
       return;

 };
 } // end anonymous namespace
 
 
 //===----------------------------------------------------------------------===//
 // DeadStoresChecker
 //===----------------------------------------------------------------------===//
 
-namespace {
-class DeadStoresChecker : public Checker<check::ASTCodeBody> {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
+void DeadStoresChecker::checkASTCodeBody(const Decl *D, AnalysisManager &mgr,
                                          BugReporter &BR) const {
-
-    // Don't do anything for template instantiations.
-    // Proving that code in a template instantiation is "dead"
-    // means proving that it is dead in all instantiations.
-    // This same problem exists with -Wunreachable-code.
-    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
-      if (FD->isTemplateInstantiation())
-        return;
 
-    if (LiveVariables *L = mgr.getAnalysis<LiveVariables>(D)) {
-      CFG &cfg = *mgr.getCFG(D);
-      AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D);
-      ParentMap &pmap = mgr.getParentMap(D);
-      FindEscaped FS;
-      cfg.VisitBlockStmts(FS);
-      DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped);
-      L->runOnAllBlocks(A);
-    }
-  }
-};
-}
 
-void ento::registerDeadStoresChecker(CheckerManager &mgr) {
-  mgr.registerChecker<DeadStoresChecker>();
+void ento::registerDeadStoresChecker(CheckerManager &Mgr) {
+  auto *Chk = Mgr.registerChecker<DeadStoresChecker>();
+  Chk->ShowFixIts =
+      Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "ShowFixIts");
 }
 
 bool ento::shouldRegisterDeadStoresChecker(const LangOptions &LO) {
   return true;
 }

clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

 } // end namespace llvm
 
 namespace {
 class VirtualCallChecker
     : public Checker<check::BeginFunction, check::EndFunction, check::PreCall> {
 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;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
 private:
   void registerCtorDtorCallInState(bool IsBeginFunction,
                                    CheckerContext &C) const;
void VirtualCallChecker::checkPreCall(const CallEvent &Call,
 
   const std::unique_ptr<BugType> &BT = IsPure ? BT_Pure : BT_Impure;
   if (!BT) {
     assert(BT == BT_Impure && "Pure-only check must be enabled!");
     return;
   }
 
   auto Report = llvm::make_unique<BugReport>(*BT, OS.str(), N);
+
+  if (ShowFixIts && !IsPure) {
+    // FIXME: These hints are valid only when the virtual call is made
+    // directly from the constructor/destructor. Otherwise the dispatch
+    // will work just fine from other callees, and the fix may break
+    // the otherwise correct program.
+    FixItHint Fixit = FixItHint::CreateInsertion(
+        CE->getBeginLoc(), MD->getParent()->getNameAsString() + "::");
+    Report->addFixItHint(Fixit);
+  }
+
   C.emitReport(std::move(Report));
 }
 
 void VirtualCallChecker::registerCtorDtorCallInState(bool IsBeginFunction,
                                                      CheckerContext &C) const {
   const auto *LCtx = C.getLocationContext();
   const auto *MD = dyn_cast_or_null<CXXMethodDecl>(LCtx->getDecl());
   if (!MD)

 
 void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.getChecker<VirtualCallChecker>();
   if (!Mgr.getAnalyzerOptions().getCheckerBooleanOption(
           Mgr.getCurrentCheckName(), "PureOnly")) {
     Chk->BT_Impure = llvm::make_unique<BugType>(
         Mgr.getCurrentCheckName(), "Unexpected loss of virtual dispatch",
         categories::CXXObjectLifecycle);
+    Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+        Mgr.getCurrentCheckName(), "ShowFixIts");
   }
 }
 
 bool ento::shouldRegisterPureVirtualCallChecker(const LangOptions &LO) {
   return true;
 }
 
 bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) {
   return true;
 }

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

 static std::unique_ptr<PathDiagnostic>
 generateEmptyDiagnosticForReport(BugReport *R, SourceManager &SM) {
   const BugType &BT = R->getBugType();
   return llvm::make_unique<PathDiagnostic>(
       R->getBugType().getCheckName(), R->getDeclWithIssue(),
       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) {
   if (!S)
     return nullptr;
 
   while (true) {
     S = PM.getParentIgnoreParens(S);
BugReporter::generateDiagnosticForConsumerMap(
 
   return Out;
 }
 
 void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
                                   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));
 }
 
 BugType *BugReporter::getBugTypeForName(CheckName CheckName, StringRef name,
                                         StringRef category) {
   SmallString<136> fullDesc;
   llvm::raw_svector_ostream(fullDesc) << CheckName.getName() << ":" << name
                                       << ":" << category;
   BugType *&BT = StrBugTypes[fullDesc];
   if (!BT)
     BT = new BugType(CheckName, name, category);
   return BT;
 }

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

 }
 
 PathDiagnostic::~PathDiagnostic() = default;
 
 PathDiagnostic::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)
     : 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,
                                 const SourceManager &SMgr) {
   SourceLocation CallLoc = CP->callEnter.asLocation();
 
   // If the call is within a macro, don't do anything (for now).
   if (CallLoc.isMacroID())

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

if (!filesMade->empty()) {
           o << "   <string>" << CI->second << "</string>\n";
         }
         o << "  </array>\n";
       }
     }
 
     printCoverage(D, /*IndentLevel=*/2, Fids, FM, o);
 
+    if (!D->getFixits().empty()) {
+      o << "  <key>fixits</key>\n";
+      o << "  <array>\n";
+      for (auto Hint : D->getFixits()) {
+        assert(!Hint.isNull());
+        // FIXME: Add support for InsertFromRange and BeforePreviousInsertion.
+        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";
   }
 
   o << " </array>\n";
 
   o << " <key>files</key>\n"
        " <array>\n";

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

 void ento::createTextPathDiagnosticConsumer(
     AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
     const std::string &Prefix, const clang::Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
   llvm_unreachable("'text' consumer should be enabled on ClangDiags");
 }
 
 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"; }
 
   bool supportsLogicalOpControlFlow() const override { return true; }
   bool supportsCrossFileDiagnostics() const override { return true; }
 
   PathGenerationScheme getGenerationScheme() const override {
     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 {
     unsigned WarnID =
         ShouldEmitAsError
             ? Diag.getCustomDiagID(DiagnosticsEngine::Error, "%0")
             : Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
     unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0");
+    unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
 
     for (std::vector<const PathDiagnostic*>::iterator I = Diags.begin(),
          E = Diags.end(); I != E; ++I) {
       const PathDiagnostic *PD = *I;
       SourceLocation WarnLoc = PD->getLocation().asLocation();
-      Diag.Report(WarnLoc, WarnID) << PD->getShortDescription()
-                                   << PD->path.back()->getRanges();
+      if (!FixitsAsRemarks) {
+        Diag.Report(WarnLoc, WarnID)
+            << PD->getShortDescription() << PD->path.back()->getRanges()
+            << PD->getFixits();
+      } else {
+        Diag.Report(WarnLoc, WarnID)
+            << PD->getShortDescription() << PD->path.back()->getRanges();
+        for (const FixItHint &Fixit : PD->getFixits()) {
+          llvm::SmallString<128> Str;
+          llvm::raw_svector_ostream OS(Str);
+          // FIXME: Add support for InsertFromRange and BeforePreviousInsertion.
+          OS << Fixit.RemoveRange.getBegin().printToString(
+                    Diag.getSourceManager())
+             << " - "
+             << Fixit.RemoveRange.getEnd().printToString(
+                    Diag.getSourceManager())
+             << " - '" << Fixit.CodeToInsert << "'";
+          Diag.Report(WarnLoc, RemarkID) << OS.str();
+        }
+      }
 
       // 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()
if (Opts->AnalysisDiagOpt != PD_NONE) {
       // Create the PathDiagnosticConsumer.
       ClangDiagPathDiagConsumer *clangDiags =
           new ClangDiagPathDiagConsumer(PP.getDiagnostics());
       PathConsumers.push_back(clangDiags);
 
       if (Opts->AnalyzerWerror)
         clangDiags->enableWerror();
 
+      if (Opts->ShouldEmitFixItHintsAsRemarks)
+        clangDiags->enableFixitsAsRemarks();
+
       if (Opts->AnalysisDiagOpt == PD_TEXT) {
         clangDiags->enablePaths();
 
       } else if (!OutDir.empty()) {
         switch (Opts->AnalysisDiagOpt) {
         default:
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)                    \
   case PD_##NAME:                                                              \

clang/test/Analysis/analyzer-config.c

 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
 // CHECK-NEXT: ctu-import-threshold = 100
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
+// CHECK-NEXT: deadcode.DeadStores:ShowFixIts = false
 // CHECK-NEXT: debug.AnalysisOrder:* = false
 // CHECK-NEXT: debug.AnalysisOrder:Bind = false
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PreCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXNewExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtOffsetOfExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:RegionChanges = false
 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
 // CHECK-NEXT: expand-macros = false
 // 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
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
 // CHECK-NEXT: max-symbol-complexity = 35
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: model-path = ""
 // CHECK-NEXT: notes-as-events = false
 // CHECK-NEXT: nullability:NoDiagnoseCallsToSystemHeaders = false
 // CHECK-NEXT: objc-inlining = true
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:CheckPointeeInitialization = false
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:IgnoreGuardedFields = false
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
 // 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
 // CHECK-NEXT: osx.cocoa.RetainCount:CheckOSObject = true
 // CHECK-NEXT: osx.cocoa.RetainCount:TrackNSCFStartParam = false
 // CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: report-in-main-source-file = false
 // CHECK-NEXT: serialize-stats = false
 // CHECK-NEXT: stable-report-filename = false
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: track-conditions = false
 // CHECK-NEXT: track-conditions-debug = false
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 89
+// CHECK-NEXT: num-entries = 92

clang/test/Analysis/dead-stores.c

-// RUN: %clang_analyze_cc1 -Wunused-variable -analyzer-checker=core,deadcode.DeadStores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
-// RUN: %clang_analyze_cc1 -Wunused-variable -analyzer-checker=core,deadcode.DeadStores -analyzer-store=region -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_analyze_cc1 -Wunused-variable -Wno-unreachable-code \
+// RUN:     -analyzer-checker=deadcode.DeadStores -fblocks -verify \
+// RUN:     -analyzer-config deadcode.DeadStores:ShowFixIts=true \
+// RUN:     -analyzer-config fixits-as-remarks=true \
+// RUN:     -analyzer-opt-analyze-nested-blocks %s
 
 void f1() {
   int k, y; // expected-warning{{unused variable 'k'}} expected-warning{{unused variable 'y'}}
   int abc=1;
   long idx=abc+3*5; // expected-warning {{never read}} expected-warning{{unused variable 'idx'}}
+  // expected-remark-re@-1{{{{.*}}:11 - {{.*}}:18 - ''}}
 }
 
 void f2(void *b) {
  char *c = (char*)b; // no-warning
  char *d = b+1; // expected-warning {{never read}} expected-warning{{unused variable 'd'}}
+  // expected-remark-re@-1{{{{.*}}:9 - {{.*}}:14 - ''}}
  printf("%s", c); // expected-warning{{implicitly declaring library function 'printf' with type 'int (const char *, ...)'}} \
  // expected-note{{include the header <stdio.h> or explicitly provide a declaration for 'printf'}}
 }
 
 int f();
 
 void f3() {
   int r;
void f4(int k) {
     
   k = 2;  // expected-warning {{never read}}
 }
   
 void f5() {
 
   int x = 4; // no-warning
   int *p = &x; // expected-warning{{never read}} expected-warning{{unused variable 'p'}}
+  // expected-remark-re@-1{{{{.*}}:9 - {{.*}}:13 - ''}}
 
 }
 
 //
 int f6() {
   
   int x = 4;
   ++x; // no-warning
void f23(int argc, char **argv) {
   ^{ 
      if (shouldLog) f23_aux("I did too use it!\n");
      else f23_aux("I shouldn't log.  Wait.. d'oh!\n");
   }();
 }
 
 void f23_pos(int argc, char **argv) {
   int shouldLog = (argc > 1); // expected-warning{{Value stored to 'shouldLog' during its initialization is never read}} expected-warning{{unused variable 'shouldLog'}}
+  // expected-remark-re@-1{{{{.*}}:16 - {{.*}}:28 - ''}}
   ^{ 
      f23_aux("I did too use it!\n");
   }();  
 }
 
 void f24_A(int y) {
   // FIXME: One day this should be reported as dead since 'z = x + y' is dead.
   int x = (y > 2); // no-warning
   ^ {
       int z = x + y; // expected-warning{{Value stored to 'z' during its initialization is never read}} expected-warning{{unused variable 'z'}}
+      // expected-remark-re@-1{{{{.*}}:12 - {{.*}}:19 - ''}}
   }();  
 }
 
 void f24_B(int y) {
   // FIXME: One day this should be reported as dead since 'x' is just overwritten.
   __block int x = (y > 2); // no-warning
   ^{
     // FIXME: This should eventually be a dead store since it is never read either.

clang/test/Analysis/edges-new.mm

-// 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 -
 
 //===----------------------------------------------------------------------===//
 // Forward declarations (from headers).
 //===----------------------------------------------------------------------===//
 
 typedef const struct __CFNumber * CFNumberRef;
 typedef const struct __CFAllocator * CFAllocatorRef;

clang/test/Analysis/objc-arc.m

-// 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;
 typedef struct _NSZone NSZone;
 @class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
 typedef unsigned long NSUInteger;
 
 @protocol NSObject

clang/test/Analysis/plist-output.m

-// 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) {
   int *p = 0;
   *p = 0xDEADBEEF;
 }
 
 void test_null_assign(void) {

clang/test/Analysis/virtualcall-fixits.cpp

+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:     -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN:     %s 2>&1 | FileCheck -check-prefix=TEXT %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:     -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN:     -analyzer-config fixits-as-remarks=true \
+// RUN:     -analyzer-output=plist -o %t.plist -verify %s
+// RUN: cat %t.plist | FileCheck -check-prefix=PLIST %s
+
+struct S {
+  virtual void foo();
+  S() {
+    foo();
+    // expected-warning@-1{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
+    // expected-remark-re@-2{{{{.*}}:5 - {{.*}}:5 - 'S::'}}
+  }
+  ~S();
+};
+
+// TEXT: warning: Call to virtual method 'S::foo' during construction
+// TEXT-SAME: bypasses virtual dispatch
+// TEXT-NEXT: foo();
+// TEXT-NEXT: ^~~~~
+// TEXT-NEXT: S::
+// TEXT-NEXT: 1 warning generated.
+
+// PLIST:  <key>fixits</key>
+// PLIST-NEXT:  <array>
+// PLIST-NEXT:   <dict>
+// PLIST-NEXT:    <key>remove_range</key>
+// PLIST-NEXT:    <array>
+// PLIST-NEXT:     <dict>
+// PLIST-NEXT:      <key>line</key><integer>13</integer>
+// PLIST-NEXT:      <key>col</key><integer>5</integer>
+// PLIST-NEXT:      <key>file</key><integer>0</integer>
+// PLIST-NEXT:     </dict>
+// PLIST-NEXT:     <dict>
+// PLIST-NEXT:      <key>line</key><integer>13</integer>
+// PLIST-NEXT:      <key>col</key><integer>4</integer>
+// PLIST-NEXT:      <key>file</key><integer>0</integer>
+// PLIST-NEXT:     </dict>
+// PLIST-NEXT:    </array>
+// PLIST-NEXT:    <key>insert_string</key>
+// PLIST-NEXT:    <string>S::</string>
+// PLIST-NEXT:   </dict>
+// PLIST-NEXT:  </array>