Bug 46849 - [OOP] MODULE PROCEDURE resolution does not work in BLOCK or SELECT TYPE
[OOP] MODULE PROCEDURE resolution does not work in BLOCK or SELECT TYPE
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: fortran
4.6.0
: P3 normal
: ---
Assigned To: janus
: ice-on-invalid-code, rejects-valid
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-08 13:07 UTC by Tobias Burnus
Modified: 2011-02-14 10:29 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.5.1, 4.6.0
Last reconfirmed: 2010-12-08 13:48:45


Attachments
Test case (1.19 KB, text/plain)
2010-12-08 13:08 UTC, Tobias Burnus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2010-12-08 13:07:25 UTC
The following problem has been reported by Reinhold Bader.

    select type (p => fun%options)
    type is (qdr_opt_qag)
       p%f = fgsl_function_init(fun_qag)
    end select

Is rejected with a current GCC 4.6 with:

pmr.f90:35.39:
       p%f = fgsl_function_init(fun_qag)
                                       1
Error: Symbol 'fun_qag' at (1) has no IMPLICIT type
f951: internal compiler error: in gfc_enforce_clean_symbol_state, at fortran/symbol.c:3472


Here, "fun_qag" is a module procedure. It works if one flips the order of "fun_qag" and the calling procedure. It also works if one uses the line:

       fun%options%f = fgsl_function_init(fun_qag)

as long as the line is neither in a BLOCK nor in a SELECT TYPE. Seemingly, resolve_symbol does not search in the module name space!
Comment 1 Tobias Burnus 2010-12-08 13:08:43 UTC
Created attachment 22685 [details]
Test case
Comment 2 Tobias Burnus 2010-12-08 13:30:35 UTC
For the reject-valid part: See comment 0.

Reduced test case for the error-recovery ICE:

module m
  implicit none
  type :: dt
  end type
contains
  subroutine test(fun)
    class(dt) :: fun
    call extern(not_existing) ! Error: No implicit type
    select type (fun)
    type is (dt)          ! TYPE IS is required for the ICE
    end select
  end subroutine test
end module m
Comment 3 janus 2010-12-08 13:48:45 UTC
Here is a reduced test case for the rejects-valid part, without CLASS and ISO_C_BINDING:


  implicit none

  block
    call init(fun)
  end block

contains

  subroutine init(func)
    real, external :: func
  end subroutine

  real function fun()
    fun = 1.1
  end function

