[analyzer] Add extra notes to ObjCDeallocChecker

The report is now highlighting instance variables and properties
referenced by the warning message with the help of the
extra notes feature recently introduced in r283092.

Differential Revision: https://reviews.llvm.org/D24915

llvm-svn: 283093

1 files changed, 29 insertions, 3 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
index ffb0adc8d63..8fd36b82866 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -107,6 +107,9 @@ class ObjCDeallocChecker
   std::unique_ptr<BugType> ExtraReleaseBugType;
   std::unique_ptr<BugType> MistakenDeallocBugType;
 
+  static constexpr const char *MsgDeclared = "Property is declared here";
+  static constexpr const char *MsgSynthesized = "Property is synthesized here";
+
 public:
   ObjCDeallocChecker();
 
@@ -128,6 +131,9 @@ public:
   void checkEndFunction(CheckerContext &Ctx) const;
 
 private:
+  void addNoteForDecl(std::unique_ptr<BugReport> &BR, StringRef Msg,
+                           const Decl *D) const;
+
   void diagnoseMissingReleases(CheckerContext &C) const;
 
   bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M,
@@ -489,6 +495,18 @@ ProgramStateRef ObjCDeallocChecker::checkPointerEscape(
   return State;
 }
 
+/// Add an extra note piece describing a declaration that is important
+/// for understanding the bug report.
+void ObjCDeallocChecker::addNoteForDecl(std::unique_ptr<BugReport> &BR,
+                                             StringRef Msg,
+                                             const Decl *D) const {
+  ASTContext &ACtx = D->getASTContext();
+  SourceManager &SM = ACtx.getSourceManager();
+  PathDiagnosticLocation Pos = PathDiagnosticLocation::createBegin(D, SM);
+  if (Pos.isValid() && Pos.asLocation().isValid())
+    BR->addNote(Msg, Pos, D->getSourceRange());
+}
+
 /// Report any unreleased instance variables for the current instance being
 /// dealloced.
 void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const {
@@ -586,6 +604,9 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const {
     std::unique_ptr<BugReport> BR(
         new BugReport(*MissingReleaseBugType, OS.str(), ErrNode));
 
+    addNoteForDecl(BR, MsgDeclared, PropDecl);
+    addNoteForDecl(BR, MsgSynthesized, PropImpl);
+
     C.emitReport(std::move(BR));
   }
 
@@ -689,11 +710,12 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue,
          );
 
   const ObjCImplDecl *Container = getContainingObjCImpl(C.getLocationContext());
-  OS << "The '" << *PropImpl->getPropertyIvarDecl()
-     << "' ivar in '" << *Container;
+  const ObjCIvarDecl *IvarDecl = PropImpl->getPropertyIvarDecl();
+  OS << "The '" << *IvarDecl << "' ivar in '" << *Container;
 
+  bool ReleasedByCIFilterDealloc = isReleasedByCIFilterDealloc(PropImpl);
 
-  if (isReleasedByCIFilterDealloc(PropImpl)) {
+  if (ReleasedByCIFilterDealloc) {
     OS << "' will be released by '-[CIFilter dealloc]' but also released here";
   } else {
     OS << "' was synthesized for ";
@@ -710,6 +732,10 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue,
       new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode));
   BR->addRange(M.getOriginExpr()->getSourceRange());
 
+  addNoteForDecl(BR, MsgDeclared, PropDecl);
+  if (!ReleasedByCIFilterDealloc)
+    addNoteForDecl(BR, MsgSynthesized, PropImpl);
+
   C.emitReport(std::move(BR));
 
   return true;