Re: [PATCH v2] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>]

On Sun, 14 May 2023 14:27:42 +0200
Mikael Morin <morin-mikael@orange.fr> wrote:

> Le 10/05/2023 à 18:47, Bernhard Reutner-Fischer via Fortran a écrit :
> > From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org>
> > 
> > gcc/fortran/ChangeLog:
> > 
> > 	PR fortran/78798
> > 	* array.cc (compare_bounds): Use narrower return type.
> > 	(gfc_compare_array_spec): Likewise.
> > 	(is_constant_element): Likewise.
> > 	(gfc_constant_ac): Likewise.  
> (...)
> > ---
> > Bootstrapped without new warnings and regression tested on
> > x86_64-linux with no regressions, OK for trunk?
> >   
> (...)
> > diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
> > index b348bda6e6c..4e3aed84b9d 100644
> > --- a/gcc/fortran/check.cc
> > +++ b/gcc/fortran/check.cc
> > @@ -1156,7 +1156,7 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int allow_assumed)
> >      dimension bi, returning 0 if they are known not to be identical,
> >      and 1 if they are identical, or if this cannot be determined.  */
> >   
> > -static int
> > +static bool
> >   identical_dimen_shape (gfc_expr *a, int ai, gfc_expr *b, int bi)
> >   {
> >     mpz_t a_size, b_size;  
> 
> To be consistent, please change as well the local variable "ret" used as 
> return value from int to bool.
> 
> > diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
> > index c3b7c7f7bd9..d7890a97287 100644
> > --- a/gcc/fortran/cpp.cc
> > +++ b/gcc/fortran/cpp.cc
> > @@ -297,7 +297,7 @@ gfc_cpp_init_options (unsigned int decoded_options_count,
> >     gfc_cpp_option.deferred_opt_count = 0;
> >   }
> >   
> > -int
> > +bool
> >   gfc_cpp_handle_option (size_t scode, const char *arg, int value ATTRIBUTE_UNUSED)
> >   {
> >     int result = 1;  
> 
> Same here, change the type of variable "result".
> 
> (...)
> > diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
> > index a648d5c7903..b398b29a642 100644
> > --- a/gcc/fortran/dependency.cc
> > +++ b/gcc/fortran/dependency.cc  
> (...)
> 
> > @@ -1091,7 +1091,7 @@ gfc_check_argument_dependency (gfc_expr *other, sym_intent intent,
> >   /* Like gfc_check_argument_dependency, but check all the arguments in ACTUAL.
> >      FNSYM is the function being called, or NULL if not known.  */
> >   
> > -int
> > +bool
> >   gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
> >   			     gfc_symbol *fnsym, gfc_actual_arglist *actual,
> >   			     gfc_dep_check elemental)  
> 
> Why not change the associated subfunctions 
> (gfc_check_argument_dependency, gfc_check_argument_var_dependency) as well ?

I have left these subfunctions alone for now to get the other hunks out
of the way. I have adjusted the patch according to your other comments
and pushed it as r14-973-gc072df1ab14450.

Thanks!

> 
> (...)
> > @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref *ref)
> >   	    there is some kind of overlap.
> >   	0 : array references are identical or not overlapping.  */
> >   
> > -int
> > +bool
> >   gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
> >   		  bool identical)
> >   {  
> 
> The function comment states that the function may return 2, which 
> doesn't seem to be the case any more.  So please update the comment.
> 
> (...)> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> > index 221165d6dac..b4b36e27d75 100644
> > --- a/gcc/fortran/symbol.cc
> > +++ b/gcc/fortran/symbol.cc
> > @@ -3216,7 +3216,7 @@ gfc_find_symtree_in_proc (const char* name, gfc_namespace* ns)
> >      any parent namespaces if requested by a nonzero parent_flag.
> >      Returns nonzero if the name is ambiguous.  */
> >   
> > -int
> > +bool
> >   gfc_find_sym_tree (const char *name, gfc_namespace *ns, int parent_flag,
> >   		   gfc_symtree **result)
> >   {  
> 
> Maybe change nonzero to true in the comment?
> 
> (...)
> 
> OK with all the above fixed.
> 
> Thanks.
>