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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/include/clang/AST/Expr.h:1447
+      bool Set, const ASTContext &Context) {
+    DeclRefExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set;
+    setDependence(computeDependence(this, Context));
----------------
erichkeane wrote:
> This bit concerns me a touch, I'm a little scared/shocked that this would matter at all... This makes me think more nefarious issues are at play here.
See https://eel.is/c++draft/temp.dep.expr#3.6 
I think this is because we don't know if the capture is mutable until the type of the object parameter is known.
I still need to find a way to devise a test for that though.

The way we do that is we save that bit and then in `computeDependence` we apply that rule.
(This bit doesn't need to be serialized once the dependence is computed)


================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:2560
       IsInLambda = true;
-    if (MD->isInstance())
+    if (MD->isImplicitObjectMemberFunction())
       HasThisQuals = true;
----------------
erichkeane wrote:
> This bit probably  needs to wait until microsoft decides what the mangling for these are (all of microsoft mangle)
I've been chatting with Cameron, microsoft has decided :)
I still have to hear back from the GCC/Itanium folks though


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:2633
+  return CGF.EmitLValueForLambdaField(FD, ThisValue);
+  // QualType TagType = CGF.getContext().getTagDeclType(FD->getParent());
+  // LValue LV = CGF.MakeNaturalAlignAddrLValue(ThisValue, TagType);
----------------
erichkeane wrote:
> Eh?
I need to remove this commented code.
the removed code was just moved around. we had 2 functions doing the same thing, now this one just forward to the non-static one.
I should get rid of it entirely actually.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4254
+                                                 llvm::Value *ThisValue) {
+  bool HasExplicitObjectParameter = false;
+  if (const CXXMethodDecl *MD =
----------------
erichkeane wrote:
> If there is a CodeGen expert in the house, I'd really hope they go through this :) 
Oh, me too!
I need to properly write tests for it too. And you know how terrible I am at code gen tests...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828