On Sunday, November 26th, 2023 at 7:40 PM, Jason Merrill <ja...@redhat.com>
wrote:
>
>
> On 11/26/23 20:44, waffl3x wrote:
>
> > > > > > The other problem I'm having is
> > > > > >
> > > > > > auto f0 = [n = 5, &m](this auto const&){ n = 10; };
> > > > > > This errors just fine, the lambda is unconditionally const so
> > > > > > LAMBDA_EXPR_MUTABLE_P flag is set for the closure.
> > > > > >
> > > > > > This on the other hand does not. The constness of the captures
> > > > > > depends
> > > > > > on (I assume) LAMBDA_EXPR_MUTABLE_P so all the captures are
> > > > > > non-const
> > > > > > here.
> > > > > > auto f1 = [n = 5](this auto&& self){ n = 10; };
> > > > > > as_const(f1)();
> > > > >
> > > > > That sounds correct, why would this be an error?
> > > > >
> > > > > The constness of the captures doesn't depend on LAMBDA_EXPR_MUTABLE_P,
> > > > > it depends on the type of the object parameter, which in this case is
> > > > > non-const, so so are the captures.
> > > >
> > > > Oops, I should have just made a less terse example, you missed the
> > > > "as_const" in the call to the lambda. The object parameter should be
> > > > deduced as const here. I definitely should have made that example
> > > > better, my bad.
> > >
> > > Ah, yes, I see it now.
> >
> > I don't remember if I relayed my planned fix for this to you. My
> > current idea is to modify the tree during instantiation of the lambda's
> > body somewhere near tsubst and apply const to all it's members. This is
> > unfortunately the best idea I have so far and it feels like an awful
> > hack. I am open to better ideas, but I don't think we can do anything
> > until the template is instantiated so I think it has to be there.
>
>
> I think the answer should be in lambda_proxy_type. The case where we
> build a DECLTYPE_TYPE may need to be expanded to cover this situation.
>
> > Should I wait until I fix the issue in tsubst_lambda_expr before
> > submitting the patch? I'm fine to do it either way, just whatever you
> > prefer. If I finish cleaning up these tests before I hear back I'll go
> > ahead and submit it and then start looking at different solutions in
> > there.
>
>
> Go ahead and submit.
>
> Jason
I'm going to need to sit down and set up a proper e-mail application
once I'm done with all this, I missed your reply because it went off to
another thread. Luckily, I decided to send the patch anyway, and when I
noticed that my patch was not under the same thread I came looking for
it. Ah well, what a pain, I guess getting used to these mail list
things is just going to take time.
> > It doesn't. The issue is messing with the type of (potentially) a lot
> > of different functions. Even if it doesn't actually break anything, it
> > seems like the kind of hidden mutation that you were objecting to.
>
>
> Oh... yeah..., I see the issue now. I still don't think the solution
> used for static lambdas will work, or be painless anyhow, but if I
> can't find something better I will try to use that one.
>
> > > Well, even so, I can just clear it after it gets used as we just need
> > > it to pass the closure type down. Perhaps I should have led with this,
> > > but as it stands the version that uses TYPE_METHOD_BASETYPE bootstraps
> > > with no regressions. I'll still look deeper but I'm pretty confident in
> > > my decision here, I really don't want to try to unravel what
> > > build_memfn_type does, I would rather find a whole different way of
> > > passing that information down.
> >
> > But the existing code already works fine, it's just a question of
> > changing the conditions so we handle xob lambdas the same way as static.
>
>
> I'm still concerned it wont cooperate with xobj parameters of unrelated
> type, but like I said, you've indicated my solution is definitely wrong
> so I'll look at fixing it.
I spent some time looking at it, I've decided you're probably right
that handling this the same way as the static lambda case is the best
in the short term. I still don't like it, but I've gone ahead and made
that change, and it seems to work just fine. I still find it icky, but
once I realized we do in fact need lambda_fntype since it might have
been substituted into in tsubst_lambda_expr, I don't see any better way
of doing this at the moment.
Since the added parameter just gets popped off by static_fn_type, and
tsubst_lambda_expr doesn't touch the xobj parameter, I'm pretty sure it
should behave properly. So no problems I guess, moving on to the
captures bug.
Alex
