public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fortran: use name of array component in runtime error message [PR30802]
@ 2024-01-24 21:39 Harald Anlauf
  2024-01-28 11:39 ` Mikael Morin
  0 siblings, 1 reply; 15+ messages in thread
From: Harald Anlauf @ 2024-01-24 21:39 UTC (permalink / raw)
  To: fortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

Dear all,

this patch is actually only a followup fix to generate the proper name
of an array reference in derived-type components for the runtime error
message generated for the bounds-checking code.  Without the proper
part ref, not only a user may get confused: I was, too...

The testcase is compile-only, as it is only important to check the
strings used in the error messages.

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

Thanks,
Harald


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr30802-part2.diff --]
[-- Type: text/x-patch, Size: 4316 bytes --]

From 43c0185764ec878576ef2255d9df24fbb1961af4 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Wed, 24 Jan 2024 22:28:31 +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): Derive name of component
	for use in runtime error message.

gcc/testsuite/ChangeLog:

	PR fortran/30802
	* gfortran.dg/bounds_check_fail_8.f90: New test.
---
 gcc/fortran/trans-array.cc                    | 34 ++++++++++++++++++
 .../gfortran.dg/bounds_check_fail_8.f90       | 35 +++++++++++++++++++
 2 files changed, 69 insertions(+)
 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 878a92aff18..f6ddce2d023 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -3497,6 +3497,10 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
   tree descriptor;
   char *msg;
   const char * name = NULL;
+  char *var_name = NULL;
+  gfc_expr *expr;
+  gfc_ref *ref;
+  size_t len;

   if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS))
     return index;
@@ -3509,6 +3513,36 @@ 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, compile name for the array section.
+     Note that there can only be one part ref.  */
+  expr = ss->info->expr;
+  if (expr->ref && !compname)
+    {
+      len = strlen (name) + 1;
+
+      /* Find a safe length.  */
+      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, name);
+
+      for (ref = expr->ref; ref; ref = ref->next)
+	{
+	  /* Append component name.  */
+	  if (ref->type == REF_COMPONENT)
+	    {
+	      strcat (var_name, "%%");
+	      strcat (var_name, ref->u.c.component->name);
+	      continue;
+	    }
+	  if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION)
+	    break;
+	}
+      name = var_name;
+    }
+
   if (VAR_P (descriptor))
     name = IDENTIFIER_POINTER (DECL_NAME (descriptor));

diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90
new file mode 100644
index 00000000000..3397e953ba6
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90
@@ -0,0 +1,35 @@
+! { dg-do compile }
+! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" }
+! { dg-output "At line 22 .*" }
+! { dg-shouldfail "dimension 3 of array 'uu%z' outside of expected range" }
+!
+! PR fortran/30802 - improve bounds-checking for array references
+!
+! Checking the proper component references is the most important part here.
+
+program test
+  implicit none
+  integer :: k = 0
+  type t
+     real, dimension(10,20,30) :: z = 23
+  end type t
+  type u
+     type(t) :: vv(4,5)
+  end type u
+  type(t) :: uu,     ww(1)
+  type(u) :: x1, x2, y1(1), y2(1)
+
+  print *, uu   % z(1,k,:)           ! runtime check only for dimension 2 of z
+  print *, ww(1)% z(1,:,k)           ! runtime check only for dimension 3 of z
+  print *, x1   % vv(2,3)% z(1,:,k)  ! runtime check only for dimension 3 of z
+  print *, x2   % vv(k,:)% z(1,2,3)  ! runtime check only for dimension 1 of vv
+  print *, y1(1)% vv(2,3)% z(1,:,k)  ! runtime check only for dimension 3 of z
+  print *, y2(1)% vv(:,k)% z(1,2,3)  ! runtime check only for dimension 2 of vv
+end program test
+
+! { dg-final { scan-tree-dump-times "'uu%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "'ww%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "'x1%%vv%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "'x2%%vv.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "'y1%%vv%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "'y2%%vv.' outside of expected range" 2 "original" } }
--
2.35.3


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-24 21:39 [PATCH] Fortran: use name of array component in runtime error message [PR30802] Harald Anlauf
@ 2024-01-28 11:39 ` Mikael Morin
  2024-01-28 19:56   ` Harald Anlauf
  0 siblings, 1 reply; 15+ messages in thread
From: Mikael Morin @ 2024-01-28 11:39 UTC (permalink / raw)
  To: Harald Anlauf, fortran, gcc-patches

Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
> Dear all,
> 
> this patch is actually only a followup fix to generate the proper name
> of an array reference in derived-type components for the runtime error
> message generated for the bounds-checking code.  Without the proper
> part ref, not only a user may get confused: I was, too...
> 
> The testcase is compile-only, as it is only important to check the
> strings used in the error messages.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> 
> Thanks,
> Harald
> 
Hello,

the change proper looks good, and is an improvement.  But I'm a little 
concerned by the production of references like in the test x1%vv%z which 
could be confusing and is strictly speaking invalid fortran (multiple 
non-scalar components).  Did you consider generating x1%vv(?,?)%zz or 
x1%vv(...)%z or similar?

There is another nit about the test, which has dg-output and 
dg-shouldfail despite being only a compile-time test.

Otherwise looks good.

Mikael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-28 11:39 ` Mikael Morin
@ 2024-01-28 19:56   ` Harald Anlauf
  2024-01-28 21:43     ` Steve Kargl
  0 siblings, 1 reply; 15+ messages in thread
