[PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Harald Anlauf]

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

Dear all,

I intend to commit the attached patch as obvious to mainline
within the next 24h unless someone complains.  It replaces a
lazy gfc_internal_error by an explicit error message and an
error recovery path.

As a side-effect, we now diagnose a previously missed error
in testcase gfortran.dg/associate_54.f90 similarly to Intel.

Regtested on x86_64-pc-linux-gnu.

Thanks,
Harald


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fortran-error-recovery-on-invalid-array-reference-of.patch --]
[-- Type: text/x-patch, Size: 3271 bytes --]

From e6ecc4d8227afea565b0555e95a4f5dcb8f4ecab Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Mon, 18 Jul 2022 22:34:53 +0200
Subject: [PATCH] Fortran: error recovery on invalid array reference of
 non-array [PR103590]

gcc/fortran/ChangeLog:

	PR fortran/103590
	* resolve.cc (find_array_spec): Change function result to bool to
	enable error recovery.  Generate error message for missing array
	spec instead of an internal error.
	(gfc_resolve_ref): Use function result from find_array_spec for
	error recovery.

gcc/testsuite/ChangeLog:

	PR fortran/103590
	* gfortran.dg/associate_54.f90: Adjust.
	* gfortran.dg/associate_59.f90: New test.
---
 gcc/fortran/resolve.cc                     | 13 ++++++++++---
 gcc/testsuite/gfortran.dg/associate_54.f90 |  3 +--
 gcc/testsuite/gfortran.dg/associate_59.f90 |  9 +++++++++
 3 files changed, 20 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/associate_59.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 2ebf076f730..dacd33561d0 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -4976,7 +4976,7 @@ gfc_resolve_dim_arg (gfc_expr *dim)
 static void
 resolve_assoc_var (gfc_symbol* sym, bool resolve_target);

-static void
+static bool
 find_array_spec (gfc_expr *e)
 {
   gfc_array_spec *as;
@@ -5004,7 +5004,11 @@ find_array_spec (gfc_expr *e)
       {
       case REF_ARRAY:
 	if (as == NULL)
-	  gfc_internal_error ("find_array_spec(): Missing spec");
+	  {
+	    gfc_error ("Symbol %qs at %L has not been declared as an array",
+		       e->symtree->n.sym->name, &e->where);
+	    return false;
+	  }

 	ref->u.ar.as = as;
 	as = NULL;
@@ -5028,6 +5032,8 @@ find_array_spec (gfc_expr *e)

   if (as != NULL)
     gfc_internal_error ("find_array_spec(): unused as(2)");
+
+  return true;
 }


@@ -5346,7 +5352,8 @@ gfc_resolve_ref (gfc_expr *expr)
   for (ref = expr->ref; ref; ref = ref->next)
     if (ref->type == REF_ARRAY && ref->u.ar.as == NULL)
       {
-	find_array_spec (expr);
+	if (!find_array_spec (expr))
+	  return false;
 	break;
       }

diff --git a/gcc/testsuite/gfortran.dg/associate_54.f90 b/gcc/testsuite/gfortran.dg/associate_54.f90
index 003175a47fd..b23a4f160ac 100644
--- a/gcc/testsuite/gfortran.dg/associate_54.f90
+++ b/gcc/testsuite/gfortran.dg/associate_54.f90
@@ -26,9 +26,8 @@ contains
     integer, intent(in) :: a
     associate (state => obj%state(TEST_STATES)) ! { dg-error "is used as array" }
 !      state = a
-      state(TEST_STATE) = a
+      state(TEST_STATE) = a ! { dg-error "has not been declared as an array" }
     end associate
   end subroutine test_alter_state1

 end module test
-
diff --git a/gcc/testsuite/gfortran.dg/associate_59.f90 b/gcc/testsuite/gfortran.dg/associate_59.f90
new file mode 100644
index 00000000000..2da97731d39
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associate_59.f90
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/103590 - ICE: find_array_spec(): Missing spec
+! Contributed by G.Steinmetz
+
+program p
+  associate (a => 1)
+    print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER expression" }
+  end associate
+end
--
2.35.3

Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Mikael Morin]

Hello,

the principle looks good, but...

Le 18/07/2022 à 22:43, Harald Anlauf via Fortran a écrit :

> diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
> index 2ebf076f730..dacd33561d0 100644
> --- a/gcc/fortran/resolve.cc
> +++ b/gcc/fortran/resolve.cc
> @@ -5004,7 +5004,11 @@ find_array_spec (gfc_expr *e)
>        {
>        case REF_ARRAY:
>  	if (as == NULL)
> -	  gfc_internal_error ("find_array_spec(): Missing spec");
> +	  {
> +	    gfc_error ("Symbol %qs at %L has not been declared as an array",
> +		       e->symtree->n.sym->name, &e->where);

... the error here only makes sense if the array reference follows a 
variable reference.  If it follows a derived type component reference, a 
slightly different error message would be more appropriate.

Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Harald Anlauf]

Hi Mikael,

Am 19.07.22 um 11:03 schrieb Mikael Morin:
> Hello,
> 
> the principle looks good, but...
> 
> Le 18/07/2022 à 22:43, Harald Anlauf via Fortran a écrit :
> 
>> diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
>> index 2ebf076f730..dacd33561d0 100644
>> --- a/gcc/fortran/resolve.cc
>> +++ b/gcc/fortran/resolve.cc
>> @@ -5004,7 +5004,11 @@ find_array_spec (gfc_expr *e)
>>        {
>>        case REF_ARRAY:
>>      if (as == NULL)
>> -      gfc_internal_error ("find_array_spec(): Missing spec");
>> +      {
>> +        gfc_error ("Symbol %qs at %L has not been declared as an array",
>> +               e->symtree->n.sym->name, &e->where);
> 
> ... the error here only makes sense if the array reference follows a 
> variable reference.  If it follows a derived type component reference, a 
> slightly different error message would be more appropriate.

how detailed or tailored should the error message be, or can
we just have a more generic message, like "Name at %L ...",
or "Invalid subscript reference at %L"?  We seem to not hit
that internal error very often...

I have played only little with invalid code in the present context,
but often hit another code path that shows up in associate_54.f90
and gives

Error: Associate-name 'state' at (1) is used as array

For the testcase in the PR, Intel says:

associate_59.f90(7): error #6410: This name has not been declared as an 
array or a function.   [A]
     print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER 
expression" }
------------------------^

Crayftn 14.0 says:

   Improper ir tree in expr_semantics.

     print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER 
expression" }
                          ^ 

ftn-873 crayftn: ERROR P, File = associate_59.f90, Line = 7, Column = 26
   Invalid subscripted reference of a scalar ASSOCIATE name.


gfortran's behavior during error handling is difficult to understand.
While the proposed new error message is emitted for associate_54.f90,
it never makes it far enough for the testcase of the present PR
(associate_59.f90).

Thanks,
Harald

Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Harald Anlauf]

Hi Mikael,

Am 19.07.22 um 11:03 schrieb Mikael Morin:
> Hello,
>
> the principle looks good, but...
>
> Le 18/07/2022 à 22:43, Harald Anlauf via Fortran a écrit :
>
>> diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
>> index 2ebf076f730..dacd33561d0 100644
>> --- a/gcc/fortran/resolve.cc
>> +++ b/gcc/fortran/resolve.cc
>> @@ -5004,7 +5004,11 @@ find_array_spec (gfc_expr *e)
>>        {
>>        case REF_ARRAY:
>>      if (as == NULL)
>> -      gfc_internal_error ("find_array_spec(): Missing spec");
>> +      {
>> +        gfc_error ("Symbol %qs at %L has not been declared as an array",
>> +               e->symtree->n.sym->name, &e->where);
>
> ... the error here only makes sense if the array reference follows a
> variable reference.  If it follows a derived type component reference, a
> slightly different error message would be more appropriate.

how detailed or tailored should the error message be, or can
we just have a more generic message, like "Name at %L ...",
or "Invalid subscript reference at %L"?  We seem to not hit
that internal error very often...

I have played only little with invalid code in the present context,
but often hit another code path that shows up in associate_54.f90
and gives

Error: Associate-name 'state' at (1) is used as array

For the testcase in the PR, Intel says:

associate_59.f90(7): error #6410: This name has not been declared as an
array or a function.   [A]
     print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER
expression" }
------------------------^

Crayftn 14.0 says:

   Improper ir tree in expr_semantics.

     print *, [character(a(1)) :: '1'] ! { dg-error "Scalar INTEGER
expression" }
                          ^

ftn-873 crayftn: ERROR P, File = associate_59.f90, Line = 7, Column = 26
   Invalid subscripted reference of a scalar ASSOCIATE name.


gfortran's behavior during error handling is difficult to understand.
While the proposed new error message is emitted for associate_54.f90,
it never makes it far enough for the testcase of the present PR
(associate_59.f90).

Thanks,
Harald

Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Mikael Morin]

Le 19/07/2022 à 21:09, Harald Anlauf via Fortran a écrit :
> Hi Mikael,
> 
> Am 19.07.22 um 11:03 schrieb Mikael Morin:
>> Hello,
>>
>> the principle looks good, but...
>>
>> Le 18/07/2022 à 22:43, Harald Anlauf via Fortran a écrit :
>>
>>> diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
>>> index 2ebf076f730..dacd33561d0 100644
>>> --- a/gcc/fortran/resolve.cc
>>> +++ b/gcc/fortran/resolve.cc
>>> @@ -5004,7 +5004,11 @@ find_array_spec (gfc_expr *e)
>>>        {
>>>        case REF_ARRAY:
>>>      if (as == NULL)
>>> -      gfc_internal_error ("find_array_spec(): Missing spec");
>>> +      {
>>> +        gfc_error ("Symbol %qs at %L has not been declared as an 
>>> array",
>>> +               e->symtree->n.sym->name, &e->where);
>>
>> ... the error here only makes sense if the array reference follows a 
>> variable reference.  If it follows a derived type component reference, 
>> a slightly different error message would be more appropriate.
> 
> how detailed or tailored should the error message be, or can
> we just have a more generic message, like "Name at %L ...",
> or "Invalid subscript reference at %L"?  We seem to not hit
> that internal error very often...
> 
It could be anything better than the (unhelpfull) internal error it is 
replacing.
I suggest for example "Invalid array reference of a non-array entity at 
%L".  Cray’s words, or Intel’s, or your other propositions work as well.

> 
> gfortran's behavior during error handling is difficult to understand.
> While the proposed new error message is emitted for associate_54.f90,
> it never makes it far enough for the testcase of the present PR
> (associate_59.f90).
> 
Indeed.  We try to match several types of statement one after the other, 
and each failed match can possibly register an error.  But it is emitted 
only if all have failed (see gfc_error_check).  There is no choice of 
the best error; the last one registered wins.

This buffering behavior is controlled by calls to gfc_buffer_error(...). 
  Error handling during resolution doesn’t need to delay error reporting 
as far as I know, and the calls to gfc_buffer_error (false) seem to 
correctly disable buffering at the end of every call to next_statement. 
  I don’t know why it remains active in this case.

Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Harald Anlauf]

Hi Mikael,

Am 19.07.22 um 22:53 schrieb Mikael Morin:
> It could be anything better than the (unhelpfull) internal error it is 
> replacing.
> I suggest for example "Invalid array reference of a non-array entity at 
> %L".

yes, that's much better!  The attached updated patch uses this.

Committed: r13-1757-gf838d15641d256e21ffc126c3277b290ed743928


>> gfortran's behavior during error handling is difficult to understand.
>> While the proposed new error message is emitted for associate_54.f90,
>> it never makes it far enough for the testcase of the present PR
>> (associate_59.f90).
>>
> Indeed.  We try to match several types of statement one after the other, 
> and each failed match can possibly register an error.  But it is emitted 
> only if all have failed (see gfc_error_check).  There is no choice of 
> the best error; the last one registered wins.
> 
> This buffering behavior is controlled by calls to gfc_buffer_error(...). 
>   Error handling during resolution doesn’t need to delay error reporting 
> as far as I know, and the calls to gfc_buffer_error (false) seem to 
> correctly disable buffering at the end of every call to next_statement. 
>   I don’t know why it remains active in this case.
> 

Alright, I should try to remember this and take a closer look next time
I get confused about not getting the error message I wanted...

Thanks,
Harald

Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Harald Anlauf]

Hi Mikael,

Am 19.07.22 um 22:53 schrieb Mikael Morin:
> It could be anything better than the (unhelpfull) internal error it is
> replacing.
> I suggest for example "Invalid array reference of a non-array entity at
> %L".

yes, that's much better!  The attached updated patch uses this.

Committed: r13-1757-gf838d15641d256e21ffc126c3277b290ed743928


>> gfortran's behavior during error handling is difficult to understand.
>> While the proposed new error message is emitted for associate_54.f90,
>> it never makes it far enough for the testcase of the present PR
>> (associate_59.f90).
>>
> Indeed.  We try to match several types of statement one after the other,
> and each failed match can possibly register an error.  But it is emitted
> only if all have failed (see gfc_error_check).  There is no choice of
> the best error; the last one registered wins.
>
> This buffering behavior is controlled by calls to gfc_buffer_error(...).
>   Error handling during resolution doesn’t need to delay error reporting
> as far as I know, and the calls to gfc_buffer_error (false) seem to
> correctly disable buffering at the end of every call to next_statement.
>   I don’t know why it remains active in this case.
>

Alright, I should try to remember this and take a closer look next time
I get confused about not getting the error message I wanted...

Thanks,
Harald

Re: [PATCH] Fortran: error recovery on invalid array reference of non-array [PR103590]

[Mikael Morin]

Le 19/07/2022 à 23:34, Harald Anlauf a écrit :
> 
> Committed: r13-1757-gf838d15641d256e21ffc126c3277b290ed743928
> 
Thanks.