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
