[PATCH] D94865: [ASTMatchers] Add callOrConstruct matcher

Stephen Kelly via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 19 13:00:54 PST 2021


steveire added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2867
+extern const internal::MapAnyOfMatcher<CallExpr, CXXConstructExpr>
+    callOrConstruct;
+
----------------
aaron.ballman wrote:
> I'm not super keen on this name. It's certainly descriptive, but I do wonder if it's a bit too specific and should perhaps be something more like `callableExpr()`, `callLikeExpr()`, or something more generic. For instance, I could imagine wanting this to match on something like:
> ```
> struct S {
>   void setter(int val) {}
>   __declspec(property(put = setter)) int x;
> };
> 
> int main() {
>   S s;
>   s.x = 12; // Match here
>   // Because the above code actually does this:
>   // s.setter(12);
> }
> ```
> because this also has an expression that isn't really a call (as far as our AST is concerned) but is a call as far as program semantics are concerned. I'm not suggesting to make the matcher support that right now (unless you felt like doing it), but thinking about the future and avoiding a name that may paint us into a corner.
> 
> WDYT about using a more generic name?
I haven't seen code like that before (ms extension?) https://godbolt.org/z/anvd43 but I think that should be matched by `binaryOperator` instead. That already matches based on what the code looks like, rather than what it is in the AST.

This `callOrConstruct` is really for using `hasArgument` and related submatchers with nodes which support it. As such I think the name is fine. I don't like `callableExpr` or `callLikeExpr` because they don't bring to mind the possibility that construction is also supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94865



More information about the cfe-commits mailing list