Hi Jason,
On 2 Oct 2024, at 0:18, Jason Merrill wrote:
> On 10/1/24 12:44 PM, Simon Martin wrote:
>> Hi Jason,
>>
>> On 30 Sep 2024, at 19:56, Jason Merrill wrote:
>>
>>> On 9/23/24 4:44 AM, Simon Martin wrote:
>>>> Hi Jason,
>>>>
>>>> On 20 Sep 2024, at 18:01, Jason Merrill wrote:
>>>>
>>>>> On 9/20/24 5:21 PM, Simon Martin wrote:
>>>>>> The following code triggers an ICE
>>>>>>
>>>>>> === cut here ===
>>>>>> class base {};
>>>>>> class derived : virtual public base {
>>>>>> public:
>>>>>> template<typename Arg> constexpr derived(Arg) {}
>>>>>> };
>>>>>> int main() {
>>>>>> derived obj(1.);
>>>>>> }
>>>>>> === cut here ===
>>>>>>
>>>>>> The problem is that cxx_bind_parameters_in_call ends up
>>>>>> attempting
>>>>>> to
>>>>
>>>>>> convert a REAL_CST (the first non artificial parameter) to
>>>>>> INTEGER_TYPE
>>>>>> (the type of the __in_chrg parameter), which ICEs.
>>>>>>
>>>>>> This patch teaches cxx_bind_parameters_in_call to handle the
>>>>>> __in_chrg
>>>>>> and __vtt_parm parameters that {con,de}structors might have.
>>>>>>
>>>>>> Note that in the test case, the constructor is not
>>>>>> constexpr-suitable,
>>>>>> however it's OK since it's a template according to my read of
>>>>>> paragraph
>>>>>> (3) of [dcl.constexpr].
>>>>>
>>>>> Agreed.
>>>>>
>>>>> It looks like your patch doesn't correct the mismatching of
>>>>> arguments
>>>>> to parameters that you describe, but at least for now it should be
>>>>> enough to set *non_constant_p and return if we see a VTT or
>>>>> in-charge
>>>>> parameter.
>>>>>
>>>> Thanks, it’s true that my initial patch was wrong in that we’d
>>>> leave
>>>> cxx_bind_parameters_in_call thinking the expression was actually a
>>>> constant expression :-/
>>>>
>>>> The attached revised patch follows your suggestion (thanks!).
>>>> Successfully tested on x86_64-pc-linux-gnu. OK for trunk?
>>>
>>> After this patch I'm seeing a regression on constexpr-dynamic10.C
>>> with
>>> -fimplicit-constexpr; we also need to give an error here when
>>> (!ctx->quiet).
>> Thanks, good catch. TIL about --stds=impcx...
>>
>> The attached patch fixes the issue, and was successfully tested on
>> x86_64-pc-linux-gnu, including with “make -C gcc -k
>> check-c++-all”.
>> OK for trunk?
>>
>> Note that it includes a new test that’s basically a copy of
>> constexpr-dynamic10.C, with -fimplicit-constexpr. Is that the right
>> thing to do or should I just leave the test suite as is, knowing that
>> someone/something will run the test suite with --stds=impcx at some
>> point before a release?
>
> I think leave it as is.
OK, it’s simpler :-)
>
>> And the next natural question is whether I should have also tested
>> with
>> --stds=impcx before my initial submission?
>
> Probably a good idea when messing with constexpr.
ACK.
>
>> https://gcc.gnu.org/contribute.html#testing advises to test with
>> “make
>> -k check”; maybe we need to also mention check-c++-all for changes
>> to
>> the C++ front-end?
>
> Sounds good.
I sent
https://gcc.gnu.org/pipermail/gcc-patches/2024-October/664275.html for
that.
>
>> + && constexpr_error (cp_expr_loc_or_input_loc (t),
>> + /*constexpr_fundef_p=*/false,
>> + "call to non-%<constexpr%> function %qD", fun))
>
> Let's use "error_at" here, like in cxx_eval_call_expression;
> constexpr_error with fundef_p false is equivalent.
>
> OK with those adjustments.
Thanks. Merged as r15—4026.
Simon
