diff mbox

[fortran] PR63744 accept duplicate use-rename

Message ID 54D524BC.2090908@sfr.fr
State New
Headers show

Commit Message

Mikael Morin Feb. 6, 2015, 8:31 p.m. UTC 
Hello,

we currently reject programs of the form
> 
> module m
>     integer :: s
> end module m
> subroutine s
>     use m, only: x => s, x => s
> end subroutine s

with an error stating that S is the name of the current program unit.
Interestingly, the duplicate rename is necessary to trigger it.

There doesn't seem to be a consensus as to whether it should be
accepted, but I think it should be.
Quoting Dominique's comment in the PR:
>  if
> 
>   use m, only: A => X
>   use m, only: B => X
> 
> is valid, I don't see why
> 
>   use m, only: A => X
>   use m, only: A => X
> 
> should not.
The problem is we check the original (symbol) name instead of the local
(symtree) name.
The fix is close to obvious, and should be safe for the branches (this
is a regression)
Regression tested on x86_64-linux. OK for trunk/4.9/4.8 ?

Mikael
2015-02-06  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/63744
	* module.c (check_for_ambiguous): Change argument type
	from gfc_symbol to gfc_symtree.  Check local (symtree) name
	instead of original (symbol) name.
	(read_module): Update caller.

2015-02-06  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/63744
	gfortran.dg/use_rename_8.f90: New.

Comments

Paul Richard Thomas Feb. 7, 2015, 12:41 p.m. UTC | #1 
Dear Mikael,

It looks good to me for trunk and the branches.

Thanks for the patch

Paul

On 6 February 2015 at 21:31, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Hello,
>
> we currently reject programs of the form
>>
>> module m
>>     integer :: s
>> end module m
>> subroutine s
>>     use m, only: x => s, x => s
>> end subroutine s
>
> with an error stating that S is the name of the current program unit.
> Interestingly, the duplicate rename is necessary to trigger it.
>
> There doesn't seem to be a consensus as to whether it should be
> accepted, but I think it should be.
> Quoting Dominique's comment in the PR:
>>  if
>>
>>   use m, only: A => X
>>   use m, only: B => X
>>
>> is valid, I don't see why
>>
>>   use m, only: A => X
>>   use m, only: A => X
>>
>> should not.
> The problem is we check the original (symbol) name instead of the local
> (symtree) name.
> The fix is close to obvious, and should be safe for the branches (this
> is a regression)
> Regression tested on x86_64-linux. OK for trunk/4.9/4.8 ?
>
> Mikael
>
>
>
diff mbox

Patch

Index: module.c
===================================================================
--- module.c	(révision 220107)
+++ module.c	(copie de travail)
@@ -4795,19 +4795,21 @@  read_cleanup (pointer_info *p)
 /* It is not quite enough to check for ambiguity in the symbols by
    the loaded symbol and the new symbol not being identical.  */
 static bool
-check_for_ambiguous (gfc_symbol *st_sym, pointer_info *info)
+check_for_ambiguous (gfc_symtree *st, pointer_info *info)
 {
   gfc_symbol *rsym;
   module_locus locus;
   symbol_attribute attr;
+  gfc_symbol *st_sym;
 
-  if (gfc_current_ns->proc_name && st_sym->name == gfc_current_ns->proc_name->name)
+  if (gfc_current_ns->proc_name && st->name == gfc_current_ns->proc_name->name)
     {
       gfc_error ("%qs of module %qs, imported at %C, is also the name of the "
-		 "current program unit", st_sym->name, module_name);
+		 "current program unit", st->name, module_name);
       return true;
     }
 
+  st_sym = st->n.sym;
   rsym = info->u.rsym.sym;
   if (st_sym == rsym)
     return false;
@@ -5037,7 +5039,7 @@  read_module (void)
 	  if (st != NULL)
 	    {
 	      /* Check for ambiguous symbols.  */
-	      if (check_for_ambiguous (st->n.sym, info))
+	      if (check_for_ambiguous (st, info))
 		st->ambiguous = 1;
 	      else
 		info->u.rsym.symtree = st;