Re: Ping [Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Stefano Zaghi]

Dear Paul and Jerry (and all gfortran brave heroes),

thank you very much for your great work!

Cheers
Stefano Zaghi  Ph.D. Aerospace Engineer

Research Scientist, Dept. of Computational Hydrodynamics at CNR-INSEAN
p: +39 0650299260 | m: +39 3497730036 | e: stefano.za...@gmail.com

Member of Fortran-FOSS-Programmers

Codes Showcase
OFF  Open source Finite volumes Fluid dynamics code
Lib_VTK_IO   Fortran library to write and read data conforming the VTK standard
FLAP Fortran command Line Arguments Parser for poor men
BeFoR64  Base64 encoding/decoding library for FoRtran poor men
FiNeRFortran INI ParseR and generator for FoRtran poor men
IR_Precision Fortran (standard 2003) module to develop portable codes
FoBis.py Fortran Building System for poor men
PreForM.py   Preprocessor for Fortran poor men
MaTiSSe.py   Markdown To Impressive Scientific Slides



On Sun, Feb 19, 2017 at 7:29 PM, Paul Richard Thomas
 wrote:
> Hi Everybody,
>
> With Jerry's OK for a commit to trunk, committed as revision 245580.
>
> I will wait a few weeks before a commit to 6-branch.
>
> Cheers
>
>
> Paul
>
> On 11 February 2017 at 13:24, Paul Richard Thomas
>  wrote:
>> Ping!
>>
>> On 8 February 2017 at 16:00, Paul Richard Thomas
>>  wrote:
>>> Dear All,
>>>
>>> The attached rework of the patch functions in the same way as
>>> yesterday's but is based in resolve.c rather than trans-decl.c. It
>>> looks to me to be by far cleaner.
>>>
>>> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
>>>
>>> Cheers
>>>
>>> Paul
>>>
>>> 2017-02-08  Paul Thomas  
>>>
>>> PR fortran/79344
>>> * resolve.c (fixup_unique_dummy): New function.
>>> (gfc_resolve_expr): Call it for dummy variables with a unique
>>> symtree name.
>>>
>>> 2017-02-08  Paul Thomas  
>>>
>>> PR fortran/79344
>>> * gfortran.dg/submodule_23.f90: New test.
>>>
>>>
>>>
>>> On 7 February 2017 at 16:06, Paul Richard Thomas
>>>  wrote:
 Dear All,

 This bug generates an ICE because the symbol for dummy 'n' in the
 specification expression for the result of 'fun1' is not the same as
 the symbol in the formal arglist. For some reason that I have been
 unable to uncover, this false dummy is associated with a unique
 symtree. The odd thing is that the dump of the parse tree for the
 failing module procedure case is identical to that where the interface
 is explcitely reproduced in the submodule. The cause of the ICE is
 that the false dummy has no backend_decl as it should.

 This patch hits the problem directly on the head by using the
 backend_decl from the symbol in the namespace of the formal arglist,
 as described in the comment in the patch. If it is deemed to be more
 hygenic, the chunk of code can be lifted out and deposited in a
 separate function.

 Bootstraps and regtests on FC23/x86_64 - OK for trunk?

 Cheers

 Paul

 2017-02-07  Paul Thomas  

 PR fortran/79344
 * trans-decl.c (gfc_get_symbol_decl): If a dummy apparently has
 a null backend_decl, look for a replacement symbol in the
 namespace of the 1st formal argument and use its backend_decl.

 2017-02-07  Paul Thomas  

 PR fortran/79344
 * gfortran.dg/submodule_23.f90: New test.
>>>
>>>
>>>
>>> --
>>> "If you can't explain it simply, you don't understand it well enough"
>>> - Albert Einstein
>>
>>
>>
>> --
>> "If you can't explain it simply, you don't understand it well enough"
>> - Albert Einstein
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein

Re: Ping [Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Paul Richard Thomas]

Hi Everybody,

With Jerry's OK for a commit to trunk, committed as revision 245580.

I will wait a few weeks before a commit to 6-branch.

Cheers


Paul

On 11 February 2017 at 13:24, Paul Richard Thomas
 wrote:
