[ASTMatchers] New matcher forFunction

[baloghadamsoftware]

Summary

Matcher proposed in the review of checker misc-assign-operator (name pending). Its goal is to find the direct enclosing function declaration of a statement and run the inner matcher on it. Two version is attached in this patch (thus it will not compile), to be decided which approach to take. The second one always chooses one single parent while the first one does a depth-first search upwards (thus a height-first search) and returns the first positive match of the inner matcher (thus it always returns zero or one matches, not more). Further questions: is it enough to implement it in-place, or ASTMatchersInternals or maybe ASTMatchFinder should be involved?

[aaron.ballman]

Please be sure to run clang\docs\tools\dump_ast_matchers.py to regenerate the documentation as well.

[baloghadamsoftware]

I will run it, once we are approaching the final version. This one is more of a question than a real patch.

[sbenza]

Thanks for doing this!

I think we want the version that iterates all parents.
Otherwise it will have problems with templates.
That is, you won't know which FunctionDecl you will get: the template or the instantiation. And you might need one or the other specifically.

include/clang/ASTMatchers/ASTMatchers.h

5124

I think this matcher works for Decls too, but we can leave it like this until we need the polymorphism.

5127

You should not assert() this.
Parents can be empty if the statement is not inside a function. eg a global variable initializer.

5132

If you are taking from the back, use a vector<> instead.
Or SmallVector<> to avoid allocation in most cases.

5134

Just call CurNode.get<FunctionDecl>. It'll do the dyn_cast for you.

5139

We should check for LambdaExpr here too, and try to match against LambdaExpr::getCallOperator()

5141

You don't need to get a Stmt. getParents() works with DynTypedNodes.
There might be some non-stmt nodes in the middle, like variable declarations.

5142

We should also not traverse FunctionDecls.
Eg:

void F() {
  struct S {
    void F2() {
    }
  };
}

So, on each node: if is a FunctionDecl or a LambdaExpr run the matcher, otherwise traverse.

unittests/ASTMatchers/ASTMatchersTest.cpp

5581

All of this is kind of unnecessary for the test.
The function could just be:

PosVec& operator=(const PosVec&) {
  auto x = [] { return 1; };
  return *this;
}

5596

EXPECT_TRUE(notMatches(...

5601

Please add test cases for the changes suggested to the matcher.

[baloghadamsoftware]

Updated according to the comments.

docs/LibASTMatchersReference.html

 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt;</td><td class="name" onclick="toggle('alignOfExpr0')"><a name="alignOfExpr0Anchor">alignOfExpr</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1UnaryExprOrTypeTraitExpr.html">UnaryExprOrTypeTraitExpr</a>&gt;  InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="alignOfExpr0"><pre>Same as unaryExprOrTypeTraitExpr, but only matching
 alignof.
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt;</td><td class="name" onclick="toggle('forFunction0')"><a name="forFunction0Anchor">forFunction</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="forFunction0"><pre>Matches declaration of the function the statemenet belongs to
+
+Given:
+F&amp; operator=(const F&amp; o) {
+  std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v &gt; 0; });
+  return *this;
+}
+returnStmt(forFunction(hasName("operator=")))
+  matches 'return *this'
+  but does match 'return &gt; 0'
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt;</td><td class="name" onclick="toggle('sizeOfExpr0')"><a name="sizeOfExpr0Anchor">sizeOfExpr</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1UnaryExprOrTypeTraitExpr.html">UnaryExprOrTypeTraitExpr</a>&gt;  InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="sizeOfExpr0"><pre>Same as unaryExprOrTypeTraitExpr, but only matching
 sizeof.
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1SwitchStmt.html">SwitchStmt</a>&gt;</td><td class="name" onclick="toggle('forEachSwitchCase0')"><a name="forEachSwitchCase0Anchor">forEachSwitchCase</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1SwitchCase.html">SwitchCase</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="forEachSwitchCase0"><pre>Matches each case or default statement belonging to the given switch

include/clang/ASTMatchers/ASTMatchers.h

 /// expr(nullPointerConstant())
 ///   matches the initializer for v1, v2, v3, cp, and ip. Does not match the
 ///   initializer for i.
 AST_MATCHER_FUNCTION(internal::Matcher<Expr>, nullPointerConstant) {
   return anyOf(
       gnuNullExpr(), cxxNullPtrLiteralExpr(),
       integerLiteral(equals(0), hasParent(expr(hasType(pointerType())))));
 }
