[PATCH] D120959: [clang][AST matchers] new hasExpectedReturnType submatcher for ReturnStmts

2022-03-16 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120959/new/

https://reviews.llvm.org/D120959

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120959: [clang][AST matchers] new hasExpectedReturnType submatcher for ReturnStmts

2022-03-15 Thread Alister Johnson via Phabricator via cfe-commits 
ajohnson-uoregon added a comment.

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)
  ))


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120959/new/

https://reviews.llvm.org/D120959

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120959: [clang][AST matchers] new hasExpectedReturnType submatcher for ReturnStmts

2022-03-04 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120959/new/

https://reviews.llvm.org/D120959

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120959: [clang][AST matchers] new hasExpectedReturnType submatcher for ReturnStmts

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits 
ajohnson-uoregon created this revision.
ajohnson-uoregon added reviewers: klimek, jdoerfert.
Herald added a project: All.
ajohnson-uoregon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120959

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -314,6 +314,7 @@
   REGISTER_MATCHER(hasEitherOperand);
   REGISTER_MATCHER(hasElementType);
   REGISTER_MATCHER(hasElse);
+  REGISTER_MATCHER(hasExpectedReturnType);
   REGISTER_MATCHER(hasExplicitSpecifier);
   REGISTER_MATCHER(hasExternalFormalLinkage);
   REGISTER_MATCHER(hasFalseExpression);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3965,6 +3965,44 @@
   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,
+InnerMatcher) {
+  const auto  = Finder->getASTContext().getParents(Node);
+
+  llvm::SmallVector Stack(Parents.begin(), Parents.end());
+  const FunctionDecl* FuncDeclNode;
+  while (!Stack.empty()) {
+const auto  = Stack.back();
+Stack.pop_back();
+FuncDeclNode = CurNode.get();
+if (FuncDeclNode != nullptr) {
+  break;
+}
+else {
+  for (const auto  : 
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:


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -314,6 +314,7 @@
   REGISTER_MATCHER(hasEitherOperand);
   REGISTER_MATCHER(hasElementType);
   REGISTER_MATCHER(hasElse);
+  REGISTER_MATCHER(hasExpectedReturnType);
   REGISTER_MATCHER(hasExplicitSpecifier);
   REGISTER_MATCHER(hasExternalFormalLinkage);
   REGISTER_MATCHER(hasFalseExpression);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3965,6 +3965,44 @@
   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,
+InnerMatcher) {
+  const auto  = Finder->getASTContext().getParents(Node);
+
+  llvm::SmallVector Stack(Parents.begin(), Parents.end());
+  const FunctionDecl* FuncDeclNode;
+  while (!Stack.empty()) {
+const auto  = Stack.back();
+Stack.pop_back();
+FuncDeclNode = CurNode.get();
+if (FuncDeclNode != nullptr) {
+  break;
+}
+else {
+  for (const auto  : 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:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits