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

[waffl3x <waffl3x@protonmail.com>]

Okay within about half an hour today I think I've figured out what the
main issue is for my problem. For the static lambda case, while I don't
like the hack that's currently present, I reckon we just leave it as it
is, as it works just fine right now. For my issue, I'm not totally sure
how to tackle it, but here's what I've identified.

build_memfn_type calls into build_method_type_directly and that takes
the main variant of the passed in basetype and sets it as the
TYPE_METHOD_BASETYPE.

TYPE_METHOD_BASETYPE (t) = TYPE_MAIN_VARIANT (basetype);

So naively just bypassing build_memfn_type is not going to work, I'm
not totally sure how to solve this though. (Below)

/* Fix the type of 'this'.  */
      fntype = build_memfn_type (fntype, type,
                                   type_memfn_quals (fntype),
                                   type_memfn_rqual (fntype));

Making tree nodes in general is something I'm not very familiar with,
and struggled with every time I tried to read any of the code. So for
now I'm going to hope that the next block (below) handles everything
properly as far as creating a new node and substitution goes.

      tree inst = (oldtmpl
                   ? tsubst_template_decl (oldtmpl, args, complain,
                                           fntype, tparms)
                   : tsubst_function_decl (oldfn, args, complain, fntype));

And I'll just see if setting the decl_context of inst to type does the
trick. I guess I forgot to elaborate that tsubst_function_decl uses the
decl_context of t on the second, which ends up having template-info
attached to it. As you might have been able to deduce from what I've
said so far, I suspect that it's because I'm not setting it properly
when the function_decl is substituted into. I don't really know where
the right place to do that is though, or really why it's not happening
already, so I'm just gonna poke around, try some things, and clean it
up later if it's messy.

That's my quick update, hopefully this gets me somewhere finally.

Alex

On Thursday, November 23rd, 2023 at 11:49 PM, waffl3x <waffl3x@protonmail.com> wrote:


> 
> 
> > and this in tsubst_lambda_expr that assumes iobj:
> 
> > /* Fix the type of 'this'. */
> > fntype = build_memfn_type (fntype, type,
> > type_memfn_quals (fntype),
> > type_memfn_rqual (fntype));
> 
> 
> Unfortunately, putting a condition on this had some unforeseen
> consequences. I've been working on this about 8 hours today and I'm a
> little defeated after discovering this.
> 
> commit 39ade88fa1632c659c5c4ed065fa2b62d16a8670
> Author: Jason Merrill jason@redhat.com
> 
> Date: Tue Jan 24 15:29:35 2023 -0500
> 
> c++: static lambda in template [PR108526]
> 
> tsubst_lambda_expr uses build_memfn_type to build a METHOD_TYPE for the new
> lamba op(). This is not what we want for a C++23 static op(), but since we
> also use that METHOD_TYPE to communicate the closure type down to
> tsubst_function_decl, let's wait and turn it back at that point.
> 
> PR c++/108526
> 
> gcc/cp/ChangeLog:
> 
> gcc/cp/ChangeLog:
> 
> * pt.cc (tsubst_function_decl): Handle static lambda.
> 
> gcc/testsuite/ChangeLog:
> 
> * g++.dg/cpp23/static-operator-call5.C: New test.
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 2a4d03c5e47..51fc246ed71 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -14306,6 +14306,11 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain,
> 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. */
> + if (lambda_fntype && DECL_STATIC_FUNCTION_P (t))
> + lambda_fntype = static_fn_type (lambda_fntype);
> +
> 
> I discovered this when I decided to try a static lambda to see if that
> would help me narrow down my current problem. I was shocked to find out
> it exhibited the exact same ICE I was trying to fix. So I was going to
> go undo my changes one by one to see what it was, thankfully this was
> the first one I tried, I undid the condition I put on it, and the crash
> was gone.
> 
> Anyway, this has been my whole day so far, I am going to have to look
> deeper to decide how exactly I fix this because I don't think this hack
> that is in place at the moment is the right way to do it. The first
> idea that comes to mind is modifying the decl_context of the call
> operator, but I'm really not sure. I'm just going to take a break, eat
> some pizza, and come back to this hopefully less defeated.
> 
> Alex