From: Harald Anlauf @ 2024-01-28 19:56 UTC (permalink / raw)
  To: Mikael Morin, fortran, gcc-patches

Hi Mikael,

Am 28.01.24 um 12:39 schrieb Mikael Morin:
> Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
>> Dear all,
>>
>> this patch is actually only a followup fix to generate the proper name
>> of an array reference in derived-type components for the runtime error
>> message generated for the bounds-checking code.  Without the proper
>> part ref, not only a user may get confused: I was, too...
>>
>> The testcase is compile-only, as it is only important to check the
>> strings used in the error messages.
>>
>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>
>> Thanks,
>> Harald
>>
> Hello,
>
> the change proper looks good, and is an improvement.  But I'm a little
> concerned by the production of references like in the test x1%vv%z which
> could be confusing and is strictly speaking invalid fortran (multiple
> non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
> x1%vv(...)%z or similar?

yes, that seems very reasonable, given that this is what NAG does.

We also have spurious %_data in some error messages that I'll try
to get rid off.

> There is another nit about the test, which has dg-output and
> dg-shouldfail despite being only a compile-time test.

I'll remove that.  With gcc-13 the runtime check would trigger
in the wrong line but the failure of the check is dealt with
by another testcase in gcc-14.

> Otherwise looks good.
>
> Mikael

I'll come up with a revised patch.  Thanks for the feedback so far!

Harald



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-28 19:56   ` Harald Anlauf
@ 2024-01-28 21:43     ` Steve Kargl
  2024-01-29  6:51       ` rep.dot.nop
  2024-01-29 17:25       ` Harald Anlauf
  0 siblings, 2 replies; 15+ messages in thread
From: Steve Kargl @ 2024-01-28 21:43 UTC (permalink / raw)
  To: Harald Anlauf; +Cc: Mikael Morin, fortran, gcc-patches

On Sun, Jan 28, 2024 at 08:56:24PM +0100, Harald Anlauf wrote:
> 
> Am 28.01.24 um 12:39 schrieb Mikael Morin:
> > Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
> > > Dear all,
> > > 
> > > this patch is actually only a followup fix to generate the proper name
> > > of an array reference in derived-type components for the runtime error
> > > message generated for the bounds-checking code.  Without the proper
> > > part ref, not only a user may get confused: I was, too...
> > > 
> > > The testcase is compile-only, as it is only important to check the
> > > strings used in the error messages.
> > > 
> > > Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> > > 
> > 
> > the change proper looks good, and is an improvement.  But I'm a little
> > concerned by the production of references like in the test x1%vv%z which
> > could be confusing and is strictly speaking invalid fortran (multiple
> > non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
> > x1%vv(...)%z or similar?
> 
> yes, that seems very reasonable, given that this is what NAG does.
> 
> We also have spurious %_data in some error messages that I'll try
> to get rid off.
> 

I haven't looked at the patch, but sometimes (if not always) things
like _data are marked with attr.artificial.  You might see if this
will help with suppressing spurious messages.



-- 
Steve

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-28 21:43     ` Steve Kargl
@ 2024-01-29  6:51       ` rep.dot.nop
  2024-01-29 17:25       ` Harald Anlauf
  1 sibling, 0 replies; 15+ messages in thread
From: rep.dot.nop @ 2024-01-29  6:51 UTC (permalink / raw)
  To: sgk, Steve Kargl, Harald Anlauf; +Cc: Mikael Morin, fortran, gcc-patches

On 28 January 2024 22:43:37 CET, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote:
>On Sun, Jan 28, 2024 at 08:56:24PM +0100, Harald Anlauf wrote:
>> 
>> Am 28.01.24 um 12:39 schrieb Mikael Morin:
>> > Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
>> > > Dear all,
>> > > 
>> > > this patch is actually only a followup fix to generate the proper name
>> > > of an array reference in derived-type components for the runtime error
>> > > message generated for the bounds-checking code.  Without the proper
>> > > part ref, not only a user may get confused: I was, too...
>> > > 
>> > > The testcase is compile-only, as it is only important to check the
>> > > strings used in the error messages.
>> > > 
>> > > Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>> > > 
>> > 
>> > the change proper looks good, and is an improvement.  But I'm a little
>> > concerned by the production of references like in the test x1%vv%z which
>> > could be confusing and is strictly speaking invalid fortran (multiple
>> > non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
>> > x1%vv(...)%z or similar?
>> 
>> yes, that seems very reasonable, given that this is what NAG does.
>> 
>> We also have spurious %_data in some error messages that I'll try
>> to get rid off.
>> 
>
>I haven't looked at the patch, but sometimes (if not always) things
>like _data are marked with attr.artificial.  You might see if this
>will help with suppressing spurious messages.
>

Reminds me of
https://inbox.sourceware.org/fortran/20211114231748.376086cd@nbbrfq/

Maybe thats missing, i did not apply that yet, did i?

HTH

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-28 21:43     ` Steve Kargl
  2024-01-29  6:51       ` rep.dot.nop
@ 2024-01-29 17:25       ` Harald Anlauf
  2024-01-29 20:50         ` Harald Anlauf
  1 sibling, 1 reply; 15+ messages in thread
From: Harald Anlauf @ 2024-01-29 17:25 UTC (permalink / raw)
  To: sgk; +Cc: Mikael Morin, fortran, gcc-patches

Am 28.01.24 um 22:43 schrieb Steve Kargl:
> On Sun, Jan 28, 2024 at 08:56:24PM +0100, Harald Anlauf wrote:
>>
>> Am 28.01.24 um 12:39 schrieb Mikael Morin:
>>> Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
>>>> Dear all,
>>>>
>>>> this patch is actually only a followup fix to generate the proper name
>>>> of an array reference in derived-type components for the runtime error
>>>> message generated for the bounds-checking code.  Without the proper
>>>> part ref, not only a user may get confused: I was, too...
>>>>
>>>> The testcase is compile-only, as it is only important to check the
>>>> strings used in the error messages.
>>>>
>>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>>
>>>
>>> the change proper looks good, and is an improvement.  But I'm a little
>>> concerned by the production of references like in the test x1%vv%z which
>>> could be confusing and is strictly speaking invalid fortran (multiple
>>> non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
>>> x1%vv(...)%z or similar?
>>
>> yes, that seems very reasonable, given that this is what NAG does.
>>
>> We also have spurious %_data in some error messages that I'll try
>> to get rid off.
>>
>
> I haven't looked at the patch, but sometimes (if not always) things
> like _data are marked with attr.artificial.  You might see if this
> will help with suppressing spurious messages.

I was talking about the generated format strings of runtime error
messages.

program p
   implicit none
   type t
      real :: zzz(10) = 42
   end type t
   class(t), allocatable :: xx(:)
   integer :: j
   j = 0
   allocate (t :: xx(1))
   print *, xx(1)% zzz(j)
end

This is generating the following error at runtime since at least gcc-7:

Fortran runtime error: Index '0' of dimension 1 of array 'xx%_data%zzz'
below lower bound of 1

I believe you were recalling bogus warnings at compile time.
There are no warnings here, and there shouldn't.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-29 17:25       ` Harald Anlauf
@ 2024-01-29 20:50         ` Harald Anlauf
  2024-01-30 10:38           ` Mikael Morin
  0 siblings, 1 reply; 15+ messages in thread
From: Harald Anlauf @ 2024-01-29 20:50 UTC (permalink / raw)
  To: sgk; +Cc: Mikael Morin, fortran, gcc-patches

Am 29.01.24 um 18:25 schrieb Harald Anlauf:
> I was talking about the generated format strings of runtime error
> messages.
>
> program p
>    implicit none
>    type t
>       real :: zzz(10) = 42
>    end type t
>    class(t), allocatable :: xx(:)
>    integer :: j
>    j = 0
>    allocate (t :: xx(1))
>    print *, xx(1)% zzz(j)
> end
>
> This is generating the following error at runtime since at least gcc-7:
>
> Fortran runtime error: Index '0' of dimension 1 of array 'xx%_data%zzz'
> below lower bound of 1

Of course this is easily suppressed by:

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 1e0d698a949..fa0e00a28a6 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -4054,7 +4054,8 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref *
ar, gfc_expr *expr,
  	{
  	  if (ref->type == REF_ARRAY && &ref->u.ar == ar)
  	    break;
-	  if (ref->type == REF_COMPONENT)
+	  if (ref->type == REF_COMPONENT
+	      && strcmp (ref->u.c.component->name, "_data") != 0)
  	    {
  	      strcat (var_name, "%%");
  	      strcat (var_name, ref->u.c.component->name);


I have been contemplating the generation the full chain of references as
suggested by Mikael and supported by NAG.  The main issue is: how do I
easily generate that call?

gfc_trans_runtime_check is a vararg function, but what I would rather
have is a function that takes either a (chained?) list of trees or
an array of trees holding the (co-)indices of the reference.

Is there an example, or a recommendation which variant to prefer?

Thanks,
Harald


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-29 20:50         ` Harald Anlauf
@ 2024-01-30 10:38           ` Mikael Morin
  2024-01-30 10:46             ` Mikael Morin
  0 siblings, 1 reply; 15+ messages in thread
From: Mikael Morin @ 2024-01-30 10:38 UTC (permalink / raw)
  To: Harald Anlauf, sgk; +Cc: fortran, gcc-patches

Le 29/01/2024 à 21:50, Harald Anlauf a écrit :
> Am 29.01.24 um 18:25 schrieb Harald Anlauf:
>> I was talking about the generated format strings of runtime error
>> messages.
>>
>> program p
>>    implicit none
>>    type t
>>       real :: zzz(10) = 42
>>    end type t
>>    class(t), allocatable :: xx(:)
>>    integer :: j
>>    j = 0
>>    allocate (t :: xx(1))
>>    print *, xx(1)% zzz(j)
>> end
>>
>> This is generating the following error at runtime since at least gcc-7:
>>
>> Fortran runtime error: Index '0' of dimension 1 of array 'xx%_data%zzz'
>> below lower bound of 1
> 
> Of course this is easily suppressed by:
> 
> diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
> index 1e0d698a949..fa0e00a28a6 100644
> --- a/gcc/fortran/trans-array.cc
> +++ b/gcc/fortran/trans-array.cc
> @@ -4054,7 +4054,8 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref *
> ar, gfc_expr *expr,
>       {
>         if (ref->type == REF_ARRAY && &ref->u.ar == ar)
>           break;
> -      if (ref->type == REF_COMPONENT)
> +      if (ref->type == REF_COMPONENT
> +          && strcmp (ref->u.c.component->name, "_data") != 0)
>           {
>             strcat (var_name, "%%");
>             strcat (var_name, ref->u.c.component->name);
> 
> 
> I have been contemplating the generation the full chain of references as
> suggested by Mikael and supported by NAG.


To be clear, my suggestion was to have the question marks (or dashes, 
dots, stars, whatever) literally in the array reference, without the 
actual values of the array indexes.

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


> The main issue is: how do I easily generate that call?

> gfc_trans_runtime_check is a vararg function, but what I would rather
> have is a function that takes either a (chained?) list of trees or
> an array of trees holding the (co-)indices of the reference.
> 
> Is there an example, or a recommendation which variant to prefer?
> 
None that I know.
For a scalarized expression, the values are present (among others) in 
the gfc_loopinfo::ss linked list, maybe just use that?
In any case, I agree it would be nice to have, but it would probably be 
a non-negligible amount of new error-prone code; I would rather not 
attempt this during the stage4 stabilization phase as we are currently.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-30 10:38           ` Mikael Morin
@ 2024-01-30 10:46             ` Mikael Morin
  2024-03-10 21:31               ` [PATCH, v2] " Harald Anlauf
  0 siblings, 1 reply; 15+ messages in thread
From: Mikael Morin @ 2024-01-30 10:46 UTC (permalink / raw)
  To: Harald Anlauf, sgk; +Cc: fortran, gcc-patches

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
  2024-01-30 10:46             ` Mikael Morin
@ 2024-03-10 21:31               ` Harald Anlauf
  2024-03-15 16:31                 ` Mikael Morin
  0 siblings, 1 reply; 15+ messages in thread
From: Harald Anlauf @ 2024-03-10 21:31 UTC (permalink / raw)
  To: Mikael Morin, sgk; +Cc: fortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

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
>

[-- Attachment #2: pr30802-part2-v2.diff --]
[-- Type: text/x-patch, Size: 9290 bytes --]

From cdf3b197beed0ce1649661b2132643b54cbade8d Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
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_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)
 	{
 	  if (ref->type == REF_ARRAY && &ref->u.ar == ar)
 	    break;
-	  if (ref->type == REF_COMPONENT)
-	    {
-	      strcat (var_name, "%%");
-	      strcat (var_name, ref->u.c.component->name);
-	    }
+	  if (ref->type == REF_COMPONENT
+	      && strcmp (ref->u.c.component->name, "_data") != 0)
+	    var_name = ref->u.c.component->name;
 	}
     }
 
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_17.f90 b/gcc/testsuite/gfortran.dg/bounds_check_17.f90
index 50d66c75a80..41e119eadbe 100644
--- a/gcc/testsuite/gfortran.dg/bounds_check_17.f90
+++ b/gcc/testsuite/gfortran.dg/bounds_check_17.f90
@@ -23,4 +23,4 @@ z(i)%y(j)%x(k)=0
 
 END
 
-! { dg-output "At line 22 of file .*bounds_check_17.f90.*Fortran runtime error: Index '11' of dimension 1 of array 'z%y%x' above upper bound of 10" }
+! { dg-output "At line 22 of file .*bounds_check_17.f90.*Fortran runtime error: Index '11' of dimension 1 of array 'x' above upper bound of 10" }
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90
index 90329131158..a549f0d5c23 100644
--- a/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90
+++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_6.f90
@@ -1,7 +1,7 @@
 ! { dg-do run }
 ! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" }
 ! { dg-output "At line 18 .*" }
-! { dg-shouldfail "dimension 3 of array 'u%z' outside of expected range" }
+! { dg-shouldfail "dimension 3 of array 'z' outside of expected range" }
 !
 ! PR fortran/30802 - improve bounds-checking for array sections
 
@@ -25,5 +25,6 @@ contains
   end subroutine foo
 end program test
 
-! { dg-final { scan-tree-dump-times "'u%%z.' outside of expected range" 2 "original" } }
-! { dg-final { scan-tree-dump-times "'x.' outside of expected range" 4 "original" } }
+! { dg-final { scan-tree-dump-times "'z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'x.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 3 of array .'x.' outside of expected range" 2 "original" } }
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90
new file mode 100644
index 00000000000..df1f9ca921a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90
@@ -0,0 +1,48 @@
+! { dg-do compile }
+! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" }
+!
+! PR fortran/30802 - improve bounds-checking for array references
+!
+! Use proper array component references in runtime error message.
+
+program test
+  implicit none
+  integer :: k = 0
+  type t
+     real, dimension(10,20,30) :: z = 23
+  end type t
+  type u
+     type(t) :: vv(4,5)
+     complex :: cc(6,7)
+  end type u
+  type vec
+     integer :: xx(3) = [2,4,6]
+  end type vec
+  type(t) :: uu,     ww(1)
+  type(u) :: x1, x2, y1(1), y2(1)
+
+  print *, uu   % z(1,k,:)           ! runtime check only for dimension 2 of z
+  print *, ww(1)% z(1,:,k)           ! runtime check only for dimension 3 of z
+  print *, x1   % vv(2,3)% z(1,:,k)  ! runtime check only for dimension 3 of z
+  print *, x2   % vv(k,:)% z(1,2,3)  ! runtime check only for dimension 1 of vv
+  print *, y1(1)% vv(2,3)% z(k,:,1)  ! runtime check only for dimension 1 of z
+  print *, y2(1)% vv(:,k)% z(1,2,3)  ! runtime check only for dimension 2 of vv
+  print *, y1(1)% cc(k,:)% re        ! runtime check only for dimension 1 of cc
+contains
+  subroutine sub (yy, k)
+    class(vec), intent(in) :: yy(:)
+    integer,    intent(in) :: k
+    print *, yy(1)%xx(k)
+  end
+end program test
+
+! { dg-final { scan-tree-dump-times "dimension 2 of array .'z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 3 of array .'z.' outside of expected range" 4 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'vv.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 2 of array .'vv.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'cc.' outside of expected range" 2 "original" } }
+
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'yy.' above upper bound" 1 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'xx.' below lower bound" 1 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'xx.' above upper bound" 1 "original" } }
diff --git a/gcc/testsuite/gfortran.dg/pr92050.f90 b/gcc/testsuite/gfortran.dg/pr92050.f90
index 64193878d8f..94d3f64d31a 100644
--- a/gcc/testsuite/gfortran.dg/pr92050.f90
+++ b/gcc/testsuite/gfortran.dg/pr92050.f90
@@ -50,4 +50,4 @@ program main
   call bad_update_foo(x)
 end program main
 
-! { dg-output "At line 39 of file .*pr92050.f90.*Fortran runtime error: Index '2' of dimension 1 of array 'this%m' above upper bound of 1" }
+! { dg-output "At line 39 of file .*pr92050.f90.*Fortran runtime error: Index '2' of dimension 1 of array 'm' above upper bound of 1" }
-- 
2.35.3


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
  2024-03-10 21:31               ` [PATCH, v2] " Harald Anlauf
@ 2024-03-15 16:31                 ` Mikael Morin
  2024-03-15 17:26                   ` Harald Anlauf
  0 siblings, 1 reply; 15+ messages in thread
From: Mikael Morin @ 2024-03-15 16:31 UTC (permalink / raw)
  To: Harald Anlauf, sgk; +Cc: fortran, gcc-patches

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
>>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
  2024-03-15 16:31                 ` Mikael Morin
@ 2024-03-15 17:26                   ` Harald Anlauf
  2024-03-15 19:29                     ` Mikael Morin
  0 siblings, 1 reply; 15+ messages in thread
From: Harald Anlauf @ 2024-03-15 17:26 UTC (permalink / raw)
  To: Mikael Morin, sgk; +Cc: fortran, gcc-patches

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
>>>
>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
  2024-03-15 17:26                   ` Harald Anlauf
@ 2024-03-15 19:29                     ` Mikael Morin
  2024-03-20 20:24                       ` [PATCH, v3] Fortran: improve array component description " Harald Anlauf
  0 siblings, 1 reply; 15+ messages in thread
From: Mikael Morin @ 2024-03-15 19:29 UTC (permalink / raw)
  To: Harald Anlauf, sgk; +Cc: fortran, gcc-patches

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
>>>>
>>
>>
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH, v3] Fortran: improve array component description in runtime error message [PR30802]
  2024-03-15 19:29                     ` Mikael Morin
@ 2024-03-20 20:24                       ` Harald Anlauf
  2024-03-21 13:07                         ` Mikael Morin
  0 siblings, 1 reply; 15+ messages in thread
From: Harald Anlauf @ 2024-03-20 20:24 UTC (permalink / raw)
  To: Mikael Morin, sgk; +Cc: fortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]

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



[-- Attachment #2: pr30802-part2-v3.diff --]
[-- Type: text/x-patch, Size: 10797 bytes --]

From 30d7cef086d440262b206bc39bcbcac89491b792 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
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 (&ref->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 * name = NULL;
+  gfc_expr *expr;
 
   if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS))
     return index;
@@ -3509,6 +3583,12 @@ 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)
+    name = ref_name = abridged_ref_name (expr, NULL);
+
   if (VAR_P (descriptor))
     name = IDENTIFIER_POINTER (DECL_NAME (descriptor));
 
@@ -3562,6 +3642,7 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n,
       free (msg);
     }
 
+  free (ref_name);
   return index;
 }
 
@@ -3573,36 +3654,17 @@ 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;
   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);
-
       for (ref = expr->ref; ref; ref = ref->next)
 	{
-	  /* Append component name.  */
-	  if (ref->type == REF_COMPONENT)
-	    {
-	      strcat (var_name, "%%");
-	      strcat (var_name, ref->u.c.component->name);
-	      continue;
-	    }
-
 	  if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION)
 	    {
 	      ar = &ref->u.ar;
+	      var_name = abridged_ref_name (expr, ar);
 	      for (dim = 0; dim < ar->dimen; dim++)
 		{
 		  if (ar->dimen_type[dim] == DIMEN_ELEMENT)
@@ -3618,6 +3680,7 @@ array_bound_check_elemental (gfc_se * se, gfc_ss * ss, gfc_expr * expr)
 					       var_name);
 		    }
 		}
+	      free (var_name);
 	    }
 	}
     }
@@ -4034,33 +4097,7 @@ 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_COMPONENT)
-	    len += 2 + strlen (ref->u.c.component->name);
-	}
-
-      var_name = XALLOCAVEC (char, len);
-      strcpy (var_name, sym->name);
-
-      for (ref = expr->ref; ref; ref = ref->next)
-	{
-	  if (ref->type == REF_ARRAY && &ref->u.ar == ar)
-	    break;
-	  if (ref->type == REF_COMPONENT)
-	    {
-	      strcat (var_name, "%%");
-	      strcat (var_name, ref->u.c.component->name);
-	    }
-	}
-    }
+    var_name = abridged_ref_name (expr, ar);
 
   decl = se->expr;
   if (UNLIMITED_POLY(sym)
@@ -4195,6 +4232,7 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr,
 	decl = NULL_TREE;
     }
 
+  free (var_name);
   se->expr = build_array_ref (se->expr, offset, decl, se->class_vptr);
 }
 
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_17.f90 b/gcc/testsuite/gfortran.dg/bounds_check_17.f90
index 50d66c75a80..e970727d7d9 100644
--- a/gcc/testsuite/gfortran.dg/bounds_check_17.f90
+++ b/gcc/testsuite/gfortran.dg/bounds_check_17.f90
@@ -23,4 +23,4 @@ z(i)%y(j)%x(k)=0
 
 END
 
-! { dg-output "At line 22 of file .*bounds_check_17.f90.*Fortran runtime error: Index '11' of dimension 1 of array 'z%y%x' above upper bound of 10" }
+! { dg-output "At line 22 of file .*bounds_check_17.f90.*Fortran runtime error: Index '11' of dimension 1 of array 'z\.\.\.%x' above upper bound of 10" }
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90
new file mode 100644
index 00000000000..7ee659f0c7e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90
@@ -0,0 +1,56 @@
+! { dg-do compile }
+! { dg-additional-options "-fcheck=bounds -g -fdump-tree-original" }
+!
+! PR fortran/30802 - improve bounds-checking for array references
+!
+! Use proper array component references in runtime error message.
+
+program test
+  implicit none
+  integer :: k = 0
+  type t
+     real, dimension(10,20,30) :: z = 23
+  end type t
+  type u
+     type(t) :: vv(4,5)
+     complex :: cc(6,7)
+  end type u
+  type vec
+     integer :: xx(3) = [2,4,6]
+  end type vec
+  type(t) :: uu,     ww(1)
+  type(u) :: x1, x2, y1(1), y2(1)
+
+  print *, uu   % z(1,k,:)           ! runtime check for dimension 2 of uu%z
+  print *, ww(1)% z(1,:,k)           ! runtime check for dimension 3 of ww...%z
+  print *, x1   % vv(2,3)% z(1,:,k)  ! runtime check for dimension 3 of x1...%z
+  print *, x2   % vv(k,:)% z(1,2,3)  ! runtime check for dimension 1 of x2%vv
+  print *, y1(k)% vv(2,3)% z(k,:,1)  ! runtime check for dimension 1 of y1
+                                     !           and for dimension 1 of y1...%z
+  print *, y2(1)% vv(:,k)% z(1,2,k)  ! runtime check for dimension 2 of y2...%vv
+                                     !           and for dimension 3 of y2...%z
+  print *, y1(1)% cc(k,:)% re        ! runtime check for dimension 1 of y1...%cc
+contains
+  subroutine sub (yy, k)
+    class(vec), intent(in) :: yy(:)
+    integer,    intent(in) :: k
+    print *, yy(1)%xx(k)             ! runtime checks for yy and yy...%xx
+  end
+end program test
+
+! { dg-final { scan-tree-dump-times "dimension 2 of array .'uu%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 3 of array .'ww\.\.\.%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 3 of array .'x1\.\.\.%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'x2%%vv.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'y1\.\.\.%%z.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 2 of array .'y2\.\.\.%%vv.' outside of expected range" 2 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'y1\.\.\.%%cc.' outside of expected range" 2 "original" } }
+
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'y1.' above upper bound" 1 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'y1.' below lower bound" 1 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 3 of array .'y2\.\.\.%%z.' above upper bound" 1 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 3 of array .'y2\.\.\.%%z.' below lower bound" 1 "original" } }
+
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'yy.' above upper bound" 1 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'yy\.\.\.%%xx.' above upper bound" 1 "original" } }
+! { dg-final { scan-tree-dump-times "dimension 1 of array .'yy\.\.\.%%xx.' below lower bound" 1 "original" } }
-- 
2.35.3


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, v3] Fortran: improve array component description in runtime error message [PR30802]
  2024-03-20 20:24                       ` [PATCH, v3] Fortran: improve array component description " Harald Anlauf
@ 2024-03-21 13:07                         ` Mikael Morin
  0 siblings, 0 replies; 15+ messages in thread
From: Mikael Morin @ 2024-03-21 13:07 UTC (permalink / raw)
  To: Harald Anlauf, sgk; +Cc: fortran, gcc-patches

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
> 
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-03-21 13:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-24 21:39 [PATCH] Fortran: use name of array component in runtime error message [PR30802] Harald Anlauf
2024-01-28 11:39 ` Mikael Morin
2024-01-28 19:56   ` Harald Anlauf
2024-01-28 21:43     ` Steve Kargl
2024-01-29  6:51       ` rep.dot.nop
2024-01-29 17:25       ` Harald Anlauf
2024-01-29 20:50         ` Harald Anlauf
2024-01-30 10:38           ` Mikael Morin
2024-01-30 10:46             ` Mikael Morin
2024-03-10 21:31               ` [PATCH, v2] " Harald Anlauf
2024-03-15 16:31                 ` Mikael Morin
2024-03-15 17:26                   ` Harald Anlauf
2024-03-15 19:29                     ` Mikael Morin
2024-03-20 20:24                       ` [PATCH, v3] Fortran: improve array component description " Harald Anlauf
2024-03-21 13:07                         ` Mikael Morin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).