end
Comment 4 janus 2010-12-13 21:59:21 UTC
(In reply to comment #3)
> Here is a reduced test case for the rejects-valid part, without CLASS and
> ISO_C_BINDING:


Not sure if it's the perfectly right thing to do, but the following patch fixes the test cases in comment #3 and #1:

Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c        (revision 167765)
+++ gcc/fortran/symbol.c        (working copy)
@@ -2717,7 +2717,7 @@ gfc_get_sym_tree (const char *name, gfc_namespace
 
   /* This doesn't usually happen during resolution.  */
   if (ns == NULL)
-    ns = gfc_current_ns;
+    ns = gfc_find_proc_namespace (gfc_current_ns);
 
   /* Try to find the symbol in ns.  */
   st = gfc_find_symtree (ns->sym_root, name);
Comment 5 Tobias Burnus 2010-12-14 08:36:07 UTC
(In reply to comment #4)
> @@ -2717,7 +2717,7 @@ gfc_get_sym_tree (const char *name, gfc_namespace
>    if (ns == NULL)
> -    ns = gfc_current_ns;
> +    ns = gfc_find_proc_namespace (gfc_current_ns);

That looks wrong. gfc_get_sym_tree is called several times in decl.c, match.c, parse.c and primary.c with NULL - I fear that this patch will cause symbols end up in the wrong name space.

I think one should rather do something like the following - though one might need to be a bit more careful and restrict it, e.g., to procedures.

--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -11784,7 +11784,7 @@ resolve_symbol (gfc_symbol *sym)
       for (ns = gfc_current_ns->parent; ns; ns = ns->parent)
        {
          symtree = gfc_find_symtree (ns->sym_root, sym->name);
-         if (symtree && symtree->n.sym->generic)
+         if (symtree) /* && symtree->n.sym->generic)*/
            {
              this_symtree = gfc_find_symtree (gfc_current_ns->sym_root,
                                               sym->name);
Comment 6 janus 2010-12-14 12:13:04 UTC
(In reply to comment #5)
> > @@ -2717,7 +2717,7 @@ gfc_get_sym_tree (const char *name, gfc_namespace
> >    if (ns == NULL)
> > -    ns = gfc_current_ns;
> > +    ns = gfc_find_proc_namespace (gfc_current_ns);
> 
> That looks wrong. gfc_get_sym_tree is called several times in decl.c, match.c,
> parse.c and primary.c with NULL - I fear that this patch will cause symbols end
> up in the wrong name space.

Yes, you're right. It produces a fair number of regressions ...


> I think one should rather do something like the following - though one might
> need to be a bit more careful and restrict it, e.g., to procedures.
> 
> --- a/gcc/fortran/resolve.c
> +++ b/gcc/fortran/resolve.c
> @@ -11784,7 +11784,7 @@ resolve_symbol (gfc_symbol *sym)
>        for (ns = gfc_current_ns->parent; ns; ns = ns->parent)
>         {
>           symtree = gfc_find_symtree (ns->sym_root, sym->name);
> -         if (symtree && symtree->n.sym->generic)
> +         if (symtree) /* && symtree->n.sym->generic)*/
>             {
>               this_symtree = gfc_find_symtree (gfc_current_ns->sym_root,
>                                                sym->name);


I agree that this is better, though it still causes some regressions. I will now test the following variant:

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c       (revision 167790)
+++ gcc/fortran/resolve.c       (working copy)
@@ -11784,7 +11784,7 @@ resolve_symbol (gfc_symbol *sym)
       for (ns = gfc_current_ns->parent; ns; ns = ns->parent)
        {
          symtree = gfc_find_symtree (ns->sym_root, sym->name);
-         if (symtree && symtree->n.sym->generic)
+         if (symtree && symtree->n.sym->attr.flavor == FL_PROCEDURE)
            {
              this_symtree = gfc_find_symtree (gfc_current_ns->sym_root,
                                               sym->name);
Comment 7 janus 2010-12-14 18:30:48 UTC
(In reply to comment #6)
> I agree that this is better, though it still causes some regressions. I will
> now test the following variant:
> 
> Index: gcc/fortran/resolve.c
> ===================================================================
> --- gcc/fortran/resolve.c       (revision 167790)
> +++ gcc/fortran/resolve.c       (working copy)
> @@ -11784,7 +11784,7 @@ resolve_symbol (gfc_symbol *sym)
>        for (ns = gfc_current_ns->parent; ns; ns = ns->parent)
>         {
>           symtree = gfc_find_symtree (ns->sym_root, sym->name);
> -         if (symtree && symtree->n.sym->generic)
> +         if (symtree && symtree->n.sym->attr.flavor == FL_PROCEDURE)
>             {
>               this_symtree = gfc_find_symtree (gfc_current_ns->sym_root,
>                                                sym->name);


Unfortunately this also triggers a few regressions, but the following regtests cleanly:

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c       (revision 167790)
+++ gcc/fortran/resolve.c       (working copy)
@@ -11784,7 +11784,9 @@ resolve_symbol (gfc_symbol *sym)
       for (ns = gfc_current_ns->parent; ns; ns = ns->parent)
        {
          symtree = gfc_find_symtree (ns->sym_root, sym->name);
-         if (symtree && symtree->n.sym->generic)
+         if (symtree && (symtree->n.sym->generic ||
+                         (symtree->n.sym->attr.flavor == FL_PROCEDURE
+                          && sym->ns->construct_entities)))
            {
              this_symtree = gfc_find_symtree (gfc_current_ns->sym_root,
                                               sym->name);
Comment 8 janus 2010-12-17 12:31:57 UTC
Author: janus
Date: Fri Dec 17 12:31:54 2010
New Revision: 167978

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167978
Log:
2010-12-17  Janus Weil  <janus@gcc.gnu.org>
	    Tobias Burnus <burnus@gcc.gnu.org>

	PR fortran/46849
	* resolve.c (resolve_symbol): Remove symbols that wrongly ended up
	in a local BLOCK namespace.

2010-12-17  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/46849
	* gfortran.dg/block_9.f08: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/block_9.f08
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 janus 2010-12-17 12:34:11 UTC
r167978 fixes the rejects-valid part, i.e. comment #3.

ToDo: ICE-on-invalid in comment #2.
Comment 10 janus 2011-02-14 10:29:20 UTC
(In reply to comment #9)
> ToDo: ICE-on-invalid in comment #2.

This is now being tracked in PR 47730. Closing this one.