Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

LambdaExpr matchers needs link to cxxMethodDecl() or functionDecl() #36954

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR37981
Status NEW
Importance P enhancement
Reported by David Poliakoff (poliakoff1@llnl.gov)
Reported on 2018-06-28 17:02:32 -0700
Last modified on 2018-10-08 04:22:22 -0700
Version unspecified
Hardware PC All
CC alexfh@google.com, development@jonas-toth.eu, klimek@google.com, steveire@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Looking through the matcher reference here

http://clang.llvm.org/docs/LibASTMatchersReference.html

I see two matchers for lambdaExpr, one that is "lambdaExpr" itself, and one to
see if a cxxRecordDecl is a lambda (isLambda()). What I really need is a way to
get a handle to the cxxMethodDecl() underlying the lambda, in a plugin this
happens with getCallOperator (
https://clang.llvm.org/doxygen/classclang_1_1LambdaExpr.html#a77f0b434b245a0a099a6317de2a7244e
). getLambdaClass() equivalents would be nice as well, perhaps called
hasCallOperator and hasClass for matcher names.

This is really breaking a process we have for identifying bugs in our codes, a
fix would be greatly appreciated.

In terms of test codes

int main(){
  [=](){};
}

Is enough to show that you can't get the cxxMethodDecl from the lambdaExpr

match lambdaExpr(hasDescendant(cxxMethodDecl()))
match lambdaExpr(hasDescendant(cxxRecordDecl()))

Even though (with output "dump" on)

match(lambdaExpr()) returns

LambdaExpr 0x2aaaad49c900
</g/g0/dzpolia/lammps/src/KOKKOS/npair_kokkos.cpp:119:29, col:40> 'class
(lambda at /g/g0/dzpolia/lammps/src/KOKKOS/npair_kokkos.cpp:119:29)'
|-CXXRecordDecl 0x2aaaad49c6f0 <col:29> col:29 implicit class definition
| |-CXXMethodDecl 0x2aaaad49c830 <col:40> col:29 used operator() 'void (int)
const' inline
......
Quuxplusone commented 6 years ago

Also, I don't know if this is considered off-topic for the mailing list, but I did want to comment on how incredibly helpful clang-query has been, you've really enabled some cool workflows here, thanks!

Quuxplusone commented 6 years ago
LambdaExpr::children() doesn't contain the implicit declarations of the lambda
type and its operator(). You would need to add a couple of matchers for
getLambdaClass() and getCallOperator().

Something along the lines of:

AST_MATCHER_P(LambdaExpr, hasLambdaClass,
              internal::Matcher<CXXRecordDecl>, InnerMatcher) {
  return InnerMatcher.matches(*Node.getLambdaClass(), Finder, Builder);
}

+ docs, tests and registration for dynamic matchers.

See https://reviews.llvm.org/D28034 for an example.

Patches are welcome! ;)
Quuxplusone commented 6 years ago

Alexander,

Thanks for the sample patch, I'll see if I can make time to write this. It's really impressive how easy it is to add these matchers, I think the SVN checkout will take longer than writing the code

Orthogonal to this issue, is there anything similar to how clang-tidy has families of checkers to allow people to contribute families of matchers to clang-query? I'd like to write some matchers for our libraries, but they don't make sense in every build of clang-tools-extra. I'm of course not opposed to maintaining those as a patch against our local Clang installs, but if there's anything like the clang-tidy "packages" I'd love to use that

Quuxplusone commented 5 years ago
> Orthogonal to this issue, is there anything similar to how clang-tidy has
> families of checkers to allow people to contribute families of matchers to
> clang-query? I'd like to write some matchers for our libraries, but they
> don't make sense in every build of clang-tools-extra. I'm of course not
> opposed to maintaining those as a patch against our local Clang installs,
> but if there's anything like the clang-tidy "packages" I'd love to use that

The is no such package for matchers. What would be possible of course is to add
primitives to clang, that can then be used by your specific matchers.
Some matchers are configurable as well, e.g. a list of typenames could be
supplied to some matcher.
This might make it easier to maintain patches.

clang-tidy itself does have private matchers for specific tasks in some checks.
It might make sense to maintain a clang-tidy module that contains these
matchers.