Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[waffl3x <waffl3x@protonmail.com>]

OKAY, I figured out SOMETHING, I think this should be fine. As noted in
the comments, this might be a better way of handling the static lambda
case too. This is still a nasty hack so it should probably be done
differently, but I question if making a whole new fntype node in
tsubst_lambda_expr makes sense. On the other hand, maybe there will be
problems if a lambda is already instantiated? I'm not sure, mutating
things this way makes me uneasy though.

I don't like that pt.cc feels like it has a ton of hidden mutations,
it's really hard to follow through it. Would you agree it's in need for
cleanup or am I just not experienced enough in this area yet?

I still have to write good tests for this case and move the error
handling for unrelated object types into here, but my gut tells me this
issue should be fixed now.

Regarding the error handling, I just had a thought about it, I have a
hunch it definitely needs to go in tsubst_template_decl or
tsubst_function_decl. There might need to be more changes to determine
the actual type of the lambda in there, but everything else I've done
changes the implicit object argument to be treated more like a regular
argument, doing error handling for the object type in
tsubst_lambda_expr would be inconsistent with that.

Also, before I forget, you were totally right about just using
dependent_type_p in cp_parser_lambda_declarator_opt. It really is just
that simple.

The code below is not necessarily final, but I wanted to get your
thoughts about doing it this way given my concerns written above. Now
that this problem is out of the way, or at least almost out of the way,
I hopefully can get a version pushed out today.

Alex

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 86c95b278ba..ea2db5c0c9d 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -14407,14 +14407,20 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain,
       gen_tmpl = NULL_TREE;
       argvec = NULL_TREE;
     }
-
+  /* We hack TYPE_METHOD_BASETYPE onto xobj member functions in
+     tsubst_lambda_expr to get the proper closure type here.  */
   tree closure = (lambda_fntype ? TYPE_METHOD_BASETYPE (lambda_fntype)
                   : NULL_TREE);
   tree ctx = closure ? closure : DECL_CONTEXT (t);
   bool member = ctx && TYPE_P (ctx);
 
   /* If this is a static lambda, remove the 'this' pointer added in
-     tsubst_lambda_expr now that we know the closure type.  */
+     tsubst_lambda_expr now that we know the closure type.
+     I suspect that we can just carry this information down in
+     TYPE_METHOD_BASETYPE without building the full method type in
+     tsubst_lambda_expr.  This wouldn't be ideal but neither is this.
+     Since that field isn't used for anything for static or xobj member
+     functions, it should be fine to do that.  */
   if (lambda_fntype && DECL_STATIC_FUNCTION_P (t))
     lambda_fntype = static_fn_type (lambda_fntype);
 
@@ -14490,12 +14496,12 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain,
     DECL_NAME (r) = make_conv_op_name (TREE_TYPE (type));
 
   tree parms = DECL_ARGUMENTS (t);
-  if (closure && !DECL_STATIC_FUNCTION_P (t))
+  if (closure && DECL_IOBJ_MEMBER_FUNC_P (t))
     parms = DECL_CHAIN (parms);
   parms = tsubst (parms, args, complain, t);
   for (tree parm = parms; parm; parm = DECL_CHAIN (parm))
     DECL_CONTEXT (parm) = r;
-  if (closure && !DECL_STATIC_FUNCTION_P (t))
+  if (closure && DECL_IOBJ_MEMBER_FUNC_P (t))
     {
       tree tparm = build_this_parm (r, closure, type_memfn_quals (type));
       DECL_NAME (tparm) = closure_identifier;
@@ -19491,9 +19497,16 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
       cp_evaluated ev;
 
       /* Fix the type of 'this'.  */
-      fntype = build_memfn_type (fntype, type,
-				 type_memfn_quals (fntype),
-				 type_memfn_rqual (fntype));
+      if (DECL_IOBJ_MEMBER_FUNC_P (oldfn))
+	fntype = build_memfn_type (fntype, type,
+				   type_memfn_quals (fntype),
+				   type_memfn_rqual (fntype));
+      /* We don't use this field anywhere else for xobj member functions, and
+	 we need a way to pass this information down.  Theres some really
+	 convoluted stuff going on unfortunately, and I can only begin to
+	 unravel it all.   */
+      if (DECL_XOBJ_MEMBER_FUNC_P (oldfn))
+	TYPE_METHOD_BASETYPE (fntype) = type;
       tree inst = (oldtmpl
                    ? tsubst_template_decl (oldtmpl, args, complain,
                                            fntype, tparms)