steveire added inline comments.
================ Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2065 if (const CXXRecordDecl *RD = MD->getParent()) { - if (RD->isLambda()) { + if (RD->isLambda() && RD->getLambdaCallOperator() == MD) { VisitBody = VisitBody && getDerived().shouldVisitLambdaBody(); ---------------- rsmith wrote: > In principle there can be multiple declarations of the lambda call operator > if we merge multiple lambdas from different modules; we should check for any > declaration of the `operator()` here rather than the exact one that > `getLambdaCallOperator` returns. > > I'd previously thought we could use `isLambdaCallOperator` here, but I think > that's wrong: for a generic lambda, that would skip instantiations of the > templated `operator()`, whereas I think you only want to skip the body of the > primary template in that case, and still visit the instantiations, right? Eg, > given: > > ``` > auto x = [](auto n) { return n; }; > int n = x(0); > ``` > ... you'd want to skip the body of the template `<lambda>::operator()(T)`, > but you'd still want to visit the body of `<lambda>::operator()<int>(int)`, > right? Yes, the instantiations should still be visited here. ================ Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:485 + A a; + auto l = [a] { }; + auto lCopy = l; ---------------- rsmith wrote: > steveire wrote: > > I don't know how to create a lambda with a default ctor body. > I think that's probably not actually possible, sorry for misleading you on > that! You can introduce a copy constructor with a body (by giving `A` a > non-trivial copy constructor), though, if you want to test that. Thanks! I think I have the copy-ctor case covered already. Is something missing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95644/new/ https://reviews.llvm.org/D95644 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits