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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I did a look through, I'm quite sure I'm not going to be able to accept this, but I'm hoping the little feedback I gave helped. I can't help with the review of the mangling unfortunately, we probably need to wait for the owners of the manglings to make a decision.



================
Comment at: clang/include/clang/AST/Decl.h:1815
+  void setIsExplicitObjectParameter(bool isExplicitObjectParam,
+                                    SourceLocation Loc = SourceLocation()) {
+    ParmVarDeclBits.IsExplicitObjectParameter = isExplicitObjectParam;
----------------
What cases are there where we wouldn't have a source location here?  Is that case common enough that this should be a default parameter?  


================
Comment at: clang/include/clang/AST/Expr.h:1447
+      bool Set, const ASTContext &Context) {
+    DeclRefExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set;
+    setDependence(computeDependence(this, Context));
----------------
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.


================
Comment at: clang/include/clang/Sema/Sema.h:2130
                                 const FunctionProtoType *Superset,
+                                unsigned SkipSupersetFirstParameter,
                                 SourceLocation SuperLoc,
----------------
These params are definitely going to need documentation, I have no idea what that means.


================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:2560
       IsInLambda = true;
-    if (MD->isInstance())
+    if (MD->isImplicitObjectMemberFunction())
       HasThisQuals = true;
----------------
This bit probably  needs to wait until microsoft decides what the mangling for these are (all of microsoft mangle)


================
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);
----------------
Eh?


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4254
+                                                 llvm::Value *ThisValue) {
+  bool HasExplicitObjectParameter = false;
+  if (const CXXMethodDecl *MD =
----------------
If there is a CodeGen expert in the house, I'd really hope they go through this :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828