Re: [patch, Fortran] Implement maxval/minval for UNSIGNED

2024-10-01 Thread Thomas Koenig

Hi Andre,


+ if (gfc_option.allow_std & GFC_STD_F2003)
+   gfc_error ("%qs argument of %qs intrinsic at %L must be INTEGER "
+  "or REAL or CHARACTER",

I had expected UNSIGNED in this enumeration, too.

+  gfc_current_intrinsic_arg[n]->name,
+  gfc_current_intrinsic, &e->where);
+ else
+   gfc_error ("%qs argument of %qs intrinsic at %L must be INTEGER "
+  "or REAL", gfc_current_intrinsic_arg[n]->name,

Same.


You're right.

Corrected accordingly and pushed as
r15-3996-g9dd9a06940a37e82d13ccd2be0c4ef68bca29750.

Thanks for the review!

Best regards

Thomas



[Fortran, Patch, PR51815, v1] Fix parsing of substring refs in coarrays.

2024-10-01 Thread Andre Vehreschild
Hi all,

this rather old PR reported a parsing bug, when a coarray'ed character substring
ref is to be parsed, aka CHARACTER(:) :: str[:] ... str(2:5). In this case the
parser confused the substring ref with an array-ref, because an array_spec was
present. This patch fixes this by requesting only coarray parsing from
gfc_match_array_ref when no regular dimension is present. The patch is not
involved when an array of coarray'ed strings is parsed (that worked
beforehand).

I had to fix the dg-error clauses in the testcase pr102532 because now the error
of having to many refs is detected by the parsing stage and no longer by the
resolve stage. It has become a simple syntax error. I hope this is ok.

Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?

Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
From 1d5e0abd0e6df0ec05c3dfb4bf7cee433b885994 Mon Sep 17 00:00:00 2001
From: Andre Vehreschild 
Date: Tue, 1 Oct 2024 09:30:59 +0200
Subject: [PATCH] [Fortran] Fix parsing of substring refs in coarrays.
 [PR51815]

The parser was greadily taking the substring ref as an array ref because
an array_spec was present.  Fix this by only parsing the coarray (pseudo)
ref when no regular array is present.

gcc/fortran/ChangeLog:

	PR fortran/51815

	* array.cc (gfc_match_array_ref): Only parse coarray part of
	ref.
	* match.h (gfc_match_array_ref): Add flag.
	* primary.cc (gfc_match_varspec): Request only coarray ref
	parsing when no regular array is present.

gcc/testsuite/ChangeLog:

	* gfortran.dg/pr102532.f90: Fix dg-errors: Now a syntax error.
	* gfortran.dg/coarray/substring_1.f90: New test.
