Re: [Patch, fortran] PR79685 - [12/13/14/15 Regression] ICE on valid code in gfc_match_structure_constructor

2024-07-28 Thread Paul Richard Thomas
I have attached the updated and rather simpler patch.

Thanks for the prod, Mikael!

Paul

Fortran: Fix ICE with structure constructor in data statement [PR79685]

2024-07-28  Paul Thomas  

gcc/fortran
PR fortran/79685
* decl.cc (match_data_constant): Find the symtree instead of
the symbol so the use renamed symbols are found. Pass this and
the derived type to gfc_match_structure_constructor.
* match.h: Update prototype of gfc_match_structure_contructor.
* primary.cc (gfc_match_structure_constructor): Remove call to
gfc_get_ha_sym_tree and use caller supplied symtree instead.

gcc/testsuite/
PR fortran/79685
* gfortran.dg/use_rename_12.f90: New test.


On Sat, 27 Jul 2024 at 22:04, Paul Richard Thomas <
paul.richard.tho...@gmail.com> wrote:

> Hi Mikael,
>
> You were absolutely right. I looked at the caller and "just didn't get
> it". Thanks. I will resubmit when I get back from a business trip.
>
> Cordialement
>
> Paul
>
>
>
> On Sat, 27 Jul 2024 at 12:35, Mikael Morin  wrote:
>
>> Hello,
>>
>> Le 27/07/2024 à 11:25, Paul Richard Thomas a écrit :
>> > This patch is straightforward but I am still puzzled as to why it is
>> > necessary for the particular case. Having looked at all the other
>> chunks
>> > of frontend code involving use renaming, it seems that the process just
>> > works everywhere else. I tried putting the new code in gfc_find_symtree
>> > but it caused some regressions unless I pinned it down to the specific
>> > case of a structure constructor.
>> >
>> I think it works as expected, symtrees have the local names, and symbols
>> the original name, so if all that is available is the symbol, name
>> lookup may not work correctly with renaming.  And I think that there are
>> other places where it happens.
>>
>> In this case, it seems the caller can provide the local name, which
>> would avoid processing the use statements.  Did you try that?
>>
>> Mikael
>>
>
diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index b8308aeee55..119c9dffa03 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
@@ -376,6 +376,7 @@ match_data_constant (gfc_expr **result)
   gfc_expr *expr;
   match m;
   locus old_loc;
+  gfc_symtree *symtree;
 
   m = gfc_match_literal_constant (&expr, 1);
   if (m == MATCH_YES)
@@ -436,9 +437,11 @@ match_data_constant (gfc_expr **result)
   if (m != MATCH_YES)
 return m;
 
-  if (gfc_find_symbol (name, NULL, 1, &sym))
+  if (gfc_find_sym_tree (name, NULL, 1, &symtree))
 return MATCH_ERROR;
 
+  sym = symtree->n.sym;
+
   if (sym && sym->attr.generic)
 dt_sym = gfc_find_dt_in_generic (sym);
 
@@ -452,7 +455,7 @@ match_data_constant (gfc_expr **result)
   return MATCH_ERROR;
 }
   else if (dt_sym && gfc_fl_struct (dt_sym->attr.flavor))
-return gfc_match_structure_constructor (dt_sym, result);
+return gfc_match_structure_constructor (dt_sym, symtree, result);
 
   /* Check to see if the value is an initialization array expression.  */
   if (sym->value->expr_type == EXPR_ARRAY)
diff --git a/gcc/fortran/match.h b/gcc/fortran/match.h
index c2b7d69c37c..519ad194a15 100644
--- a/gcc/fortran/match.h
+++ b/gcc/fortran/match.h
@@ -101,7 +101,7 @@ match gfc_match_call (void);
 /* We want to use this function to check for a common-block-name
that can exist in a bind statement, so removed the "static"
declaration of the function in match.cc. */
- 
+
 match gfc_match_common_name (char *name);
 
 match gfc_match_common (void);
@@ -302,7 +302,7 @@ match gfc_match_bind_c_stmt (void);
 match gfc_match_bind_c (gfc_symbol *, bool);
 
 /* primary.cc.  */
