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

Reply via email to