> Ping!
>
> On 8 February 2017 at 16:00, Paul Richard Thomas
>  wrote:
>> Dear All,
>>
>> The attached rework of the patch functions in the same way as
>> yesterday's but is based in resolve.c rather than trans-decl.c. It
>> looks to me to be by far cleaner.
>>
>> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
>>
>> Cheers
>>
>> Paul
>>
>> 2017-02-08  Paul Thomas  
>>
>> PR fortran/79344
>> * resolve.c (fixup_unique_dummy): New function.
>> (gfc_resolve_expr): Call it for dummy variables with a unique
>> symtree name.
>>
>> 2017-02-08  Paul Thomas  
>>
>> PR fortran/79344
>> * gfortran.dg/submodule_23.f90: New test.
>>
>>
>>
>> On 7 February 2017 at 16:06, Paul Richard Thomas
>>  wrote:
>>> Dear All,
>>>
>>> This bug generates an ICE because the symbol for dummy 'n' in the
>>> specification expression for the result of 'fun1' is not the same as
>>> the symbol in the formal arglist. For some reason that I have been
>>> unable to uncover, this false dummy is associated with a unique
>>> symtree. The odd thing is that the dump of the parse tree for the
>>> failing module procedure case is identical to that where the interface
>>> is explcitely reproduced in the submodule. The cause of the ICE is
>>> that the false dummy has no backend_decl as it should.
>>>
>>> This patch hits the problem directly on the head by using the
>>> backend_decl from the symbol in the namespace of the formal arglist,
>>> as described in the comment in the patch. If it is deemed to be more
>>> hygenic, the chunk of code can be lifted out and deposited in a
>>> separate function.
>>>
>>> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
>>>
>>> Cheers
>>>
>>> Paul
>>>
>>> 2017-02-07  Paul Thomas  
>>>
>>> PR fortran/79344
>>> * trans-decl.c (gfc_get_symbol_decl): If a dummy apparently has
>>> a null backend_decl, look for a replacement symbol in the
>>> namespace of the 1st formal argument and use its backend_decl.
>>>
>>> 2017-02-07  Paul Thomas  
>>>
>>> PR fortran/79344
>>> * gfortran.dg/submodule_23.f90: New test.
>>
>>
>>
>> --
>> "If you can't explain it simply, you don't understand it well enough"
>> - Albert Einstein
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Jerry DeLisle]

On 02/08/2017 08:00 AM, Paul Richard Thomas wrote:

Dear All,

The attached rework of the patch functions in the same way as
yesterday's but is based in resolve.c rather than trans-decl.c. It
looks to me to be by far cleaner.

Bootstraps and regtests on FC23/x86_64 - OK for trunk?

Cheers

Paul

2017-02-08  Paul Thomas  

PR fortran/79344
* resolve.c (fixup_unique_dummy): New function.
(gfc_resolve_expr): Call it for dummy variables with a unique
symtree name.

2017-02-08  Paul Thomas  

PR fortran/79344
* gfortran.dg/submodule_23.f90: New test.



On 7 February 2017 at 16:06, Paul Richard Thomas
 wrote:

Dear All,

This bug generates an ICE because the symbol for dummy 'n' in the
specification expression for the result of 'fun1' is not the same as
the symbol in the formal arglist. For some reason that I have been
unable to uncover, this false dummy is associated with a unique
symtree. The odd thing is that the dump of the parse tree for the
failing module procedure case is identical to that where the interface
is explcitely reproduced in the submodule. The cause of the ICE is
that the false dummy has no backend_decl as it should.

This patch hits the problem directly on the head by using the
backend_decl from the symbol in the namespace of the formal arglist,
as described in the comment in the patch. If it is deemed to be more
hygenic, the chunk of code can be lifted out and deposited in a
separate function.

Bootstraps and regtests on FC23/x86_64 - OK for trunk?


Yes OK.


Jerry

Re: [Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Steve Kargl]

On Wed, Feb 08, 2017 at 04:00:35PM +, Paul Richard Thomas wrote:
> 
> The attached rework of the patch functions in the same way as
> yesterday's but is based in resolve.c rather than trans-decl.c. It
> looks to me to be by far cleaner.
> 
> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
> 

