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

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

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 ?

(...)
> @@ -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.