Bug 54195 - [4.8 Regression][OOP] IMPORT fails with GENERIC TBP: "is already present in the interface"
Summary: [4.8 Regression][OOP] IMPORT fails with GENERIC TBP: "is already present in t...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.8.0
: P4 normal
Target Milestone: 4.8.0
Assignee: janus
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2012-08-07 15:44 UTC by Tobias Burnus
Modified: 2013-02-04 20:53 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-08-07 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2012-08-07 15:44:27 UTC
Reported by Damian at http://gcc.gnu.org/ml/fortran/2012-08/msg00041.html

The following program fails with:
    generic :: operator(-) => unary
                             1
Error: Entity 'unary' at (1) is already present in the interface

It works if one replaces "IMPORT :: foo" by "IMPORT".


module import_clashes_with_generic
  type ,abstract :: foo
  contains
    procedure :: unary
    generic :: operator(-) => unary
  end type

  abstract interface
    integer function bar()
      import :: foo
    end function
  end interface
contains
  integer function unary(rhs)
    class(foo) ,intent(in) :: rhs
  end function
end module
Comment 1 janus 2012-08-07 16:44:48 UTC
First guess:

http://gcc.gnu.org/viewcvs?view=revision&revision=189022
Comment 2 Tobias Burnus 2012-08-07 16:47:10 UTC
It works with the snapshot GCC 4.8.0 20120624 and fails with the one from 20120701.

The changelog lists only one entry in that period, namely:

Author: janus
Date: Wed Jun 27 17:38:00 2012
New Revision: 189022

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189022
Log:
2012-06-27  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/41951
    PR fortran/49591
    * interface.c (check_new_interface): Rename, add 'loc' argument,
    make non-static.
    (gfc_add_interface): Rename 'check_new_interface'
    * gfortran.h (gfc_check_new_interface): Add prototype.
    * resolve.c (resolve_typebound_intrinsic_op): Add typebound operator
    targets to non-typebound operator list.
Comment 3 janus 2012-08-07 18:12:07 UTC
(In reply to comment #2)
> The changelog lists only one entry in that period, namely:
> 
> Author: janus
> Date: Wed Jun 27 17:38:00 2012
> New Revision: 189022

Yeah, I knew it ;)