OK.

__ 
Steve
20161221 https://www.youtube.com/watch?v=IbCHE-hONow

Ping [Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Paul Richard Thomas]

Ping!

On 8 February 2017 at 16:00, Paul Richard Thomas
 wrote:
> Dear All,
>
> The attached rework of the patch functions in the same way as
> yesterday's but is based in resolve.c rather than trans-decl.c. It
> looks to me to be by far cleaner.
>
> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
>
> Cheers
>
> Paul
>
> 2017-02-08  Paul Thomas  
>
> PR fortran/79344
> * resolve.c (fixup_unique_dummy): New function.
> (gfc_resolve_expr): Call it for dummy variables with a unique
> symtree name.
>
> 2017-02-08  Paul Thomas  
>
> PR fortran/79344
> * gfortran.dg/submodule_23.f90: New test.
>
>
>
> On 7 February 2017 at 16:06, Paul Richard Thomas
>  wrote:
>> Dear All,
>>
>> This bug generates an ICE because the symbol for dummy 'n' in the
>> specification expression for the result of 'fun1' is not the same as
>> the symbol in the formal arglist. For some reason that I have been
>> unable to uncover, this false dummy is associated with a unique
>> symtree. The odd thing is that the dump of the parse tree for the
>> failing module procedure case is identical to that where the interface
>> is explcitely reproduced in the submodule. The cause of the ICE is
>> that the false dummy has no backend_decl as it should.
>>
>> This patch hits the problem directly on the head by using the
>> backend_decl from the symbol in the namespace of the formal arglist,
>> as described in the comment in the patch. If it is deemed to be more
>> hygenic, the chunk of code can be lifted out and deposited in a
>> separate function.
>>
>> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
>>
>> Cheers
>>
>> Paul
>>
>> 2017-02-07  Paul Thomas  
>>
>> PR fortran/79344
>> * trans-decl.c (gfc_get_symbol_decl): If a dummy apparently has
>> a null backend_decl, look for a replacement symbol in the
>> namespace of the 1st formal argument and use its backend_decl.
>>
>> 2017-02-07  Paul Thomas  
>>
>> PR fortran/79344
>> * gfortran.dg/submodule_23.f90: New test.
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Paul Richard Thomas]

Dear All,

The attached rework of the patch functions in the same way as
yesterday's but is based in resolve.c rather than trans-decl.c. It
looks to me to be by far cleaner.

Bootstraps and regtests on FC23/x86_64 - OK for trunk?

Cheers

Paul

2017-02-08  Paul Thomas  

PR fortran/79344
* resolve.c (fixup_unique_dummy): New function.
(gfc_resolve_expr): Call it for dummy variables with a unique
symtree name.

2017-02-08  Paul Thomas  

PR fortran/79344
* gfortran.dg/submodule_23.f90: New test.



On 7 February 2017 at 16:06, Paul Richard Thomas
 wrote:
> Dear All,
>
> This bug generates an ICE because the symbol for dummy 'n' in the
> specification expression for the result of 'fun1' is not the same as
> the symbol in the formal arglist. For some reason that I have been
> unable to uncover, this false dummy is associated with a unique
> symtree. The odd thing is that the dump of the parse tree for the
> failing module procedure case is identical to that where the interface
> is explcitely reproduced in the submodule. The cause of the ICE is
> that the false dummy has no backend_decl as it should.
>
> This patch hits the problem directly on the head by using the
> backend_decl from the symbol in the namespace of the formal arglist,
> as described in the comment in the patch. If it is deemed to be more
> hygenic, the chunk of code can be lifted out and deposited in a
> separate function.
>
> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
>
> Cheers
>
> Paul
>
> 2017-02-07  Paul Thomas  
>
> PR fortran/79344
> * trans-decl.c (gfc_get_symbol_decl): If a dummy apparently has
> a null backend_decl, look for a replacement symbol in the
> namespace of the 1st formal argument and use its backend_decl.
>
> 2017-02-07  Paul Thomas  
>
> PR fortran/79344
> * gfortran.dg/submodule_23.f90: New test.



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Index: gcc/fortran/resolve.c
===
*** gcc/fortran/resolve.c   (revision 245196)
--- gcc/fortran/resolve.c   (working copy)
*** gfc_is_expandable_expr (gfc_expr *e)
*** 6433,6438 
--- 6433,6463 
return false;
  }
  