---
 gcc/fortran/array.cc  |  9 --
 gcc/fortran/match.h   |  3 +-
 gcc/fortran/primary.cc| 30 ---
 .../gfortran.dg/coarray/substring_1.f90   | 16 ++
 gcc/testsuite/gfortran.dg/pr102532.f90| 13 
 5 files changed, 51 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/coarray/substring_1.f90

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index 1fa61ebfe2a..ed8cb54803b 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -179,7 +179,7 @@ matched:

 match
 gfc_match_array_ref (gfc_array_ref *ar, gfc_array_spec *as, int init,
-		 int corank)
+		 int corank, bool coarray_only)
 {
   match m;
   bool matched_bracket = false;
@@ -198,6 +198,8 @@ gfc_match_array_ref (gfc_array_ref *ar, gfc_array_spec *as, int init,
matched_bracket = true;
goto coarray;
 }
+  else if (coarray_only && corank != 0)
+goto coarray;

   if (gfc_match_char ('(') != MATCH_YES)
 {
@@ -243,11 +245,12 @@ gfc_match_array_ref (gfc_array_ref *ar, gfc_array_spec *as, int init,
 coarray:
   if (!matched_bracket && gfc_match_char ('[') != MATCH_YES)
 {
-  if (ar->dimen > 0)
+  int dim = coarray_only ? 0 : ar->dimen;
+  if (dim > 0 || coarray_only)
 	{
 	  if (corank != 0)
 	{
-	  for (int i = ar->dimen; i < GFC_MAX_DIMENSIONS; ++i)
+	  for (int i = dim; i < GFC_MAX_DIMENSIONS; ++i)
 		ar->dimen_type[i] = DIMEN_THIS_IMAGE;
 	  ar->codimen = corank;
 	}
diff --git a/gcc/fortran/match.h b/gcc/fortran/match.h
index 84d84b81825..2c76afb179a 100644
--- a/gcc/fortran/match.h
+++ b/gcc/fortran/match.h
@@ -317,7 +317,8 @@ match gfc_match_init_expr (gfc_expr **);

 /* array.cc.  */
 match gfc_match_array_spec (gfc_array_spec **, bool, bool);
-match gfc_match_array_ref (gfc_array_ref *, gfc_array_spec *, int, int);
+match gfc_match_array_ref (gfc_array_ref *, gfc_array_spec *, int, int,
+			   bool = false);
 match gfc_match_array_constructor (gfc_expr **);

 /* interface.cc.  */
diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
index 09add925fcd..d73d5eaed84 100644
--- a/gcc/fortran/primary.cc
+++ b/gcc/fortran/primary.cc
@@ -2192,7 +2192,7 @@ gfc_match_varspec (gfc_expr *primary, int equiv_flag, bool sub_flag,
   bool intrinsic;
   bool inferred_type;
   locus old_loc;
-  char sep;
+  char peeked_char;

   tail = NULL;

@@ -2282,9 +2282,10 @@ gfc_match_varspec (gfc_expr *primary, int equiv_flag, bool sub_flag,
 	sym->ts.u.derived = tgt_expr->ts.u.derived;
 }

-  if ((inferred_type && !sym->as && gfc_peek_ascii_char () == '(')
-  || (equiv_flag && gfc_peek_ascii_char () == '(')
-  || gfc_peek_ascii_char () == '[' || sym->attr.codimension
+  peeked_char = gfc_peek_ascii_char ();
+  if ((inferred_type && !sym->as && peeked_char == '(')
+  || (equiv_flag && peeked_char == '(') || peeked_char == '['
+  || sym->attr.codimension
   || (sym->attr.dimension && sym->ts.type != BT_CLASS
 	  && !sym->attr.proc_pointer && !gfc_is_proc_ptr_comp (primary)
 	  && !(gfc_matching_procptr_assignment
@@ -2295,6 +2296,8 @@ gfc_match_varspec (gfc_expr *primary, int equiv_flag, bool sub_flag,
 	  || CLASS_DATA (sym)->attr.codimension)))
 {
   gfc_array_spec *as;
+  bool coarray_only

Re: [PATCH] fortran: Fix default initialization of finalizable non-polymorphic intent(out) arguments [PR116829]

2024-10-01 Thread Andre Vehreschild
Hi Tomáš,

one small nit: I would prefer to use i = -1 as the initializer for component of
the derived type, because 0 is a default generated one and that way one can
distinguish them. With that, ok for mainline and thanks for the patch.

- Andre

On Wed, 25 Sep 2024 17:20:12 +0200
Tomáš Trnka  wrote:

> This fixes PR fortran/116829 by making sure that s->value is always
> applied to non-allocatable BT_DERIVED intent(out) arguments, no matter
> if they are finalizable or not.
> 
> Tested on x86_64-pc-linux-gnu (Fedora 40), any feedback is welcome.
> 
> gcc/fortran/ChangeLog:
> 
> * trans-decl.cc (init_intent_out_dt): Always call
> gfc_init_default_dt() for BT_DERIVED to apply s->value
> if the symbol isn't allocatable. Also simplify the logic a bit.
> 
> gcc/testsuite/ChangeLog:
> 
> * gfortran.dg/derived_init_7.f90: New test.
> 
> Signed-off-by: Tomáš Trnka 
> ---
>  gcc/fortran/trans-decl.cc|  9 +---
>  gcc/testsuite/gfortran.dg/derived_init_7.f90 | 49 
>  2 files changed, 51 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/gfortran.dg/derived_init_7.f90
> 
> diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
> index 8231bd255d6..50e8adcb819 100644
> --- a/gcc/fortran/trans-decl.cc
> +++ b/gcc/fortran/trans-decl.cc
> @@ -4461,7 +4461,6 @@ init_intent_out_dt (gfc_symbol * proc_sym,
> gfc_wrapped_block * block) tree tmp;
>tree present;
>gfc_symbol *s;
> -  bool dealloc_with_value = false;
>  
>gfc_init_block (&init);
>for (f = gfc_sym_get_dummy_args (proc_sym); f; f = f->next)
> @@ -4496,7 +4495,6 @@ init_intent_out_dt (gfc_symbol * proc_sym,
> gfc_wrapped_block * block) tmp = gfc_deallocate_alloc_comp (s->ts.u.derived,
>s->backend_decl,
>s->as ? s->as->rank : 0);
> - dealloc_with_value = s->value;
> }
>  
>   if (tmp != NULL_TREE && (s->attr.optional
> @@ -4507,13 +4505,10 @@ init_intent_out_dt (gfc_symbol * proc_sym,
> gfc_wrapped_block * block) present, tmp, build_empty_stmt (input_location));
> }
>  
> - if (tmp != NULL_TREE && !dealloc_with_value)
> -   gfc_add_expr_to_block (&init, tmp);
> - else if (s->value && !s->attr.allocatable)
> + gfc_add_expr_to_block (&init, tmp);
> + if (s->value && !s->attr.allocatable)
> {
> - gfc_add_expr_to_block (&init, tmp);
>   gfc_init_default_dt (s, &init, false);
> - dealloc_with_value = false;
> }
>}
>  else if (f->sym && f->sym->attr.intent == INTENT_OUT
> diff --git a/gcc/testsuite/gfortran.dg/derived_init_7.f90
> b/gcc/testsuite/gfortran.dg/derived_init_7.f90 new file mode 100644
> index 000..8efb7ba1c77
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/derived_init_7.f90
> @@ -0,0 +1,49 @@
> +! { dg-do run }
> +! Check that finalizable intent(out) dummy arguments are first finalized
> +! and then correctly default-initialized (PR116829)
> +!
> +module FinalizableIntentOutTestModule
> +   implicit none
> +
> +   type :: AapType
> +  integer  :: i = 0
> +   contains
> +  final:: Finalizer
> +   end type
> +
> +contains
> +
> +   subroutine Finalizer(self)
> +  type(AapType), intent(inout) :: self
> +
> +  ! Fail if Finalizer gets called again on an already finalized object
> +  if (self%i == 42) stop 1
> +
> +  self%i = 42 ! Nobody should ever see this value after finalization
> +   end subroutine
> +
> +end module
> +
> +
> +program test
> +   use FinalizableIntentOutTestModule
> +
> +   implicit none
> +
> +   type(AapType) :: aap
> +
> +   ! Set "i" to nonzero so that initialization in MakeAap has something to do
> +   aap%i = 1
> +
> +   call MakeAap(aap)
> +
> +contains
> +
> +   subroutine MakeAap(a)
> +  type(AapType), intent(out) :: a
> +
> +  ! Fail if "a" wasn't initialized properly
> +  if (a%i /= 0) stop 2
> +   end subroutine
> +
> +end program


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 


Re: [patch, Fortran] Implement maxval/minval for UNSIGNED

2024-10-01 Thread Andre Vehreschild
Hi Thomas,

one small nit:

diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index dd79a49a0c9..afab66a901f 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -637,6 +637,38 @@ int_or_real_or_char_check_f2003 (gfc_expr *e, int n)
   return true;
 }

+/* Check that an expression is integer or real or unsigned; allow character for
+   F2003 or later.  */
+
+static bool
+int_or_real_or_char_or_unsigned_check_f2003 (gfc_expr *e, int n)
+{
+  if (e->ts.type != BT_INTEGER && e->ts.type != BT_REAL
+  && e->ts.type != BT_UNSIGNED)
+{
+  if (e->ts.type == BT_CHARACTER)
+   return gfc_notify_std (GFC_STD_F2003, "Fortran 2003: Character for "
+  "%qs argument of %qs intrinsic at %L",
+  gfc_current_intrinsic_arg[n]->name,
+  gfc_current_intrinsic, &e->where);
+  else
+   {
+ if (gfc_option.allow_std & GFC_STD_F2003)
+   gfc_error ("%qs argument of %qs intrinsic at %L must be INTEGER "
+  "or REAL or CHARACTER",

I had expected UNSIGNED in this enumeration, too.

+  gfc_current_intrinsic_arg[n]->name,
+  gfc_current_intrinsic, &e->where);
+ else
+   gfc_error ("%qs argument of %qs intrinsic at %L must be INTEGER "
+  "or REAL", gfc_current_intrinsic_arg[n]->name,

Same.

+  gfc_current_intrinsic, &e->where);
+   }
+  return false;
+}
+
+  return true;
+}
+
 /* Check that an expression is an intrinsic type.  */
 static bool
 intrinsic_type_check (gfc_expr *e, int n)


Ok for mainline with me.

Thanks for the patch,
Andre

On Sun, 29 Sep 2024 16:22:57 +0200
Thomas Koenig  wrote:

> Hello world,
>
> here's the implementation for MAXVAL and MINVAL for UNSIGNED.
>
> I found https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116886
> while working on the test case (something to be fixed later).
>
> (@Mikael: This actually touches trans-intrinsic.cc, but I don't
> think there will be collisions with what you're working on, it
> is only the part where the minimum/maximum is set).
>
> Regression-tested. OK for trunk?
>
> Best regards
>
>   Thomas
>
> gcc/fortran/ChangeLog:
>
>   * check.cc (int_or_real_or_char_or_unsigned_check_f2003): New
> function. (gfc_check_minval_maxval): Use it.
>   * trans-intrinsic.cc (gfc_conv_intrinsic_minmaxval): Handle
>   initial values for UNSIGNED.
>   * gfortran.texi: Document MINVAL and MAXVAL for unsigned.
>
> libgfortran/ChangeLog:
>
>   * Makefile.am: Add minval and maxval files.
>   * Makefile.in: Regenerated.
>   * gfortran.map: Add new functions.
>   * generated/maxval_m1.c: New file.
>   * generated/maxval_m16.c: New file.
>   * generated/maxval_m2.c: New file.
>   * generated/maxval_m4.c: New file.
>   * generated/maxval_m8.c: New file.
>   * generated/minval_m1.c: New file.
>   * generated/minval_m16.c: New file.
>   * generated/minval_m2.c: New file.
>   * generated/minval_m4.c: New file.
>   * generated/minval_m8.c: New file.
>
> gcc/testsuite/ChangeLog:
>
>   * gfortran.dg/unsigned_34.f90: New test.


--
Andre Vehreschild * Email: vehre ad gmx dot de


Re: [Fortran, Patch, PR51815, v1] Fix parsing of substring refs in coarrays.

2024-10-01 Thread Harald Anlauf

Hi Andre,

Am 01.10.24 um 09:43 schrieb Andre Vehreschild:

Hi all,

this rather old PR reported a parsing bug, when a coarray'ed character substring
ref is to be parsed, aka CHARACTER(:) :: str[:] ... str(2:5). In this case the
parser confused the substring ref with an array-ref, because an array_spec was
present. This patch fixes this by requesting only coarray parsing from
gfc_match_array_ref when no regular dimension is present. The patch is not
involved when an array of coarray'ed strings is parsed (that worked
beforehand).


while the patch addresses the issue mentioned in the PR,


I had to fix the dg-error clauses in the testcase pr102532 because now the error
of having to many refs is detected by the parsing stage and no longer by the
resolve stage. It has become a simple syntax error. I hope this is ok.


I find the error messages now less helpful to users: before the patch
we got "Rank mismatch in array reference", which was more suitable
than the newer version with more or less confusing syntax errors.

I assume you tried to find a better solution - but Intel and NAG
also give syntax errors - so basically I am fine with the patch.

You may want to wait for a second opinion.  If nobody else responds
within the next 2 days, you may proceed nevertheless.

Thanks,
Harald


Regtests ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline?

Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de