Bug 33541 - gfortran wrongly imports renamed-use-associated symbol unrenamed
Summary: gfortran wrongly imports renamed-use-associated symbol unrenamed
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Paul Thomas
URL:
Keywords: wrong-code
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2007-09-24 08:44 UTC by Tobias Burnus
Modified: 2007-11-27 20:50 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-10-02 08:51:52


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2007-09-24 08:44:59 UTC
Reported by John Harper,
http://gcc.gnu.org/ml/fortran/2007-09/msg00397.html

The following
  use module
  use module, only: xrenamed => x
surely makes xrenamed available; however, -- see clause (3) below -- it does not make 'x' available. In gfortran the symbol 'x' is also imported into the namespace.

The Fortran 2003 standard contains (11.2.1):

    More than one USE statement for a given module may appear in
    a scoping unit.  If one of the USE statements is without an
    ONLY qualifier, all public entities in the module are accessible.
    If all the USE statements have ONLY qualifiers, only those
    entities named in one or more of the only-lists are accessible.

however, this is followed by the following restrictions:

    An accessible entity in the referenced module has one or more
    local identifiers. These identifiers are
    (1) The identifier of the entity in the referenced module if
        that identifier appears as an only use-name or as the
        defined-operator of a generic-spec in any only for that module,
    (2) Each of the local-names or local-defined-operators that the
        entity is given in any rename for that module, and
    (3) The identifier of the entity in the referenced module if
        that identifier does not appear as a use-name or
        use-defined-operator in any rename for that module.

For 'x' case 3 applies (or better it does not apply) and thus 'x' shall not be imported.

Example by John Harper (ifort, sunf95, NAG f95 and openf95 properly print .FALSE., gfortran prints .TRUE.):

MODULE xmod
  REAL(kind(1d0)) :: x = -666
END MODULE xmod

PROGRAM test2uses
  USE xmod
  USE xmod, ONLY: xrenamed => x ! kind(1d0)
  x = 666                       ! kind(1.0): see below
  PRINT *,'kind(xrenamed)==kind(x)?',kind(xrenamed)==kind(x)
  PRINT *,'That should have printed',.FALSE.
END PROGRAM test2uses
Comment 1 Tobias Burnus 2007-09-24 08:52:57 UTC
Post script: Fortran 95 contains the same in 11.3.2; the only difference is that Fortran 2003 talks also about defined-operators.
Comment 2 Paul Thomas 2007-10-02 08:51:51 UTC
confirmed - Paul
Comment 3 Paul Thomas 2007-11-15 09:46:51 UTC
The patch below fixes the reported bug.  I am going to check to see what needs to be done to extend this to generic interfaces and operators.

Paul

Index: gcc/fortran/module.c
===================================================================
*** gcc/fortran/module.c        (révision 130174)
--- gcc/fortran/module.c        (copie de travail)
*************** find_symtree_for_symbol (gfc_symtree *st
*** 3492,3497 ****
--- 3492,3522 ----
  }


+ /* A recursive function to look for a spefici symbol by name and by
+    module.  Whilst several symtrees might point to one symbol, its
+    is sufficient for the purposes here than one exist.  */
+ static gfc_symtree *
+ find_symbol (gfc_symtree *st, const char *name, const char *module)
+ {
+   int c;
+   gfc_symtree *retval;
+
+   if (st == NULL || st->n.sym == NULL)
+     return NULL;
+
+   c = strcmp (name, st->n.sym->name);
+   if (c == 0 && strcmp (module, st->n.sym->module) == 0)
+     return st;
+
+   retval = find_symbol (st->left, name, module);
+
+   if (retval == NULL)
+     retval = find_symbol (st->right, name, module);
+
+   return retval;
+ }
+
+
  /* Read a module file.  */

  static void
*************** read_module (void)
*** 3616,3621 ****
--- 3641,3655 ----
              continue;
            }

+         /* If a symbol of the same name and module exists already,
+            this symbol, which is not in an ONLY clause, must not be
+            added to the namespace(11.3.2).  Note that find_symbol
+            only returns the first occurrence that it finds.  */
+         if (p == name && strcmp (name, module_name) != 0
+               && find_symbol (gfc_current_ns->sym_root, name,
+                               module_name))
+           continue;
+
          st = gfc_find_symtree (gfc_current_ns->sym_root, p);

          if (st != NULL)
