Re: [PATCH v2] Fortran: Narrow return types [PR78798]
Le 10/05/2023 à 18:47, Bernhard Reutner-Fischer via Fortran a écrit : From: Bernhard Reutner-Fischer 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.
Re: [PATCH v2] Fortran: Narrow return types [PR78798]
On 14.05.23 14:27, Mikael Morin wrote: (...) @@ -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. Hm, this makes me a litte suspicious. Was functionality for reversing loops lost, maybe unintentionally? I assume that, at some time, we did use the '2' as return value (or did we?) Best regards Thomas
Re: [PATCH v2] Fortran: Narrow return types [PR78798]
On Sun, 14 May 2023 15:04:15 +0200 Thomas Koenig wrote: > On 14.05.23 14:27, Mikael Morin wrote: > > > > (...) > >> @@ -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. > > Hm, this makes me a litte suspicious. Was functionality for reversing > loops lost, maybe unintentionally? I assume that, at some time, we did > use the '2' as return value (or did we?) There was 7c428aa29d75ef163c334cf3974f87b3630d8b8b (a revert because it miscompiled spec2k) which might have associated the comment of the former static gfc_dependency dep_ref (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse) to the current gfc_dep_resolver. The commit which introduced the return value 2 documentation was 3d03ead0b8273efde57f6194617b35111a84b05d "re PR fortran/24524 (Fortran dependency checking should reverse loops)" but TBH i don't see how it returned 2 in that revision? Looks like when writing that patch it deemed useful to return 2 for this specific situation but in the end it was dropped but the comment survived. I will update the comment to document the true / false return values. And Mikael, do you want me to cleanup 1/0 to true/false assignments for the boolean variables, or can we do that in a separate patch (or not at all right now)? many thanks for the reviews!
Re: [PATCH v2] Fortran: Narrow return types [PR78798]
Le 14/05/2023 à 17:24, Bernhard Reutner-Fischer a écrit : On Sun, 14 May 2023 15:04:15 +0200 Thomas Koenig wrote: On 14.05.23 14:27, Mikael Morin wrote: (...) @@ -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. Hm, this makes me a litte suspicious. Was functionality for reversing loops lost, maybe unintentionally? I assume that, at some time, we did use the '2' as return value (or did we?) There was 7c428aa29d75ef163c334cf3974f87b3630d8b8b (a revert because it miscompiled spec2k) which might have associated the comment of the former static gfc_dependency dep_ref (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse) to the current gfc_dep_resolver. The commit which introduced the return value 2 documentation was 3d03ead0b8273efde57f6194617b35111a84b05d "re PR fortran/24524 (Fortran dependency checking should reverse loops)" but TBH i don't see how it returned 2 in that revision? Looks like when writing that patch it deemed useful to return 2 for this specific situation but in the end it was dropped but the comment survived. Yes, I came to the same conclusion that we never returned 2 here. The information that reversal is needed is already provided on a per dimension basis by the gfc_reverse pointer passed as argument, so providing the information in the return value would be redundant anyway. I will update the comment to document the true / false return values. And Mikael, do you want me to cleanup 1/0 to true/false assignments for the boolean variables, or can we do that in a separate patch (or not at all right now)? I don't mind too much either way. As long as the variables are not assigned integer values outside of the [0,1] range, and we consistently use true/false or 0/1, not a mix of them, it's fine with me.
[PATCH] Fortran: CLASS pointer function result in variable definition context [PR109846]
Dear all, Fortran allows functions in variable definition contexts when the result variable is a pointer. We already handle this for the non-CLASS case (in 11+), but the logic that checks the pointer attribute was looking in the wrong place for the CLASS case. Once found, the fix is simple and obvious, see attached patch. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 6406f19855a3b664597d75369f0935d3d31384dc Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 14 May 2023 21:53:51 +0200 Subject: [PATCH] Fortran: CLASS pointer function result in variable definition context [PR109846] gcc/fortran/ChangeLog: PR fortran/109846 * expr.cc (gfc_check_vardef_context): Check appropriate pointer attribute for CLASS vs. non-CLASS function result in variable definition context. gcc/testsuite/ChangeLog: PR fortran/109846 * gfortran.dg/ptr-func-5.f90: New test. --- gcc/fortran/expr.cc | 2 +- gcc/testsuite/gfortran.dg/ptr-func-5.f90 | 39 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/ptr-func-5.f90 diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index d91722e6ac6..09a16c9b367 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -6256,7 +6256,7 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, bool alloc_obj, && !(sym->attr.flavor == FL_PROCEDURE && sym == sym->result) && !(sym->attr.flavor == FL_PROCEDURE && sym->attr.proc_pointer) && !(sym->attr.flavor == FL_PROCEDURE - && sym->attr.function && sym->attr.pointer)) + && sym->attr.function && attr.pointer)) { if (context) gfc_error ("%qs in variable definition context (%s) at %L is not" diff --git a/gcc/testsuite/gfortran.dg/ptr-func-5.f90 b/gcc/testsuite/gfortran.dg/ptr-func-5.f90 new file mode 100644 index 000..05fd56703ca --- /dev/null +++ b/gcc/testsuite/gfortran.dg/ptr-func-5.f90 @@ -0,0 +1,39 @@ +! { dg-do compile } +! PR fortran/109846 +! CLASS pointer function result in variable definition context + +module foo + implicit none + type :: parameter_list + contains +procedure :: sublist, sublist_nores + end type +contains + function sublist (this) result (slist) +class(parameter_list), intent(inout) :: this +class(parameter_list), pointer :: slist +allocate (slist) + end function + function sublist_nores (this) +class(parameter_list), intent(inout) :: this +class(parameter_list), pointer :: sublist_nores +allocate (sublist_nores) + end function +end module + +program example + use foo + implicit none + type(parameter_list) :: plist + call sub1 (plist%sublist()) + call sub1 (plist%sublist_nores()) + call sub2 (plist%sublist()) + call sub2 (plist%sublist_nores()) +contains + subroutine sub1 (plist) +type(parameter_list), intent(inout) :: plist + end subroutine + subroutine sub2 (plist) +type(parameter_list) :: plist + end subroutine +end program -- 2.35.3
Re: [PATCH] Fortran: CLASS pointer function result in variable definition context [PR109846]
On Sun, May 14, 2023 at 10:04:25PM +0200, Harald Anlauf via Fortran wrote: > > Fortran allows functions in variable definition contexts when the > result variable is a pointer. We already handle this for the > non-CLASS case (in 11+), but the logic that checks the pointer > attribute was looking in the wrong place for the CLASS case. > > Once found, the fix is simple and obvious, see attached patch. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > Yes. As you say, it is obvious once found. Ok to backport after a few days. -- Steve