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));
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