I just verified that reverting the resolve.c part of this commit makes the error go away.
Comment 4 janus 2012-08-07 20:23:26 UTC
My first attempt to fix it would be something like this:

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c       (revision 190186)
+++ gcc/fortran/resolve.c       (working copy)
@@ -11448,7 +11448,7 @@
 
       /* Add target to non-typebound operator list.  */
       if (!target->specific->deferred && !derived->attr.use_assoc
-         && p->access != ACCESS_PRIVATE)
+         && !derived->attr.imported && p->access != ACCESS_PRIVATE)
        {
          gfc_interface *head, *intr;
          if (gfc_check_new_interface (derived->ns->op[op], target_proc,

This indeed makes the error in comment #0 go away, and does not seem to introduce any regressions in the typebound_* test cases.

However: Witout the patch, the code guarded by the IF statement above is called twice (which leads to the error). But with the patch, it is not called at all any more (while it should be called once!). So the patch can not be fully correct.

What I do not understand is: The symbol 'foo' exists in the module namespace, as well as in the namespace of 'bar'. But in both it has the 'imported' attribute, which sounds wrong to me (or at least contra-intuitive).
Comment 5 janus 2012-08-07 20:57:47 UTC
Second attempt:

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 190186)
+++ gcc/fortran/resolve.c	(working copy)
@@ -11448,7 +11448,7 @@ resolve_typebound_intrinsic_op (gfc_symbol* derive
 
       /* Add target to non-typebound operator list.  */
       if (!target->specific->deferred && !derived->attr.use_assoc
-	  && p->access != ACCESS_PRIVATE)
+	  && p->access != ACCESS_PRIVATE && derived->ns == gfc_current_ns)
 	{
 	  gfc_interface *head, *intr;
 	  if (gfc_check_new_interface (derived->ns->op[op], target_proc,


As the first attempt, it fixes the error in comment 0 and survives the typebound_* test cases, but it also results in the code in question being called exactly once for the test case in comment 0.

Will start a full regtest now ...
Comment 6 janus 2012-08-07 21:21:23 UTC
(In reply to comment #4)
> However: Witout the patch, the code guarded by the IF statement above is called
> twice (which leads to the error). But with the patch, it is not called at all
> any more (while it should be called once!). So the patch can not be fully
> correct.
> 
> What I do not understand is: The symbol 'foo' exists in the module namespace,
> as well as in the namespace of 'bar'. But in both it has the 'imported'
> attribute, which sounds wrong to me (or at least contra-intuitive).

As I just learned from Tobias, this problem is probably related to PR53537.
Comment 7 janus 2012-08-07 22:05:49 UTC
(In reply to comment #5)
> Will start a full regtest now ...

Completed successfully.
Comment 8 janus 2012-08-20 09:45:15 UTC
A similar test case (also a regression) was reported by Andrew Benson at http://gcc.gnu.org/ml/fortran/2012-08/msg00101.html:


module gn

  implicit none

  type :: nc
   contains
     procedure :: assign => nca
     generic   :: assignment(=) => assign
  end type

  type, extends(nc) :: ncb
   contains
     procedure , nopass :: tis => bf
  end type

contains

  subroutine nca(to,from)
    class(nc), intent(out) :: to
    type(nc), intent(in) :: from
  end subroutine

  logical function bf()
    bf=.false.
  end function

end module



This fails with:

     generic   :: assignment(=) => assign
                                  1
Error: Entity 'nca' at (1) is already present in the interface
Comment 9 janus 2013-01-07 21:23:20 UTC
Apparently comment #0 has been fixed by

http://gcc.gnu.org/viewcvs?view=revision&revision=193568

which applied the patch of comment #5. However, the test case in comment #8 currently still fails.
Comment 10 janus 2013-01-08 09:47:17 UTC
For comment #8, resolve_typebound_intrinsic_op is called twice: Once for the base type 'nc', and once for the extended type 'ncb'.

A crude way to avoid this is the following:

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 194927)
+++ gcc/fortran/resolve.c	(working copy)
@@ -12315,15 +12315,10 @@ static gfc_try
 resolve_typebound_procedures (gfc_symbol* derived)
 {
   int op;
-  gfc_symbol* super_type;
 
   if (!derived->f2k_derived || !derived->f2k_derived->tb_sym_root)
     return SUCCESS;
 
-  super_type = gfc_get_derived_super_type (derived);
-  if (super_type)
-    resolve_typebound_procedures (super_type);
-
   resolve_bindings_derived = derived;
   resolve_bindings_result = SUCCESS;
 

This fails on:

FAIL: gfortran.dg/abstract_type_6.f03  -O  (test for excess errors)
FAIL: gfortran.dg/typebound_operator_14.f90  -O  (internal compiler error)
Comment 11 Mikael Morin 2013-01-11 14:07:22 UTC
(In reply to comment #10)
> For comment #8, resolve_typebound_intrinsic_op is called twice: Once for the
> base type 'nc', and once for the extended type 'ncb'.
> 
The `gfc_namespace' struct has a `resolved' attribute.  Maybe we can use it?
Comment 12 janus 2013-01-11 14:29:41 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > For comment #8, resolve_typebound_intrinsic_op is called twice: Once for the
> > base type 'nc', and once for the extended type 'ncb'.
> > 
> The `gfc_namespace' struct has a `resolved' attribute.  Maybe we can use it?

Not sure. I was thinking that we may need such an attribute for each type symbol ("gfc_symbol.resolved"?). See e.g. PR 44978 for a related problem.
Comment 13 Mikael Morin 2013-02-02 11:07:30 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > The `gfc_namespace' struct has a `resolved' attribute.  Maybe we can use it?
> 
> Not sure. I was thinking that we may need such an attribute for each type
> symbol ("gfc_symbol.resolved"?). See e.g. PR 44978 for a related problem.

It is actually not as trivial as it looks.  In PR44978, the recursion on the parent type is in resolve_fl_derived0.  So the flag should be used there.
However, in this bug the recursion on the parent type is in resolve_typebound_procedures, so the same flag can't be used for both. The latter recursion can be made through resolve_fl_derived, or even through resolve_symbol (see patch below), but it would keep the recursion in resolve_fl_derived0 untouched, hence PR44978 unfixed.  To fix both, there should be almost one flag per function, and one should cache the return values as well. :-(

So the easiest IMO is the following:
This fixes PR54107 comment 26:

diff --git a/gfortran.h b/gfortran.h
index 16751b4..f93e4d3 100644
--- a/gfortran.h
+++ b/gfortran.h
@@ -696,6 +696,9 @@ extern const ext_attr_t ext_attr_list[];
 /* Symbol attribute structure.  */
 typedef struct
 {
+  gfc_try resolve_cached_result;
+  unsigned resolved:1;
+
   /* Variable attributes.  */
   unsigned allocatable:1, dimension:1, codimension:1, external:1, intrinsic:1,
     optional:1, pointer:1, target:1, value:1, volatile_:1, temporary:1,
diff --git a/resolve.c b/resolve.c
index d6bae43..ef06f4e 100644
--- a/resolve.c
+++ b/resolve.c
@@ -13170,6 +13170,10 @@ resolve_symbol (gfc_symbol *sym)
   gfc_array_spec *as;
   bool saved_specification_expr;
 
+  if (sym->attr.resolved)
+    return;
+  sym->attr.resolved = 1;
+
   if (sym->attr.artificial)
     return;
 

and then, with this:

diff --git a/resolve.c b/resolve.c
index d6bae43..ef06f4e 100644
--- a/resolve.c
+++ b/resolve.c
@@ -12349,7 +12349,7 @@ resolve_typebound_procedures (gfc_symbol* derived)
 
   super_type = gfc_get_derived_super_type (derived);
   if (super_type)
-    resolve_typebound_procedures (super_type);
+    resolve_symbol (super_type);
 
   resolve_bindings_derived = derived;
   resolve_bindings_result = SUCCESS;

comment #8 of this bug is fixed as well.  I'm inclined to test and submit both, and leave PR44978 unfixed.
Comment 14 Mikael Morin 2013-02-02 11:15:48 UTC
(In reply to comment #13)
>  typedef struct
>  {
> +  gfc_try resolve_cached_result;
> +  unsigned resolved:1;
> +
>    /* Variable attributes.  */

resolve_cached_result is actually unneeded.  It comes from previous attempts.
Comment 15 Mikael Morin 2013-02-02 12:13:41 UTC
(In reply to comment #13)
> I'm inclined to test and submit both,

They break class_20.f03.  Fixed with this (reverts partially patches for pr44044 and pr48112):

diff --git a/resolve.c b/resolve.c
index ef06f4e..5d0d4a6 100644
--- a/resolve.c
+++ b/resolve.c
@@ -11051,11 +11051,6 @@ resolve_fl_var_and_proc (gfc_symbol *sym, int mp_flag)
 {
   gfc_array_spec *as;
 
-  /* Avoid double diagnostics for function result symbols.  */
-  if ((sym->result || sym->attr.result) && !sym->attr.dummy
-      && (sym->ns != gfc_current_ns))
-    return SUCCESS;
-
   if (sym->ts.type == BT_CLASS && sym->attr.class_ok)
     as = CLASS_DATA (sym)->as;
   else
Comment 16 Mikael Morin 2013-02-04 18:34:42 UTC
Author: mikael
Date: Mon Feb  4 18:34:30 2013
New Revision: 195729

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195729
Log:
fortran/
	PR fortran/54107
	PR fortran/54195
	* gfortran.h (struct gfc_symbol): New field 'resolved'.
	* resolve.c (resolve_fl_var_and_proc): Don't skip result symbols.
	(resolve_symbol): Skip duplicate calls.  Don't check the current
	namespace.

testsuite/
	PR fortran/54107
	* gfortran.dg/recursive_interface_1.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/recursive_interface_1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog
Comment 17 Mikael Morin 2013-02-04 19:06:15 UTC
Author: mikael
Date: Mon Feb  4 19:06:06 2013
New Revision: 195730

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195730
Log:
fortran/
	PR fortran/54195
	* resolve.c (resolve_typebound_procedures): Recurse through
	resolve_symbol.

testsuite/
	PR fortran/54195
	* gfortran.dg/typebound_operator_19.f90: New test.
	* gfortran.dg/typebound_assignment_4.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/typebound_assignment_4.f90
    trunk/gcc/testsuite/gfortran.dg/typebound_operator_19.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog
Comment 18 Mikael Morin 2013-02-04 20:53:33 UTC
(In reply to comment #9)
> Apparently comment #0 has been fixed by
> 
> http://gcc.gnu.org/viewcvs?view=revision&revision=193568
> 
> which applied the patch of comment #5. However, the test case in comment #8
> currently still fails.

Comment #8 is now fixed as well.