Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]

2024-03-15 Thread Mikael Morin

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

Hi Mikael,

On 3/15/24 17:31, Mikael Morin wrote:

Le 10/03/2024 à 22:31, Harald Anlauf a écrit :

Dear all,

after playing for some time with NAG and Intel, and an off-list
discussion with Jerry, I am getting more and more convinced that
simpler runtime error messages (also simpler to parse by a human)
are superior to awkward solutions.  This is also what Intel does:
use only the name of the array (component) in the message whose
indices are out of bounds.

(NAG's solution appears also inconsistent for nested derived types.)

So no x%z, or x%_data, etc. in runtime error messages any more.


That's a pity.  What about providing the root variable and the failing
component only?

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

The data reference doesn't look great, but it provides valuable (in my
opinion) information.


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.


Cheers,
Harald


Please give it a spin...

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


On 1/30/24 11:46, Mikael Morin wrote:

Le 30/01/2024 à 11:38, Mikael Morin a écrit :


Another (easier) way to clarify the data reference would be rephrasing
the message so that the array part is separate from the scalar part,
like so (there are too many 'of', but I lack inspiration):
Index '0' of dimension 1 of component 'zz' of element from 'x1%vv'
below lower bound of 1


This has the same number of 'of' but sounds better maybe:
Out of bounds accessing component 'zz' of element from 'x1%yy': index
'0' of dimension 1 below lower bound of 1










Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]

2024-03-15 Thread Harald Anlauf

Hi Mikael,

On 3/15/24 17:31, Mikael Morin wrote:

Le 10/03/2024 à 22:31, Harald Anlauf a écrit :

Dear all,

after playing for some time with NAG and Intel, and an off-list
discussion with Jerry, I am getting more and more convinced that
simpler runtime error messages (also simpler to parse by a human)
are superior to awkward solutions.  This is also what Intel does:
use only the name of the array (component) in the message whose
indices are out of bounds.

(NAG's solution appears also inconsistent for nested derived types.)

So no x%z, or x%_data, etc. in runtime error messages any more.

That's a pity.  What about providing the root variable and the failing 
component only?


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

The data reference doesn't look great, but it provides valuable (in my 
opinion) information.


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)

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

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?

Anything else?

Cheers,
Harald


Please give it a spin...

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


On 1/30/24 11:46, Mikael Morin wrote:

Le 30/01/2024 à 11:38, Mikael Morin a écrit :


Another (easier) way to clarify the data reference would be rephrasing
the message so that the array part is separate from the scalar part,
like so (there are too many 'of', but I lack inspiration):
Index '0' of dimension 1 of component 'zz' of element from 'x1%vv'
below lower bound of 1


This has the same number of 'of' but sounds better maybe:
Out of bounds accessing component 'zz' of element from 'x1%yy': index
'0' of dimension 1 below lower bound of 1









Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]

2024-03-15 Thread Mikael Morin

Le 10/03/2024 à 22:31, Harald Anlauf a écrit :

Dear all,

after playing for some time with NAG and Intel, and an off-list
discussion with Jerry, I am getting more and more convinced that
simpler runtime error messages (also simpler to parse by a human)
are superior to awkward solutions.  This is also what Intel does:
use only the name of the array (component) in the message whose
indices are out of bounds.

(NAG's solution appears also inconsistent for nested derived types.)

So no x%z, or x%_data, etc. in runtime error messages any more.

That's a pity.  What about providing the root variable and the failing 
component only?


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

The data reference doesn't look great, but it provides valuable (in my 
opinion) information.



Please give it a spin...

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


On 1/30/24 11:46, Mikael Morin wrote:

Le 30/01/2024 à 11:38, Mikael Morin a écrit :


Another (easier) way to clarify the data reference would be rephrasing
the message so that the array part is separate from the scalar part,
like so (there are too many 'of', but I lack inspiration):
Index '0' of dimension 1 of component 'zz' of element from 'x1%vv'
below lower bound of 1


This has the same number of 'of' but sounds better maybe:
Out of bounds accessing component 'zz' of element from 'x1%yy': index
'0' of dimension 1 below lower bound of 1





[PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]

2024-03-10 Thread Harald Anlauf

Dear all,

after playing for some time with NAG and Intel, and an off-list
discussion with Jerry, I am getting more and more convinced that
simpler runtime error messages (also simpler to parse by a human)
are superior to awkward solutions.  This is also what Intel does:
use only the name of the array (component) in the message whose
indices are out of bounds.

