Re: [PATCH v3] c++/reflection: ICE with &template [:members_of():] [PR124794]

On 7/31/26 6:21 PM, Marek Polacek wrote:
> On Fri, Jul 31, 2026 at 12:47:08PM -0400, Jason Merrill wrote:
>> On 7/31/26 12:38 PM, Marek Polacek wrote:
>>> On Thu, Jul 30, 2026 at 09:35:26AM -0400, Jason Merrill wrote:
>>>> On 7/28/26 1:34 PM, Marek Polacek wrote:
>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/16?
>>>>>
>>>>> -- >8 --
>>>>> Given
>>>>>
>>>>>      struct C { template <class T> void f(T); };
>>>>>
>>>>> we handle "&template [:^^C::f:]" correctly because the spliced
>>>>> expression is
>>>>>
>>>>>      BASELINK<OVERLOAD<TEMPLATE_DECL f>>, binfo C>
>>>>>
>>>>> which is fine: we have an OVERLOAD around the TEMPLATE_DECL and
>>>>> lookup_member wrapped the whole thing in a BASELINK.  But when
>>>>> we're splicing members_of(^^C, ac)[0], we ended up with
>>>>>
>>>>>      OVERLOAD<TEMPLATE_DECL f>>
>>>>>
>>>>> and then go down the wrong path in cp_parser_splice_expression.
>>>>>
>>>>> splice already correctly adds the missing OVERLOAD but it also
>>>>> needs to (maybe) add a BASELINK.
>>>>
>>>> Yes, though I'm nervous about the use of currently_open_derived_class in
>>>> baselink_for_fns.  Does that cause trouble with a splice after ->?
>>>
>>> We test a splice after -> in e.g. member1.C and member3.C and they still pass.
>>>
>>> And given
>>>
>>>     struct C {
>>>       void g (int);
>>>     };
>>>
>>>     C *pc = ...;
>>>
>>> this
>>>
>>>     auto a = &pc->[: ^^C::g :];
>>>
>>> is invalid (clang++ also rejects).
>>>
>>> ...but we don't have a test for a valid &p->[: x :] so this
>>> version adds it.
>>>
>>> +void
>>> +g (C *pc)
>>> +{
>>> +  const int *p = &pc->[: ^^C::val :];
>>> +}
>> That tests the case where currently_open_derived_class is null, so it
>> doesn't matter.  How about a case where we're in a member function of a
>> class derived from C and the object argument is a different derived class?
> 
> Ah, that's a great catch.  This is where it breaks:
> 
>    struct C {
>      void g (int);
>    };
> 
>    constexpr auto ac = std::meta::access_context::current();
>    constexpr auto g1 = members_of(^^C, ac)[0];
> 
>    struct D1 : C { };
> 
>    struct D2 : C {
>      void mfn (D1 *pd)
>      {
>        pd->[:g1:] (42);
>      }
>    };
> 
> because the BASELINK has D2 as the access_binfo, but it should be
> C which is what it was when the reflection was formed.
> 
> So perhaps baselink_for_fns should ignore currently_open_derived_class
> which is what this patch does.
> 
> Tested reflect/* on x86_64-pc-linux-gnu, ok for trunk/16?
OK for trunk and 16 after it reopens.

Jason