Bug 1170388 - Restrict the static analysis error given about raw pointers to refcounted objects inside a lambda, to the case where the raw pointer is captured. r=ehsan
authorBotond Ballo <botond@mozilla.com>
Wed, 03 Jun 2015 16:51:36 -0400
changeset 247691 03ff333d49c037b4b15dbf501f177491b8e2fc0b
parent 247690 ab4fcd2c9ca6c7d2b4cb82109a0ce94fb3cef859
child 247692 eb611a75140e1e26d9930d92f27c413697c6032e
push idunknown
push userunknown
push dateunknown
reviewersehsan
bugs1170388
milestone41.0a1
Bug 1170388 - Restrict the static analysis error given about raw pointers to refcounted objects inside a lambda, to the case where the raw pointer is captured. r=ehsan
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestNoRefcountedInsideLambdas.cpp
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -594,16 +594,43 @@ AST_MATCHER(MemberExpr, isAddRefOrReleas
   return false;
 }
 
 /// This matcher will select classes which are refcounted.
 AST_MATCHER(QualType, isRefCounted) {
   return isClassRefCounted(Node);
 }
 
+#if CLANG_VERSION_FULL < 304
+
+/// The 'equalsBoundeNode' matcher was added in clang 3.4.
+/// Since infra runs clang 3.3, we polyfill it here.
+AST_POLYMORPHIC_MATCHER_P(equalsBoundNode,
+                          std::string, ID) {
+  BoundNodesTree bindings = Builder->build();
+  bool haveMatchingResult = false;
+  struct Visitor : public BoundNodesTree::Visitor {
+    const NodeType &Node;
+    std::string ID;
+    bool &haveMatchingResult;
+    Visitor(const NodeType &Node, const std::string &ID, bool &haveMatchingResult)
+      : Node(Node), ID(ID), haveMatchingResult(haveMatchingResult) {}
+    void visitMatch(const BoundNodes &BoundNodesView) override {
+      if (BoundNodesView.getNodeAs<NodeType>(ID) == &Node) {
+        haveMatchingResult = true;
+      }
+    }
+  };
+  Visitor visitor(Node, ID, haveMatchingResult);
+  bindings.visitMatches(&visitor);
+  return haveMatchingResult;
+}
+
+#endif
+
 }
 }
 
 namespace {
 
 bool isPlacementNew(const CXXNewExpr *expr) {
   // Regular new expressions aren't placement new
   if (expr->getNumPlacementArgs() == 0)
@@ -699,18 +726,24 @@ DiagnosticsMatcher::DiagnosticsMatcher()
   // Then, look for MemberExpr that need to be casted to the right type using
   // an intermediary CastExpr before we get to the CallExpr.
   astMatcher.addMatcher(callExpr(callee(functionDecl(hasNoAddRefReleaseOnReturnAttr()).bind("func")),
                                  hasParent(castExpr(hasParent(memberExpr(isAddRefOrRelease(),
                                                                          hasParent(callExpr())).bind("member"))))
       ).bind("node"),
     &noAddRefReleaseOnReturnChecker);
 
+  // Match declrefs with type "pointer to object of ref-counted type" inside a
+  // lambda, where the declaration they reference is not inside the lambda.
+  // This excludes arguments and local variables, leaving only captured
+  // variables.
   astMatcher.addMatcher(lambdaExpr(
-            hasDescendant(declRefExpr(hasType(pointerType(pointee(isRefCounted())))).bind("node"))
+            hasDescendant(declRefExpr(hasType(pointerType(pointee(isRefCounted()))),
+                                      to(decl().bind("decl"))).bind("declref")),
+            unless(hasDescendant(decl(equalsBoundNode("decl"))))
         ),
     &refCountedInsideLambdaChecker);
 
   // Older clang versions such as the ones used on the infra recognize these
   // conversions as 'operator _Bool', but newer clang versions recognize these
   // as 'operator bool'.
   astMatcher.addMatcher(methodDecl(anyOf(hasName("operator bool"),
                                          hasName("operator _Bool"))).bind("node"),
@@ -912,24 +945,24 @@ void DiagnosticsMatcher::NoAddRefRelease
 
   Diag.Report(node->getLocStart(), errorID) << func << method;
 }
 
 void DiagnosticsMatcher::RefCountedInsideLambdaChecker::run(
     const MatchFinder::MatchResult &Result) {
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Error, "Refcounted variable %0 of type %1 cannot be used inside a lambda");
+      DiagnosticIDs::Error, "Refcounted variable %0 of type %1 cannot be captured by a lambda");
   unsigned noteID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Note, "Please consider using a smart pointer");
-  const DeclRefExpr *node = Result.Nodes.getNodeAs<DeclRefExpr>("node");
+  const DeclRefExpr *declref = Result.Nodes.getNodeAs<DeclRefExpr>("declref");
 
-  Diag.Report(node->getLocStart(), errorID) << node->getFoundDecl() <<
-    node->getType()->getPointeeType();
-  Diag.Report(node->getLocStart(), noteID);
+  Diag.Report(declref->getLocStart(), errorID) << declref->getFoundDecl() <<
+    declref->getType()->getPointeeType();
+  Diag.Report(declref->getLocStart(), noteID);
 }
 
 void DiagnosticsMatcher::ExplicitOperatorBoolChecker::run(
     const MatchFinder::MatchResult &Result) {
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Error, "bad implicit conversion operator for %0");
   unsigned noteID = Diag.getDiagnosticIDs()->getCustomDiagID(
--- a/build/clang-plugin/tests/TestNoRefcountedInsideLambdas.cpp
+++ b/build/clang-plugin/tests/TestNoRefcountedInsideLambdas.cpp
@@ -14,45 +14,81 @@ struct SmartPtr {
 struct R : RefCountedBase {
   void method();
 };
 
 void take(...);
 void foo() {
   R* ptr;
   SmartPtr<R> sp;
-  take([&]() {
-    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be used inside a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([&](R* argptr) {
+    R* localptr;
+    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    argptr->method();
+    localptr->method();
   });
-  take([&]() {
+  take([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
     sp->method();
+    argsp->method();
+    localsp->method();
   });
-  take([&]() {
-    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be used inside a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([&](R* argptr) {
+    R* localptr;
+    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    take(argptr);
+    take(localptr);
   });
-  take([&]() {
+  take([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
     take(sp);
+    take(argsp);
+    take(localsp);
   });
-  take([=]() {
-    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be used inside a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([=](R* argptr) {
+    R* localptr;
+    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    argptr->method();
+    localptr->method();
   });
-  take([=]() {
+  take([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
     sp->method();
+    argsp->method();
+    localsp->method();
   });
-  take([=]() {
-    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be used inside a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([=](R* argptr) {
+    R* localptr;
+    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    take(argptr);
+    take(localptr);
   });
-  take([=]() {
+  take([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
     take(sp);
+    take(argsp);
+    take(localsp);
   });
-  take([ptr]() {
-    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be used inside a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([ptr](R* argptr) {
+    R* localptr;
+    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    argptr->method();
+    localptr->method();
   });
-  take([sp]() {
+  take([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
     sp->method();
+    argsp->method();
+    localsp->method();
   });
-  take([ptr]() {
-    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be used inside a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([ptr](R* argptr) {
+    R* localptr;
+    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    take(argptr);
+    take(localptr);
   });
-  take([sp]() {
+  take([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
     take(sp);
+    take(argsp);
+    take(localsp);
   });
 }