+ 
+ /* Sometimes variables in specification expressions of the result
+of module procedures in submodules wind up not being the 'real'
+dummy.  Find this, if possible, in the namespace of the first
+formal argument.  */
+ 
+ static void
+ fixup_unique_dummy (gfc_expr *e)
+ {
+   gfc_symtree *st = NULL;
+   gfc_symbol *s = NULL;
+ 
+   if (e->symtree->n.sym->ns->proc_name
+   && e->symtree->n.sym->ns->proc_name->formal)
+ s = e->symtree->n.sym->ns->proc_name->formal->sym;
+ 
+   if (s != NULL)
+ st = gfc_find_symtree (s->ns->sym_root, e->symtree->n.sym->name);
+ 
+   if (st != NULL
+   && st->n.sym != NULL
+   && st->n.sym->attr.dummy)
+ e->symtree = st;
+ }
+ 
  /* Resolve an expression.  That is, make sure that types of operands agree
 with their operators, intrinsic operators are converted to function calls
 for overloaded types and unresolved function references are resolved.  */
*** gfc_resolve_expr (gfc_expr *e)
*** 6457,6462 
--- 6482,6495 
actual_arg = false;
first_actual_arg = false;
  }
+   else if (e->symtree != NULL
+  && *e->symtree->name == '@'
+  && e->symtree->n.sym->attr.dummy)
+ {
+   /* Deal with submodule specification expressions that are not
+found to be referenced in module.c(read_cleanup).  */
+   fixup_unique_dummy (e);
+ }
  
switch (e->expr_type)
  {
Index: gcc/testsuite/gfortran.dg/submodule_23.f90
===
*** gcc/testsuite/gfortran.dg/submodule_23.f90  (nonexistent)
--- gcc/testsuite/gfortran.dg/submodule_23.f90  (working copy)
***
*** 0 
--- 1,29 
+ ! { dg-do compile }
+ !
+ ! Test the fix for PR79402, in which the module procedure 'fun1' picked
+ ! up a spurious symbol for the dummy 'n' in the specification expression
+ ! for the result 'y'.
+ !
+ ! Contributed by Chris Coutinho  
+ !
+ module mod
+   interface myfun
+ module function fun1(n) result(y)
+   integer,  intent(in):: n
+   real, dimension(n)  :: y
+ end function fun1
+   end interface myfun
+ 
+ end module mod
+ 
+ submodule (mod) submod
+ contains
+   module procedure fun1
+ integer :: i
+ y = [(float (i), i = 1, n)]
+   end procedure fun1
+ end submodule
+ 
+   use mod
+   print *, fun1(10)
+ end

Re: [Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Chris Coutinho]

Thanks for the quick action Paul - I hope the fix goes through

A small correction in the example problem though: there is no begin
program statement in your snippet. It still fails, as is, due to the
ICE, but I think if you make the fix this example program will still
fail to compile. A simple `program main` before `use mod` should do
the trick.

Regards,
Chris


On Tue, Feb 7, 2017 at 5:06 PM, Paul Richard Thomas
 wrote:
> Dear All,
>
> This bug generates an ICE because the symbol for dummy 'n' in the
> specification expression for the result of 'fun1' is not the same as
> the symbol in the formal arglist. For some reason that I have been
> unable to uncover, this false dummy is associated with a unique
> symtree. The odd thing is that the dump of the parse tree for the
> failing module procedure case is identical to that where the interface
> is explcitely reproduced in the submodule. The cause of the ICE is
> that the false dummy has no backend_decl as it should.
>
> This patch hits the problem directly on the head by using the
> backend_decl from the symbol in the namespace of the formal arglist,
> as described in the comment in the patch. If it is deemed to be more
> hygenic, the chunk of code can be lifted out and deposited in a
> separate function.
>
> Bootstraps and regtests on FC23/x86_64 - OK for trunk?
>
> Cheers
>
> Paul
>
> 2017-02-07  Paul Thomas  
>
> PR fortran/79344
> * trans-decl.c (gfc_get_symbol_decl): If a dummy apparently has
> a null backend_decl, look for a replacement symbol in the
> namespace of the 1st formal argument and use its backend_decl.
>
> 2017-02-07  Paul Thomas  
>
> PR fortran/79344
> * gfortran.dg/submodule_23.f90: New test.

[Patch, fortran] PR79402 - ICE with submodules: module procedure interface defined in parent module

[Paul Richard Thomas]

Dear All,

This bug generates an ICE because the symbol for dummy 'n' in the
specification expression for the result of 'fun1' is not the same as
the symbol in the formal arglist. For some reason that I have been
unable to uncover, this false dummy is associated with a unique
symtree. The odd thing is that the dump of the parse tree for the
failing module procedure case is identical to that where the interface
is explcitely reproduced in the submodule. The cause of the ICE is
that the false dummy has no backend_decl as it should.

This patch hits the problem directly on the head by using the
backend_decl from the symbol in the namespace of the formal arglist,
as described in the comment in the patch. If it is deemed to be more
hygenic, the chunk of code can be lifted out and deposited in a
separate function.

Bootstraps and regtests on FC23/x86_64 - OK for trunk?

Cheers

Paul

2017-02-07  Paul Thomas  

PR fortran/79344
* trans-decl.c (gfc_get_symbol_decl): If a dummy apparently has
a null backend_decl, look for a replacement symbol in the
namespace of the 1st formal argument and use its backend_decl.

2017-02-07  Paul Thomas  

PR fortran/79344
* gfortran.dg/submodule_23.f90: New test.
Index: gcc/fortran/trans-decl.c
===
*** gcc/fortran/trans-decl.c(revision 245196)
--- gcc/fortran/trans-decl.c(working copy)
*** gfc_get_symbol_decl (gfc_symbol * sym)
*** 1510,1516 
sym->backend_decl = DECL_CHAIN (sym->backend_decl);
}
  