*************** read_module (void)
*** 3627,3632 ****
--- 3661,3674 ----
            }
          else
            {
+             st = gfc_find_symtree (gfc_current_ns->sym_root, name);
+
+             /* Make symtree inaccessible by renaming if the symbol has
+                been added by a USE statement without an ONLY(11.3.2).  */
+             if (st && st->name == st->n.sym->name
+                    && strcmp (st->n.sym->module, module_name) == 0)
+               st->name = gfc_get_string ("hidden.%s", name);
+
              /* Create a symtree node in the current namespace for this
                 symbol.  */
              st = check_unique_name (p)
Index: gcc/testsuite/gfortran.dg/use_only_1.f90
===================================================================
*** gcc/testsuite/gfortran.dg/use_only_1.f90    (révision 0)
--- gcc/testsuite/gfortran.dg/use_only_1.f90    (révision 0)
***************
*** 0 ****
--- 1,31 ----
+ ! { dg-do run }
+ ! { dg-options "-O1" }
+ ! Checks the fix for PR33541, in which a requirement
+ ! of F95 11.3.2 was not being met: The local names
+ ! 'x' and 'y' coming from the USE statements without
+ ! an ONLY clause should not survive in the presence
+ ! of the locally renamed versions.
+ !
+ ! Reported by Reported by John Harper in
+ ! http://gcc.gnu.org/ml/fortran/2007-09/msg00397.html
+ !
+ MODULE xmod
+   integer(4) :: x = -666
+ END MODULE xmod
+
+ MODULE ymod
+   integer(4) :: y = -666
+ END MODULE ymod
+
+ PROGRAM test2uses
+   USE xmod
+   USE xmod, ONLY: xrenamed => x
+   USE ymod, ONLY: yrenamed => y
+   USE ymod
+   implicit integer(2) (a-z)
+   x = 666  ! These assignments should generate implicitly typed
+   y = 666  ! local variable 'x' and 'y'.
+   if (kind(xrenamed) == kind(x)) call abort ()
+   if (kind(yrenamed) == kind(y)) call abort ()
+ END PROGRAM test2uses
+ ! { dg-final { cleanup-modules "xmod ymod" } }
Comment 4 Paul Thomas 2007-11-15 11:16:02 UTC
(In reply to comment #3)

Bother, the patch causes some regressions (interface_[3-5].f90)...

Paul
Comment 5 Paul Thomas 2007-11-15 15:19:31 UTC
(In reply to comment #4)
> (In reply to comment #3)
> Bother, the patch causes some regressions (interface_[3-5].f90)...
> Paul

These were easily fixed - also nested_modules_1.f90 was not standard compliant in this regard.  It now regtests with find_symbol modified to:

  c = strcmp (name, st->n.sym->name);
  if (c == 0 && st->n.sym->module
	&& strcmp (module, st->n.sym->module) == 0)
    return st;

Off to generics and operators now!

Paul
Comment 6 Paul Thomas 2007-11-19 15:10:37 UTC
(In reply to comment #5)
> Off to generics and operators now!

Ahhh... I have run into a serious problem here.  It transpires that renaming is not accomplished for generic interfaces by keeping the use-name symbol with multiple symtrees, each with a local-name.  Instead, the symtree and the symbol have the local-name.  This breaks the mechanism that I evolved above.

The DEC Manual puts the rules rather nicely:

If more than one USE statement for a given module appears in a scoping unit, the following rules apply: 

If one USE statement does not have the ONLY option, all public entities in the module are accessible, and any rename- lists and only-lists are interpreted as a single, concatenated rename-list. 

If all the USE statements have ONLY options, all the only-lists are interpreted as a single, concatenated only-list. Only those entities named in one or more of the only-lists are accessible.
 
This, I think, shows the way forward.  ie. In matching the USE statements, we only build up the rename-list and flag the presence of a USE statement without an ONLY. Then, at the last USE statement, we process all the referenced .mod files just once, using the concatenated rename-list as an only-list or a rename list, according to what we have seen.

Paul

Comment 7 patchapp@dberlin.org 2007-11-22 19:03:02 UTC
Subject: Bug number PR33541

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01166.html
Comment 8 Paul Thomas 2007-11-24 10:17:35 UTC
Subject: Bug 33541

Author: pault
Date: Sat Nov 24 10:17:26 2007
New Revision: 130395

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=130395
Log:
2007-11-24  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33541
	* module.c (find_symtree_for_symbol): Move to new location.
	(find_symbol): New function.
	(load_generic_interfaces): Rework completely so that symtrees
	have the local name and symbols have the use name.  Renamed
	generic interfaces exclude the use of the interface without an
	ONLY clause (11.3.2).
	(read_module): Implement 11.3.2 in the same way as for generic
	interfaces.

2007-11-24  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33541
	* gfortran.dg/nested_modules_1.f90: Change the reference to
	FOO, forbidden by the standard, to a reference to W.
	* gfortran.dg/use_only_1.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/use_only_1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/module.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/nested_modules_1.f90

Comment 9 Dominique d'Humieres 2007-11-24 11:04:43 UTC
The following test now fails:

! { dg-do run }
! A test to prove that, a sub-routine, variables from the module takes 
! precedence in case of name conflict with local entities.
! Reference Fortran 95 standard document section 11.3.2
        module modone
          real :: var1 = 11.
          contains
               subroutine set_real(a)
                  real, intent(in)   :: a
                  var1 = a
               end subroutine set_real
        end module modone

        program module_use_test
          integer :: failures=0
          real :: var1=5.
          call test1
          print *, failures
          call test2
          print *, failures
          call test3
          print *, failures
          call check_results

          contains
! A local sub-routine, with similar name from the module
             subroutine set_real(a)
                real :: a
             end subroutine set_real

             subroutine check_local_val(a)
                 real :: a
                 if ( var1 .eq. a ) then
                   failures = failures + 1
                 end if 
             end subroutine check_local_val

             subroutine test1
                 USE modone
                 call set_real(10.)
                 if ( var1 .ne. 10. ) then
                   failures = failures + 1
                 end if 
             end subroutine test1

! When used rename option as local_name => global_name, a global variable gets referred by 
! the new name only,  and local_name takes precedence in case of name conflict as in 
! this situation.
             subroutine test2
                 USE modone, global_var_ref1 => var1
                 USE modone, global_var_ref2 => var1

!  This should refer to the global variable 'var1' in the module scope
                 global_var_ref1 = 13        ! some value

!  This should refer to the local variable 'var1' in the scope of 'program'
                 var1 = 7                    ! some value
                 call check_local_val(global_var_ref1)

                 if ( global_var_ref1 .ne. global_var_ref2 ) then
                   failures = failures + 1
                 end if 

             end subroutine test2

! scoping tests for the local and global variables
             subroutine test3
                 USE modone, global_var => var1
                 USE modone

!  This should refer to the global variable var1' in the module
                 global_var = 13.

!  This should refer to the global variable 'var1' in the module
                 var1 = var1 + 0
                 if ( var1 .ne. global_var ) then
                   failures = failures + 1
                 end if 

!  This should refer to the local variable in the scope of 'program code'
                 call check_local_val(var1)

             end subroutine test3

             subroutine check_results
                 if ( failures .ne. 0 ) call abort()
             end subroutine check_results

        end program module_use_test

[ibook-dhum] f90/bug% a.out
           0
           1
           3
Abort

Comment 10 Tobias Burnus 2007-11-24 12:09:16 UTC
(In reply to comment #9)
> The following test now fails:

That test passes (0 0 0) with gfortran (w/o patch) and g95.
With ifort, NAG f95, openf95 and sunf95 it fails with "0 0 2".
And with the patched gfortran it fails with "0 1 3".

The following is definitely a bug:

module m
  integer :: a
end module m

use m, local1 => a
use m, local2 => a
local1 = 5
local2 = 3
print *, local1, local2 ! prints "5 3" instead of "3 3"
end

Dump:
  extern integer(kind=4) a;
  integer(kind=4) local2;
  a = 5;
  local2 = 3;

Comment 11 Tobias Burnus 2007-11-24 12:20:17 UTC
I think the failures in test3 are ok. Example program:

module m
  integer :: a = 10
end module m
program p
 integer :: a
 a = 5
 call s()
contains
  subroutine s()
    use m, local1 => a
    use m
    print *, local1, a
  end subroutine
end program p

This prints "10 10" with g95 and "10 5" with the patched gfortran, NAG f95, ifort, sunf95 and openf95. Actually, g95 shows the bug with this PR fixes:

    (3) The identifier of the entity in the referenced module if
        that identifier does not appear as a use-name or
        use-defined-operator in any rename for that module.

(see comment 0 for the full quote). That is: "a" is not imported from the module as "a" as it is already imported as "local1".
Comment 12 Paul Thomas 2007-11-25 10:40:54 UTC
(In reply to comment #11)
> I think the failures in test3 are ok. Example program:

Yes, you are right.  You and Dominique are correct about test2.  The odd thing is that there is a test for this in the testsuite already.  I'll fix it tonight.

Many thanks to both of you.

Paul
Comment 13 Paul Thomas 2007-11-27 19:22:10 UTC
Subject: Bug 33541

Author: pault
Date: Tue Nov 27 19:21:52 2007
New Revision: 130471

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=130471
Log:
2007-11-27  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33541
	*interface.c (compare_actual_formal): Exclude assumed size
	arrays from the possibility of scalar to array mapping.
	* decl.c (get_proc_name): Fix whitespace problem.

	PR fortran/34231
	* gfortran.h : Add 'use_rename' bit to symbol_attribute.
	* module.c : Add 'renamed' field to pointer_info.u.rsym.
	(load_generic_interfaces): Add 'renamed' that is set after the
	number_use_names is called.  This is used to set the attribute
	use_rename, which, in its turn identifies those symbols that
	have not been renamed.
	(load_needed): If pointer_info.u.rsym->renamed is set, then
	set the use_rename attribute of the symbol.
	(read_module): Correct an erroneous use of use_flag. Use the
	renamed flag and the use_rename attribute to determine which
	symbols are not renamed.

2007-11-27  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33541
	* gfortran.dg/use_11.f90: New test.

	PR fortran/34231
	* gfortran.dg/generic_15.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/generic_15.f90
    trunk/gcc/testsuite/gfortran.dg/use_11.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/decl.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/interface.c
    trunk/gcc/fortran/module.c
    trunk/gcc/testsuite/ChangeLog

Comment 14 Paul Thomas 2007-11-27 20:50:48 UTC
Fixed on trunk.... I hope!

Paul