[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 3 inline comments as done.
cor3ntin added inline comments.


================
Comment at: clang/include/clang/AST/ASTLambda.h:45
+  return isLambdaCallOperator(DC) &&
+         !cast<CXXMethodDecl>(DC)->getType().isNull() &&
+         !cast<CXXMethodDecl>(DC)->isExplicitObjectMemberFunction();
----------------
erichkeane wrote:
> Under what cases does this end up being null?  It seems like this condition shouldn't be necessary, and it doesn't seem like we should have a case where we create a method/function without a type set?
If you have invalid captures for example, you can end up with no type. (but we do need to create a method because you need to have a context to which attach the captures to)


================
Comment at: clang/test/CodeGenCXX/cxx2b-deducing-this.cpp:4
+
+struct TrivialStruct {
+    void explicit_object_function(this TrivialStruct) {}
----------------
aaron.ballman wrote:
> I'd like to see more codegen tests in general -- for example, a test that demonstrates we properly handle code like:
> ```
> struct B {
>   virtual void f();
> };
> 
> struct T {};
> struct D3 : B {
>   void f(this T&); // Okay, not an override
> };
> 
> void func() {
>   T t;
>   t.f(); // Verify this calls D3::f() and not B::f()
> }
> ```
> but also tests that show that we do the correct thing for calling conventions (do explicit object parameter functions act as `__fastcall` functions?), explicit object parameters in lambdas, call through a pointer to member function, and so on.
> 
> Another test that could be interesting is how chained calls look (roughly):
> ```
> struct S {
>   void foo(this const S&);
> };
> 
> struct T {
>   S bar(this const &T);
> };
> 
> void func() {
>   T t;
>   t.bar().foo();
> }
> ```
That first example is ill-formed (it is an override, not allowed)

I will need help for codegen tests.
For `__thiscall`, are we even doing the same thing? https://compiler-explorer.com/z/KTea6W36T


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828