!   /* Dummy variables should already have been created.  */
gcc_assert (sym->backend_decl);
  
/* Create a character length variable.  */
--- 1510,1538 
sym->backend_decl = DECL_CHAIN (sym->backend_decl);
}
  
!   /* Dummy variables should already have been created.  However,
!sometimes variables in specification expressions of the result
!of module procedures in submodules wind up not being the 'real'
!dummy.  Find this, if possible, in the namespace of the first
!formal argument.  */
!   if (sym->backend_decl == NULL_TREE)
!   {
! gfc_symtree *st = NULL;
! gfc_symbol *s = NULL;
! 
! if (sym->ns->proc_name
! && sym->ns->proc_name->formal)
!   s = sym->ns->proc_name->formal->sym;
! 
! if (s != NULL)
!   st = gfc_find_symtree (s->ns->sym_root, sym->name);
! 
! if (st != NULL
! && st->n.sym != NULL
! && st->n.sym->attr.dummy)
!   sym->backend_decl = st->n.sym->backend_decl;
!   }
! 
gcc_assert (sym->backend_decl);
  
/* Create a character length variable.  */
Index: gcc/testsuite/gfortran.dg/submodule_23.f90
===
*** gcc/testsuite/gfortran.dg/submodule_23.f90  (nonexistent)
--- gcc/testsuite/gfortran.dg/submodule_23.f90  (working copy)
***
*** 0 
--- 1,29 
+ ! { dg-do compile }
+ !
+ ! Test the fix for PR79402, in which the module procedure 'fun1' picked
+ ! up a spurious symbol for the dummy 'n' in the specification expression
+ ! for the result 'y'.
+ !
+ ! Contributed by Chris Coutinho  
+ !
+ module mod
+   interface myfun
+ module function fun1(n) result(y)
+   integer,  intent(in):: n
+   real, dimension(n)  :: y
+ end function fun1
+   end interface myfun
+ 
+ end module mod
+ 
+ submodule (mod) submod
+ contains
+   module procedure fun1
+ integer :: i
+ y = [(float (i), i = 1, n)]
+   end procedure fun1
+ end submodule
+ 
+   use mod
+   print *, fun1(10)
+ end