+
+/// \brief Matches declaration of the function the statemenet belongs to
+///
+/// Given:
+/// \code
+/// F& operator=(const F& o) {
+///   std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v > 0; });
+///   return *this;
+/// }
+/// \endcode
+/// returnStmt(forFunction(hasName("operator=")))
+///   matches 'return *this'
+///   but does match 'return > 0'
+AST_MATCHER_P(Stmt, forFunction, internal::Matcher<FunctionDecl>,
+              InnerMatcher) {
+  const auto &Parents = Finder->getASTContext().getParents(Node);
+
+  llvm::SmallVector<ast_type_traits::DynTypedNode, 8> Stack(Parents.begin(),
+                                                            Parents.end());
+  while(!Stack.empty()) {
+    const auto &CurNode = Stack.back();
+    Stack.pop_back();
+    if(const auto *FuncDeclNode = CurNode.get<FunctionDecl>()) {
+      if(InnerMatcher.matches(*FuncDeclNode, Finder, Builder)) {
+        return true;
+      }
+    } else if(const auto *LambdaExprNode = CurNode.get<LambdaExpr>()) {
+      if(InnerMatcher.matches(*LambdaExprNode->getCallOperator(),
+                              Finder, Builder)) {
+        return true;
+      }
+    } else {
+      for(const auto &Parent: Finder->getASTContext().getParents(CurNode))
+        Stack.push_back(Parent);
+    }
+  }
+  return false;
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
 
 #endif

lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(fieldDecl);
   REGISTER_MATCHER(floatLiteral);
   REGISTER_MATCHER(forEach);
   REGISTER_MATCHER(forEachArgumentWithParam);
   REGISTER_MATCHER(forEachConstructorInitializer);
   REGISTER_MATCHER(forEachDescendant);
   REGISTER_MATCHER(forEachSwitchCase);
   REGISTER_MATCHER(forField);
+  REGISTER_MATCHER(forFunction);
   REGISTER_MATCHER(forStmt);
   REGISTER_MATCHER(friendDecl);
   REGISTER_MATCHER(functionDecl);
   REGISTER_MATCHER(functionProtoType);
   REGISTER_MATCHER(functionTemplateDecl);
   REGISTER_MATCHER(functionType);
   REGISTER_MATCHER(gnuNullExpr);
   REGISTER_MATCHER(gotoStmt);

unittests/ASTMatchers/ASTMatchersTest.cpp

 
 TEST(StatementMatcher, HasReturnValue) {
   StatementMatcher RetVal = returnStmt(hasReturnValue(binaryOperator()));
   EXPECT_TRUE(matches("int F() { int a, b; return a + b; }", RetVal));
   EXPECT_FALSE(matches("int F() { int a; return a; }", RetVal));
   EXPECT_FALSE(matches("void F() { return; }", RetVal));
 }
 
+TEST(StatementMatcher, ForFunction) {
+  const auto CppString1 =
+    "struct PosVec {"
+    "  PosVec& operator=(const PosVec&) {"
+    "    auto x = [] { return 1; };"
+    "    return *this;"
+    "  }"
+    "};";
+  const auto CppString2 =
+    "void F() {"
+    "  struct S {"
+    "    void F2() {"
+    "       return;"
+    "    }"
+    "  };"
+    "}";
+  EXPECT_TRUE(
+    matches(
+      CppString1,
+      returnStmt(forFunction(hasName("operator=")),
+                 has(unaryOperator(hasOperatorName("*"))))));
+  EXPECT_TRUE(
+    notMatches(
+      CppString1,
+      returnStmt(forFunction(hasName("operator=")),
+                 has(integerLiteral()))));
+  EXPECT_TRUE(
+    matches(
+      CppString1,
+      returnStmt(forFunction(hasName("operator()")),
+                 has(integerLiteral()))));
+  EXPECT_TRUE(matches(CppString2, returnStmt(forFunction(hasName("F2")))));
+  EXPECT_TRUE(notMatches(CppString2, returnStmt(forFunction(hasName("F")))));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang