Re: [PATCH] fortran: Avoid infinite self-recursion [PR105381]

[Mikael Morin <morin-mikael@orange.fr>]

Le 26/04/2022 à 15:32, Jakub Jelinek a écrit :
> On Tue, Apr 26, 2022 at 03:22:08PM +0200, Tobias Burnus wrote:
>> LGTM - however:
>>
>> On 26.04.22 14:38, Mikael Morin wrote:
>>> --- a/gcc/fortran/trans-array.cc
>>> +++ b/gcc/fortran/trans-array.cc
>>> @@ -3698,7 +3698,8 @@ non_negative_strides_array_p (tree expr)
>>>      if (DECL_P (expr)
>>>          && DECL_LANG_SPECIFIC (expr))
>>>        if (tree orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr))
>>> -      return non_negative_strides_array_p (orig_decl);
>>> +      if (orig_decl != expr)
>>> +     return non_negative_strides_array_p (orig_decl);
>>
>> Is the if()if()if() cascade really needed? I can see a reason that an
>> extra 'if' is preferred for the variable declaration of orig_decl, but
>> can't we at least put the new 'orig_decl != expr' with an '&&' into the
>> same if as the decl/in the second if? Besides clearer, it also avoids
>> further identing the return line.
> 
> I think we can't in C++11/C++14.  The options can be if orig_decl would be declared
> earlier, then it can be
>      tree orig_decl;
>      if (DECL_P (expr)
> 	&& DECL_LANG_SPECIFIC (expr)
> 	&& (orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr))
> 	&& orig_decl != expr)
>        return non_negative_strides_array_p (orig_decl);
> but I think this is generally frowned upon,
> or one can repeat it like:
>      if (DECL_P (expr)
> 	&& DECL_LANG_SPECIFIC (expr)
> 	&& GFC_DECL_SAVED_DESCRIPTOR (expr)
> 	&& GFC_DECL_SAVED_DESCRIPTOR (expr) != expr)
>        return non_negative_strides_array_p (GFC_DECL_SAVED_DESCRIPTOR (expr));

I think I’ll use that.  There are numerous places where macros are 
repeated like this already and everybody seems to be pleased with it.
Thanks for the feedback, and for the suggestions.

> or what Mikael wrote, perhaps with the && on one line:
>      if (DECL_P (expr) && DECL_LANG_SPECIFIC (expr))
>        if (tree orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr))
> 	if (orig_decl != expr)
> 	  return non_negative_strides_array_p (orig_decl);
> In C++17 and later one can write:
>      if (DECL_P (expr) && DECL_LANG_SPECIFIC (expr))
>        if (tree orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr);
> 	  orig_decl && orig_decl != expr)
> 	return non_negative_strides_array_p (orig_decl);
> 
> 	Jakub
>