rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land.
Alright, LGTM. ================ Comment at: clang/include/clang/AST/DeclCXX.h:1013 CXXMethodDecl *getLambdaStaticInvoker() const; + CXXMethodDecl *getLambdaStaticInvoker(CallingConv CC) const; + ---------------- erichkeane wrote: > erichkeane wrote: > > rjmccall wrote: > > > Probably worth clarifying in the comment which invoker is returned by the > > > no-arguments variant. > > I just looked and the first is actually only used 1x! I think I can just > > replace that usage and remove the former overload if that is acceptable to > > you. > On second thought... I might leave this alone. Otherwise the ItaniumMangler > basically needs to reproduce the functionality. I think the improved comment > is an appropriate change here. Fine by me. ================ Comment at: clang/lib/AST/DeclCXX.cpp:1550 + return FTy->getCallConv() == CC; + }); + ---------------- erichkeane wrote: > rjmccall wrote: > > This seems both rather more complicated and rather more expensive than a > > simple loop which assigns to a local pointer. > Do you mean the filtering copy-if? The idea was to make it so that the > assert below still applies. If that isn't necessary, this DOES simplify to a > std::find and a return. I mean that if you really want to assert that, you could do so by checking whether the existing function is the same as what you've already found; you don't need a vector. But just not checking that seems fine to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89559/new/ https://reviews.llvm.org/D89559 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
