[clang][AST matchers] new hasExpectedReturnType submatcher for ReturnStmts

[ajohnson-uoregon]

Summary

new AST matcher that matches the expected return type of a ReturnStmt by looking at the enclosing function's stated return type; matches against the function's type

[aaron.ballman]

As mentioned in another review, it's not clear that this rises to the level of need to add it to the general AST matching interfaces. Can you explain the intended in-tree use cases for this?

[ajohnson-uoregon]

This is our use case: https://github.com/ajohnson-uoregon/llvm-project/blob/feature-ajohnson/clang-tools-extra/clang-rewrite/MatcherGenCallback.h#L365
I'm not sure if anyone else would want to use this, but it feels like it could be generally useful. Now that I know we can define local matchers though we could do that instead.

Basically, we want to match based on the declared return type of the function, not the actual type of the thing returned, for cases where the thing returned is cast to something else. We used it to reimplement part of clang-tidy's modernize-use-nullptr check; we needed to get the declared (template) return type so we could check if it was a pointer type even though the thing returned was an int.

We haven't gotten around to implementing this yet (and the type code is actually a little broken), but there are other cases where we might want to specifically look for ReturnStmts that return a thing of type X when the function says it returns a thing of type Y, in which case we'd be using both hasType() and hasExpectedReturnType() kind of like this:

returnStmt(allOf(
    hasReturnValue(hasType(X)), 
    hasExpectedReturnType(Y)
))

[aaron.ballman]

In D120959#3384525, @ajohnson-uoregon wrote:

This is our use case: https://github.com/ajohnson-uoregon/llvm-project/blob/feature-ajohnson/clang-tools-extra/clang-rewrite/MatcherGenCallback.h#L365
I'm not sure if anyone else would want to use this, but it feels like it could be generally useful. Now that I know we can define local matchers though we could do that instead.

I think my preference is to use a local matcher to do this for now; if we find more use cases, we can always lift it into the general matcher framework then. But this feels too special-purpose to add currently.

Basically, we want to match based on the declared return type of the function, not the actual type of the thing returned, for cases where the thing returned is cast to something else. We used it to reimplement part of clang-tidy's modernize-use-nullptr check; we needed to get the declared (template) return type so we could check if it was a pointer type even though the thing returned was an int.

We haven't gotten around to implementing this yet (and the type code is actually a little broken), but there are other cases where we might want to specifically look for ReturnStmts that return a thing of type X when the function says it returns a thing of type Y, in which case we'd be using both hasType() and hasExpectedReturnType() kind of like this:

returnStmt(allOf(
    hasReturnValue(hasType(X)), 
    hasExpectedReturnType(Y)
))

I think you should be able to do the same thing by traversal though -- the return statement is within a function declaration context, so you can walk up the tree to the functionDecl() (or blockDecl()?) to get to its declared return type information from the return statement itself. You're effectively implementing that same logic with your matcher, and the local matcher may be a cleaner implementation (I've not tried to write the matchers out myself).

clang/include/clang/ASTMatchers/ASTMatchers.h

AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, TypedefNameDecl,
                                     ValueDecl, CXXBaseSpecifier),
     internal::Matcher<QualType>, InnerMatcher, 0) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())
     return InnerMatcher.matches(QT, Finder, Builder);
   return false;
 }
 
 /// Overloaded to match the declaration of the expression's or value
 /// declaration's type.
 ///
 /// In case of a value declaration (for example a variable declaration),
 /// this resolves one layer of indirection. For example, in the value
 /// declaration "X x;", cxxRecordDecl(hasName("X")) matches the declaration of
 /// X, while varDecl(hasType(cxxRecordDecl(hasName("X")))) matches the
 /// declaration of x.
 ///
AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl,
                                     CXXBaseSpecifier),
     internal::Matcher<Decl>, InnerMatcher, 1) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())
     return qualType(hasDeclaration(InnerMatcher)).matches(QT, Finder, Builder);
   return false;
 }
 
+/// Matches the type of a return statement as declared by the enclosing
+/// function.
+///
+/// Example: returnStmt(hasExpectedReturnType(asString("int *"))) will match
+/// return 0; in
+/// \code
+///   int* foo() {
+///     return 0;
+///   }
+/// \endcode
+/// despite the return value being an IntegerLiteral.
+
+AST_MATCHER_P(ReturnStmt, hasExpectedReturnType, internal::Matcher<QualType>,
+    InnerMatcher) {
+      const auto &Parents = Finder->getASTContext().getParents(Node);
+
+      llvm::SmallVector<DynTypedNode, 8> Stack(Parents.begin(), Parents.end());
+      const FunctionDecl* FuncDeclNode;
+      while (!Stack.empty()) {
+        const auto &CurNode = Stack.back();
+        Stack.pop_back();
+        FuncDeclNode = CurNode.get<FunctionDecl>();
+        if (FuncDeclNode != nullptr) {
+          break;
+        }
+        else {
+          for (const auto &Parent : Finder->getASTContext().getParents(CurNode))
+            Stack.push_back(Parent);
+        }
+      }
+      if (FuncDeclNode == nullptr) {
+        return false;
+      }
+      else {
+        return InnerMatcher.matches(FuncDeclNode->getReturnType(), Finder, Builder);
+      }
+}
+
 /// Matches if the type location of a node matches the inner matcher.
 ///
 /// Examples:
 /// \code
 ///   int x;
 /// \endcode
 /// declaratorDecl(hasTypeLoc(loc(asString("int"))))
 ///   matches int x

clang/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasDefinition);
   REGISTER_MATCHER(hasDescendant);
   REGISTER_MATCHER(hasDestinationType);
   REGISTER_MATCHER(hasDirectBase);
   REGISTER_MATCHER(hasDynamicExceptionSpec);
   REGISTER_MATCHER(hasEitherOperand);
   REGISTER_MATCHER(hasElementType);
   REGISTER_MATCHER(hasElse);
+  REGISTER_MATCHER(hasExpectedReturnType);
   REGISTER_MATCHER(hasExplicitSpecifier);
   REGISTER_MATCHER(hasExternalFormalLinkage);
   REGISTER_MATCHER(hasFalseExpression);
   REGISTER_MATCHER(hasGlobalStorage);
   REGISTER_MATCHER(hasImplicitDestinationType);
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);