(NAG's solution appears also inconsistent for nested derived types.)

So no x%z, or x%_data, etc. in runtime error messages any more.

Please give it a spin...

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


On 1/30/24 11:46, Mikael Morin wrote:

Le 30/01/2024 à 11:38, Mikael Morin a écrit :


Another (easier) way to clarify the data reference would be rephrasing 
the message so that the array part is separate from the scalar part, 
like so (there are too many 'of', but I lack inspiration):

Index '0' of dimension 1 of component 'zz' of element from 'x1%vv'
below lower bound of 1


This has the same number of 'of' but sounds better maybe:
Out of bounds accessing component 'zz' of element from 'x1%yy': index 
'0' of dimension 1 below lower bound of 1


From cdf3b197beed0ce1649661b2132643b54cbade8d Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Sun, 10 Mar 2024 22:14:30 +0100
Subject: [PATCH] Fortran: use name of array component in runtime error message
 [PR30802]

gcc/fortran/ChangeLog:

	PR fortran/30802
	* trans-array.cc (trans_array_bound_check): Find name of component
	to use in runtime error message.
	(array_bound_check_elemental): Likewise.
	(gfc_conv_array_ref): Likewise.

gcc/testsuite/ChangeLog:

	PR fortran/30802
	* gfortran.dg/bounds_check_17.f90: Adjust dg-pattern.
	* gfortran.dg/bounds_check_fail_6.f90: Likewise.
	* gfortran.dg/pr92050.f90: Likewise.
	* gfortran.dg/bounds_check_fail_8.f90: New test.
---
 gcc/fortran/trans-array.cc| 60 +--
 gcc/testsuite/gfortran.dg/bounds_check_17.f90 |  2 +-
 .../gfortran.dg/bounds_check_fail_6.f90   |  7 ++-
 .../gfortran.dg/bounds_check_fail_8.f90   | 48 +++
 gcc/testsuite/gfortran.dg/pr92050.f90 |  2 +-
 5 files changed, 83 insertions(+), 36 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 3673fa40720..9c62b070c50 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -3497,6 +3497,8 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
   tree descriptor;
   char *msg;
   const char * name = NULL;
+  gfc_expr *expr;
+  gfc_ref *ref;
 
   if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS))
 return index;
@@ -3509,6 +3511,24 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
   name = ss->info->expr->symtree->n.sym->name;
   gcc_assert (name != NULL);
 
+  /* When we have a component ref, get name of the array section.
+ Note that there can only be one part ref.  */
+  expr = ss->info->expr;
+  if (expr->ref && !compname)
+{
+  for (ref = expr->ref; ref; ref = ref->next)
+	{
+	  /* Remember component name.  */
+	  if (ref->type == REF_COMPONENT)
+	{
+	  name = ref->u.c.component->name;
+	  continue;
+	}
+	  if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION)
+	break;
+	}
+}
+
   if (VAR_P (descriptor))
 name = IDENTIFIER_POINTER (DECL_NAME (descriptor));
 
@@ -3574,29 +3594,20 @@ array_bound_check_elemental (gfc_se * se, gfc_ss * ss, gfc_expr * expr)
   gfc_array_ref *ar;
   gfc_ref *ref;
   gfc_symbol *sym;
-  char *var_name = NULL;
-  size_t len;
+  const char *var_name = NULL;
   int dim;
 
   if (expr->expr_type == EXPR_VARIABLE)
 {
   sym = expr->symtree->n.sym;
-  len = strlen (sym->name) + 1;
-
-  for (ref = expr->ref; ref; ref = ref->next)
-	if (ref->type == REF_COMPONENT)
-	  len += 2 + strlen (ref->u.c.component->name);
-
-  var_name = XALLOCAVEC (char, len);
-  strcpy (var_name, sym->name);
+  var_name = sym->name;
 
   for (ref = expr->ref; ref; ref = ref->next)
 	{
-	  /* Append component name.  */
+	  /* Get component name.  */
 	  if (ref->type == REF_COMPONENT)
 	{
-	  strcat (var_name, "%%");
-	  strcat (var_name, ref->u.c.component->name);
+	  var_name = ref->u.c.component->name;
 	  continue;
 	}
 
@@ -4001,7 +4012,7 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr,
   gfc_se indexse;
   gfc_se tmpse;
   gfc_symbol * sym = expr->symtree->n.sym;
-  char *var_name = NULL;
+  const char *var_name = NULL;
 
   if (ar->dimen == 0)
 {
@@ -4035,30 +4046,17 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr,
 
   if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)
 {
-  size_t len;
   gfc_ref *ref;
 
-  len = strlen (sym->name) + 1;
-  for (ref = expr->ref; ref; ref = ref->next)
-	{
-	  if (ref->type == REF_ARRAY && &ref->u.ar == ar)
-	break;
-	  if (ref->type == REF_C