Re: [PATCH, v3] Fortran: improve array component description in runtime error message [PR30802]

2024-03-21 Thread Mikael Morin

Le 20/03/2024 à 21:24, Harald Anlauf a écrit :

Hi Mikael, all,

here's now the third version of the patch that implements the following
scheme:

On 3/15/24 20:29, Mikael Morin wrote:

Le 15/03/2024 à 18:26, Harald Anlauf a écrit :

OK, that sounds interesting.  To clarify the options:

- for ordinary array x it would stay 'x'

- when z is a DT scalar, and z%x is the array in question, use 'z%x'
   (here z...%x would look strange to me)


Yes, the ellipsis would look strange to me as well.


- when z is a DT array, and x some component further down, 'z...%x'


This case also applies when z is a DT scalar and x is more than one
level deep.


I would rather not make the error message text vary too much to avoid
to run into issues with translation.  Would it be fine with you to have

... dimension 1 of array 'z...%x' above array bound ...

only?


OK, let's drop "component".


Anything else?


No, I think you covered everything.


I've created a new helper function that centralizes the generation of
the abbreviated name of the array (component) and use it to simplify
related code in multiple places.  If we change our mind how a bounds
violation error message should look like, it will be easier to adjust
in the future.

Is this OK for 14-mainline?


Yes, thanks.


Thanks,
Harald






[PATCH, v3] Fortran: improve array component description in runtime error message [PR30802]

2024-03-20 Thread Harald Anlauf

Hi Mikael, all,

here's now the third version of the patch that implements the following
scheme:

On 3/15/24 20:29, Mikael Morin wrote:

Le 15/03/2024 à 18:26, Harald Anlauf a écrit :

OK, that sounds interesting.  To clarify the options:

- for ordinary array x it would stay 'x'

- when z is a DT scalar, and z%x is the array in question, use 'z%x'
   (here z...%x would look strange to me)


Yes, the ellipsis would look strange to me as well.


- when z is a DT array, and x some component further down, 'z...%x'


This case also applies when z is a DT scalar and x is more than one
level deep.


I would rather not make the error message text vary too much to avoid
to run into issues with translation.  Would it be fine with you to have

... dimension 1 of array 'z...%x' above array bound ...

only?


OK, let's drop "component".


Anything else?


No, I think you covered everything.


I've created a new helper function that centralizes the generation of
the abbreviated name of the array (component) and use it to simplify
related code in multiple places.  If we change our mind how a bounds
violation error message should look like, it will be easier to adjust
in the future.

Is this OK for 14-mainline?

Thanks,
Harald


From 30d7cef086d440262b206bc39bcbcac89491b792 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Wed, 20 Mar 2024 20:59:24 +0100
Subject: [PATCH] Fortran: improve array component description in runtime error
 message [PR30802]

Runtime error messages for array bounds violation shall use the following
scheme for a coherent, abridged description of arrays or array components
of derived types:
(1) If x is an ordinary array variable, use "x"
(2) if z is a DT scalar and x an array component at level 1, use "z%x"
(3) if z is a DT scalar and x an array component at level > 1, or
if z is a DT array and x an array (at any level), use "z...%x"
Use a new helper function abridged_ref_name for construction of that name.

gcc/fortran/ChangeLog:

	PR fortran/30802
	* trans-array.cc (abridged_ref_name): New helper function.
	(trans_array_bound_check): Use it.
	(array_bound_check_elemental): Likewise.
	(gfc_conv_array_ref): Likewise.

gcc/testsuite/ChangeLog:

	PR fortran/30802
	* gfortran.dg/bounds_check_17.f90: Adjust pattern.
	* gfortran.dg/bounds_check_fail_8.f90: New test.
---
 gcc/fortran/trans-array.cc| 132 +++---
 gcc/testsuite/gfortran.dg/bounds_check_17.f90 |   2 +-
 .../gfortran.dg/bounds_check_fail_8.f90   |  56 
 3 files changed, 142 insertions(+), 48 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 0a453828bad..30b84762346 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -3485,6 +3485,78 @@ gfc_conv_array_ubound (tree descriptor, int dim)
 }
 
 
+/* Generate abridged name of a part-ref for use in bounds-check message.
+   Cases:
+   (1) for an ordinary array variable x return "x"
+   (2) for z a DT scalar and array component x (at level 1) return "z%%x"
+   (3) for z a DT scalar and array component x (at level > 1) or
+   for z a DT array and array x (at any number of levels): "z...%%x"
+ */
+
+static char *
+abridged_ref_name (gfc_expr * expr, gfc_array_ref * ar)
+{
+  gfc_ref *ref;
+  gfc_symbol *sym;
+  char *ref_name = NULL;
+  const char *comp_name = NULL;
+  int len_sym, last_len = 0, level = 0;
+  bool sym_is_array;
+
+  gcc_assert (expr->expr_type == EXPR_VARIABLE && expr->ref != NULL);
+
+  sym = expr->symtree->n.sym;
+  sym_is_array = (sym->ts.type != BT_CLASS
+		  ? sym->as != NULL
+		  : IS_CLASS_ARRAY (sym));
+  len_sym = strlen (sym->name);
+
+  /* Scan ref chain to get name of the array component (when ar != NULL) or
+ array section, determine depth and remember its component name.  */
+  for (ref = expr->ref; ref; ref = ref->next)
+{
+  if (ref->type == REF_COMPONENT
+	  && strcmp (ref->u.c.component->name, "_data") != 0)
+	{
+	  level++;
+	  comp_name = ref->u.c.component->name;
+	  continue;
+	}
+
+  if (ref->type != REF_ARRAY)
+	continue;
+
+  if (ar)
+	{
+	  if (>u.ar == ar)
+	break;
+	}
+  else if (ref->u.ar.type == AR_SECTION)
+	break;
+}
+
+  if (level > 0)
+last_len = strlen (comp_name);
+
+  /* Provide a buffer sufficiently large to hold "x...%%z".  */
+  ref_name = XNEWVEC (char, len_sym + last_len + 6);
+  strcpy (ref_name, sym->name);
+
+  if (level == 1 && !sym_is_array)
+{
+  strcat (ref_name, "%%");
+  strcat (ref_name, comp_name);
+}
+  else if (level > 0)
+{
+  strcat (ref_name, "...%%");
+  strcat (ref_name, comp_name);
+}
+
+  return ref_name;
+}
+
+
 /* Generate code to perform an array index bound check.  */
 
 static tree
@@ -3496,7 +3568,9 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
   tree tmp_lo, tmp_up;
   tree descriptor;
   char *msg;
+  char *ref_name = NULL;
   const char