[fortran] PR103471 - [11/12/13/14 Regression] ICE in gfc_typenode_for_spec, at fortran/trans-types.c:1114

Message ID CAGkQGiJv6oqU8_PWiWq-U4BpyDrk8nhvqtm8PC56cQVZWJQpxw@mail.gmail.com
State New
Headers
Series [fortran] PR103471 - [11/12/13/14 Regression] ICE in gfc_typenode_for_spec, at fortran/trans-types.c:1114 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Paul Richard Thomas April 19, 2024, 4:52 p.m. UTC
  Hi All,

This is a more or less obvious patch. The action is in resolve.cc. The
chunk in symbol.cc is a tidy up of a diagnostic marker to distinguish where
the 'no IMPLICIT type' error was coming from and the chunk in trans-decl.cc
follows from discussion with Harald on the PR.

Regtests fine. OK for mainline and backporting in a couple of weeks?

Paul

Fortran: Detect 'no implicit type' error in right place [PR103471]

2024-04-19  Paul Thomas  <pault@gcc.gnu.org>

gcc/fortran
PR fortran/103471
* resolve.cc (gfc_resolve_index_1): Block index expressions of
unknown type from being converted to default integer, avoiding
the fatal error in trans-decl.cc.
* symbol.cc (gfc_set_default_type): Remove '(symbol)' from the
'no IMPLICIT type' error message.
* trans-decl.cc (gfc_get_symbol_decl): Change fatal error locus
to that of the symbol declaration.
(gfc_trans_deferred_vars): Remove two trailing tabs.

gcc/testsuite/
PR fortran/103471
* gfortran.dg/pr103471.f90: New test.
  

Comments

Harald Anlauf April 19, 2024, 6:33 p.m. UTC | #1
Hi Paul,

the patch is OK, but I had to manually fix it.  I wonder how you managed
to produce:

diff --git a/gcc/testsuite/gfortran.dg/pr93484.f90
b/gcc/testsuite/gfortran.dg/pr93484.f90
new file mode 100644
index 00000000000..4dcad47e8da
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr103471.f90
@@ -0,0 +1,13 @@

A minor comment on the error message and the testcase.
Take for example:

subroutine sub
   implicit none
   real, external :: x
   real           :: y(10)
   integer :: kk
   print *, [real(x(k))]
!  print *, [real(y(k))]
end

The original testcase in the PR would - without implicit none -
resemble the function invocation x(k) here and emit the error:

Fatal Error: k at (1) has no default type
compilation terminated.

while commenting the first print and uncommenting the second
would emit the message

Error: Symbol 'k' at (1) has no IMPLICIT type; did you mean 'kk'?

Thus I have the impression that the testcase tests something different
on the one hand, and on the other I wonder if we would want to change
the error message and replace "no default type" to "no IMPLICIT type".
It still would not hit the fuzzy check, but that is something that
might not be important now.

Thanks,
Harald


On 4/19/24 18:52, Paul Richard Thomas wrote:
> Hi All,
>
> This is a more or less obvious patch. The action is in resolve.cc. The
> chunk in symbol.cc is a tidy up of a diagnostic marker to distinguish where
> the 'no IMPLICIT type' error was coming from and the chunk in trans-decl.cc
> follows from discussion with Harald on the PR.
>
> Regtests fine. OK for mainline and backporting in a couple of weeks?
>
> Paul
>
> Fortran: Detect 'no implicit type' error in right place [PR103471]
>
> 2024-04-19  Paul Thomas  <pault@gcc.gnu.org>
>
> gcc/fortran
> PR fortran/103471
> * resolve.cc (gfc_resolve_index_1): Block index expressions of
> unknown type from being converted to default integer, avoiding
> the fatal error in trans-decl.cc.
> * symbol.cc (gfc_set_default_type): Remove '(symbol)' from the
> 'no IMPLICIT type' error message.
> * trans-decl.cc (gfc_get_symbol_decl): Change fatal error locus
> to that of the symbol declaration.
> (gfc_trans_deferred_vars): Remove two trailing tabs.
>
> gcc/testsuite/
> PR fortran/103471
> * gfortran.dg/pr103471.f90: New test.
>
  
Paul Richard Thomas April 20, 2024, 7:54 a.m. UTC | #2
Hi Harald,

>
> the patch is OK, but I had to manually fix it.  I wonder how you managed
> to produce:
>

Yes, I had to use --whitespace fix when I reapplied it a few minutes ago.

>
> diff --git a/gcc/testsuite/gfortran.dg/pr93484.f90
>

I had followed comment 1 in the PR and wrongly named the file because of
it. Now corrected.


>
> subroutine sub
>    implicit none
>    real, external :: x
>    real           :: y(10)
>    integer :: kk
>    print *, [real(x(k))]
> !  print *, [real(y(k))]
> end
>

This is another problem, somewhere upstream from resolve.cc, which I have
just spent an hour failing to find. In the presence of both print
statements, in no matter which order, it is the error in trans-decl.cc that
applies.


> Thus I have the impression that the testcase tests something different
> on the one hand, and on the other I wonder if we would want to change
> the error message and replace "no default type" to "no IMPLICIT type".
> It still would not hit the fuzzy check, but that is something that
> might not be important now.
>

