Re: [PATCH v6] c++: Move consteval folding to cp_fold_r

[Jason Merrill <jason@redhat.com>]

On 9/15/23 16:32, Marek Polacek wrote:
> On Fri, Sep 15, 2023 at 02:08:46PM -0400, Jason Merrill wrote:
>> On 9/13/23 20:02, Marek Polacek wrote:
>>> On Wed, Sep 13, 2023 at 05:57:47PM -0400, Jason Merrill wrote:
>>>> On 9/13/23 16:56, Marek Polacek wrote:
>>>>> On Tue, Sep 12, 2023 at 05:26:25PM -0400, Jason Merrill wrote:
>>>>>> On 9/8/23 14:24, Marek Polacek wrote:
>>>>>>> +  switch (TREE_CODE (stmt))
>>>>>>> +    {
>>>>>>> +    /* Unfortunately we must handle code like
>>>>>>> +	 false ? bar () : 42
>>>>>>> +       where we have to check bar too.  */
>>>>>>> +    case COND_EXPR:
>>>>>>> +      if (cp_fold_immediate_r (&TREE_OPERAND (stmt, 1), walk_subtrees, data))
>>>>>>> +	return error_mark_node;
>>>>>>> +      if (TREE_OPERAND (stmt, 2)
>>>>>>> +	  && cp_fold_immediate_r (&TREE_OPERAND (stmt, 2), walk_subtrees, data))
>>>>>>> +	return error_mark_node;
>>>>>>
>>>>>> Is this necessary?  Doesn't walk_tree already walk into the arms of
>>>>>> COND_EXPR?
>>>>>
>>>>> Unfortunately yes.  The cp_fold call in cp_fold_r could fold the ?: into
>>>>> a constant before we see it here.  I've added a comment saying just that.
>>>>
>>>> Ah.  But in that case I guess we need to walk into the arms, not just check
>>>> the top-level expression in them.
>>> Arg, of course.  I was fooled into thinking that it would recurse, but
>>> you're right.  Fixed by using cp_walk_tree as I intended.  Tested in
>>> consteval34.C.
>>>
>>>> But maybe cp_fold_r should do that before the cp_fold, instead of this
>>>> function?
>>>
>>> I...am not sure how that would be better than what I did.
>>
>> Callers of cp_fold_immediate don't need this because cp_fold_r isn't
>> involved, so it isn't folding anything.
> 
> This is true.
>   
>> cp_fold_r can walk the arms with cp_fold_r and then clear *walk_subtrees to
>> avoid walking the arms again normally.
> 
> I didn't think we wanted to do everything cp_fold_r does even in dead
> branches, but ok.

Ah, that's a good point.  With the recursive walk in 
cp_fold_immediate_r, I suppose we could suppress it when called from 
cp_fold_immediate with a new fold_flag?  That would still allow for 
cp_walk_tree_without_duplicates.

Incidentally, I notice you check for null op2 of COND_EXPR, should 
probably also check op1.

Jason