-match gfc_match_structure_constructor (gfc_symbol *, gfc_expr **);
+match gfc_match_structure_constructor (gfc_symbol *, gfc_symtree *, gfc_expr **);
 match gfc_match_variable (gfc_expr **, int);
 match gfc_match_equiv_variable (gfc_expr **);
 match gfc_match_actual_arglist (int, gfc_actual_arglist **, bool = false);
diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
index 76f6bcb8a78..9000b9cbbbc 100644
--- a/gcc/fortran/primary.cc
+++ b/gcc/fortran/primary.cc
@@ -3520,18 +3520,16 @@ gfc_convert_to_structure_constructor (gfc_expr *e, gfc_symbol *sym, gfc_expr **c
 
 
 match
-gfc_match_structure_constructor (gfc_symbol *sym, gfc_expr **result)
+gfc_match_structure_constructor (gfc_symbol *sym, gfc_symtree *symtree,
+ gfc_expr **result)
 {
   match m;
   gfc_expr *e;
-  gfc_symtree *symtree;
   bool t = true;
 
-  gfc_get_ha_sym_tree (sym->name, &symtree);
-
   e = gfc_get_expr ();
-  e->symtree = symtree;
   e->expr_type = EXPR_FUNCTION;
+  e->symtree = symtree;
   e->where = gfc_current_locus;
 
   gcc_assert (gfc_fl_struct (sym->attr.flavor)
diff --git a/gcc/testsuite/gfortran.dg/use_rename_12.f90 b/gcc/testsuite/gfortran.dg/use_rename_12.f90
new file mode 100644
index 000..97f26f42f76
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/use_rename_12.f90
@@ -0,0 +1,37 @@
+! { dg-do compile }
+!
+! Test the fix for pr79685, which failed as in the comments below.
+!
+! Contributed by J

Re: [Patch, fortran] PR79685 - [12/13/14/15 Regression] ICE on valid code in gfc_match_structure_constructor

2024-07-28 Thread Mikael Morin

Le 28/07/2024 à 11:24, Paul Richard Thomas a écrit :

I have attached the updated and rather simpler patch.


OK for master and backport.
Thanks for the patch.


Thanks for the prod, Mikael!

Paul

Fortran: Fix ICE with structure constructor in data statement [PR79685]

2024-07-28  Paul Thomas  mailto:pa...@gcc.gnu.org>>

gcc/fortran
PR fortran/79685
* decl.cc (match_data_constant): Find the symtree instead of
the symbol so the use renamed symbols are found. Pass this and
the derived type to gfc_match_structure_constructor.
* match.h: Update prototype of gfc_match_structure_contructor.
* primary.cc (gfc_match_structure_constructor): Remove call to
gfc_get_ha_sym_tree and use caller supplied symtree instead.

gcc/testsuite/
PR fortran/79685
* gfortran.dg/use_rename_12.f90: New test.


On Sat, 27 Jul 2024 at 22:04, Paul Richard Thomas 
mailto:paul.richard.tho...@gmail.com>> 
wrote:


Hi Mikael,

You were absolutely right. I looked at the caller and "just didn't
get it". Thanks. I will resubmit when I get back from a business trip.

Cordialement

Paul



On Sat, 27 Jul 2024 at 12:35, Mikael Morin mailto:morin-mik...@orange.fr>> wrote:

Hello,

Le 27/07/2024 à 11:25, Paul Richard Thomas a écrit :
 > This patch is straightforward but I am still puzzled as to
why it is
 > necessary for the particular case. Having looked at all the
other chunks
 > of frontend code involving use renaming, it seems that the
process just
 > works everywhere else. I tried putting the new code in
gfc_find_symtree
 > but it caused some regressions unless I pinned it down to the
specific
 > case of a structure constructor.
 >
I think it works as expected, symtrees have the local names, and
symbols
the original name, so if all that is available is the symbol, name
lookup may not work correctly with renaming.  And I think that
there are
other places where it happens.

In this case, it seems the caller can provide the local name, which
would avoid processing the use statements.  Did you try that?

Mikael