The fuzzy check was intended to ensure that the error was being detected in
the "right" place. I want to keep the "no default type" message for the
time being at least so as to identify exactly where it comes from. Getting
to trans-decl.cc with an unknown type is just wrong.

I'll come back to you on this.

Thanks for the report.

Paul
  
Harald Anlauf April 20, 2024, 1:05 p.m. UTC | #3
Hi Paul!

On 4/20/24 09:54, Paul Richard Thomas wrote:
>> subroutine sub
>>     implicit none
>>     real, external :: x
>>     real           :: y(10)
>>     integer :: kk
>>     print *, [real(x(k))]
>> !  print *, [real(y(k))]
>> end
>>
>
> This is another problem, somewhere upstream from resolve.cc, which I have
> just spent an hour failing to find. In the presence of both print
> statements, in no matter which order, it is the error in trans-decl.cc that
> applies.

Indeed, the gfc_fatal_error always wins.

(I had tried to replace it with gfc_error()/return NULL_TREE, but then
I hit an ICE later on.  When trying to find out who added the said
code, guess whom I found :)

>
>> Thus I have the impression that the testcase tests something different
>> on the one hand, and on the other I wonder if we would want to change
>> the error message and replace "no default type" to "no IMPLICIT type".
>> It still would not hit the fuzzy check, but that is something that
>> might not be important now.
>>
>
> The fuzzy check was intended to ensure that the error was being detected in
> the "right" place. I want to keep the "no default type" message for the
> time being at least so as to identify exactly where it comes from. Getting
> to trans-decl.cc with an unknown type is just wrong.

True.

> I'll come back to you on this.

This PR is marked as a regression.  Depending on your progress,
it might be worth to consider fixing what you think is needed
to get rid of the regression marker and defer the improvement
of the diagnostics to a second patch.

Harald

> Thanks for the report.
>
> Paul
>
  

Patch

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 6b3e5ba4fcb..9b7fabd3707 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -5001,7 +5001,8 @@  gfc_resolve_index_1 (gfc_expr *index, int check_scalar,

   if ((index->ts.kind != gfc_index_integer_kind
        && force_index_integer_kind)
-      || index->ts.type != BT_INTEGER)
+      || (index->ts.type != BT_INTEGER
+	  && index->ts.type != BT_UNKNOWN))
     {
       gfc_clear_ts (&ts);
       ts.type = BT_INTEGER;
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 3a3b6de5cec..8f7deac1d1e 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -320,7 +320,7 @@  gfc_set_default_type (gfc_symbol *sym, int error_flag, gfc_namespace *ns)
 		       "; did you mean %qs?",
 		       sym->name, &sym->declared_at, guessed);
 	  else
-	    gfc_error ("Symbol %qs at %L has no IMPLICIT type(symbol)",
+	    gfc_error ("Symbol %qs at %L has no IMPLICIT type",
 		       sym->name, &sym->declared_at);
 	  sym->attr.untyped = 1; /* Ensure we only give an error once.  */
 	}
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index e160c5c98c1..301439baaf5 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -1797,7 +1797,8 @@  gfc_get_symbol_decl (gfc_symbol * sym)
     }

   if (sym->ts.type == BT_UNKNOWN)
-    gfc_fatal_error ("%s at %C has no default type", sym->name);
+    gfc_fatal_error ("%s at %L has no default type", sym->name,
+		     &sym->declared_at);

   if (sym->attr.intrinsic)
     gfc_internal_error ("intrinsic variable which isn't a procedure");
@@ -5214,8 +5215,8 @@  gfc_trans_deferred_vars (gfc_symbol * proc_sym, gfc_wrapped_block * block)
 	tree tmp = lookup_attribute ("omp allocate",
 				     DECL_ATTRIBUTES (n->sym->backend_decl));
 	tmp = TREE_VALUE (tmp);
-	TREE_PURPOSE (tmp) = se.expr;
-	TREE_VALUE (tmp) = align;
+	TREE_PURPOSE (tmp) = se.expr;
+	TREE_VALUE (tmp) = align;
 	TREE_PURPOSE (TREE_CHAIN (tmp)) = init_stmtlist;
 	TREE_VALUE (TREE_CHAIN (tmp)) = cleanup_stmtlist;
       }
diff --git a/gcc/testsuite/gfortran.dg/pr93484.f90 b/gcc/testsuite/gfortran.dg/pr93484.f90
new file mode 100644
index 00000000000..4dcad47e8da
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr103471.f90
@@ -0,0 +1,13 @@ 
+! { dg-do compile }
+! Test the fix for PR103471 in which, rather than giving a "no IMPLICIT type"
+! message, gfortran took to ICEing. The fuzzy symbol check for 'kk' demonstrates
+! that the error is being detected in the right place.
+!
+! Contributed by Gerhard Steinmetz  <gscfq@t-online.de>
+!
+program p
+   implicit none
+   integer, parameter :: x(4) = [1,2,3,4]
+   integer :: kk
+   print *, [real(x(k))] ! { dg-error "has no IMPLICIT type; did you mean .kk.\\?" }
+end