Bug 41951 - [OOP] Not diagnosing ambiguous operators (TB vs. INTERFACE)
Summary: [OOP] Not diagnosing ambiguous operators (TB vs. INTERFACE)
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: accepts-invalid
Depends on: 41539
Blocks: 29670
  Show dependency treegraph
 
Reported: 2009-11-05 17:00 UTC by Tobias Burnus
Modified: 2018-10-13 12:32 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-12-05 22:14:52


Attachments
Test case - see marked line for the ICE (749 bytes, text/plain)
2009-11-05 17:01 UTC, Tobias Burnus
Details
legalized test case (747 bytes, text/plain)
2009-11-06 10:35 UTC, janus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2009-11-05 17:00:01 UTC
The following program is an extension to the one in PR 41539. It finally requires to many features that ifort 11.1 does not like it - nor the dated NAG f95 5.1 (current is 5.2). And while gfortran should have everything needed, it still does not work.

I have no idea whether the code is correct - probably not - but an ICE is definitely wrong. (For other issues see PR 41539.)

$ gfortran sort.f90
f951: internal compiler error: in gfc_match_varspec, at fortran/primary.c:1815
Please submit a full bug report
Comment 1 Tobias Burnus 2009-11-05 17:01:02 UTC
Created attachment 18974 [details]
Test case - see marked line for the ICE
Comment 2 janus 2009-11-06 10:35:46 UTC
Created attachment 18978 [details]
legalized test case

The test case in comment #1 is clearly illegal (and rejected by NAG 5.2). This version should be valid.
Comment 3 Thomas Koenig 2009-12-05 22:14:51 UTC
Reduced test case, from c2.f90:

module m_sort
  implicit none
  type, abstract :: sort_t
  contains
    procedure(gt_cmp), deferred :: gt_cmp
  end type sort_t
contains
  subroutine bsort(a)
    class(sort_t), intent(inout) :: a(:)
    if (a(1)%gt_cmp(a(2))) then ! ICE
    end if
  end subroutine bsort
end module m_sort
Comment 4 Thomas Koenig 2011-01-23 10:10:33 UTC
Another test case, from

http://groups.google.com/group/comp.lang.fortran/msg/e6a865eda59e86db?hl=de


module mytypes
   implicit none
   private
   public :: mytype, get_i

   integer, save :: i_priv = 13

   type :: mytype
      integer :: dummy
    contains
      procedure, nopass :: i => get_i
   end type mytype

 contains

   pure function get_i () result (i)
     integer :: i
     i = i_priv
   end function get_i

end module mytypes


program test
   use mytypes
   implicit none

   type(mytype) :: a
   type(mytype), parameter :: a_const = mytype (0)

!   integer, dimension (get_i()) :: x            ! #1
!   integer, dimension (a%i()) :: x              ! #2
   integer, dimension (a_const%i()) :: x        ! #3

   print *, size (x)

end program test
Comment 5 Dominique d'Humieres 2011-01-23 10:57:05 UTC
> Another test case, from ...

It is pr47399.
Comment 6 Tobias Burnus 2011-12-12 08:03:41 UTC
STATUS:
With the commit for PR 41539, there is no longer an ICE.

However, while the example of comment 2 (attachment 18978 [details]) compiles, it fails to link with:   undefined reference to `assign_'

Given that NAG 5.2 compiles it and as "assign" is only called via "assignment(=)", there must be some issue with the assignment resolution.
Comment 7 Tobias Burnus 2011-12-12 13:14:24 UTC
(In reply to comment #6)
> However, while the example of comment 2 (attachment 18978 [details]) compiles,
> it fails to link with:   undefined reference to `assign_'

The problem is in "subroutine bsort(a,tmp)": The line
    tmp = a(i)
should call tmp->_vtr->assign. However, it calls a simple external "assign" procedure. If one uses:
    call tmp%assign(a(i))
one finds the correct dump, namely:
    tmp->_vptr->assign ((struct __class_m_sort_Sort_t *) tmp, &class.2);


The "assign" is found when resolve_ordinary_assign calls gfc_extend_assign, which runs:
  3452    sym = gfc_search_interface (ns->op[INTRINSIC_ASSIGN], 1, &actual);

That returns the INTERFACE instead of the type bound procedure:

(gdb) p sym->name
$9 = 0x2aaaacde8fd0 "assign"
(gdb) p sym->attr.external
$11 = 1

If one changes the interface to an abstract one, one gets 
   ERROR: ABSTRACT INTERFACE 'assign' must not be referenced


Using "assign" instead of the type-bound "assign" is definitely a bug as no  "INTERFACE ASSIGNMENT(=)" exists - there is only the type-based "generic :: assignment(=) => assign".
Comment 8 Tobias Burnus 2011-12-13 11:40:51 UTC
(In reply to comment #6)
> Given that NAG 5.2 compiles it and as "assign" is only called via
> "assignment(=)", there must be some issue with the assignment resolution.

Actually, I wonder whether the code is valid. One has:

  type, abstract :: sort_t
  contains
    generic :: assignment(=) => assign
    ...

  interface assignment(=)
    procedure assign
  end interface

Thus, one defines twice a generic name with the same (= identical) interface, once as TBP and once as external procedure. Thus, when one encounters:
    tmp = a(i)
the compiler has the choice between "assign_" and "tmp->_vtab->assign".

I failed to nail it in the standard, but I am sure that "12.4.3.4.5 Restrictions on generic declarations" somehow must apply.
Comment 9 Tobias Burnus 2011-12-16 14:30:15 UTC
(In reply to comment #8)
> I failed to nail it in the standard, but I am sure that "12.4.3.4.5
> Restrictions on generic declarations" somehow must apply.

Thus I have now asked at http://j3-fortran.org/pipermail/j3/2011-December/004946.html
Comment 10 Tobias Burnus 2012-01-17 08:33:22 UTC
I have now updated the summary line to reflect the remaining issue.

(In reply to comment #9)
> Thus I have now asked at
> http://j3-fortran.org/pipermail/j3/2011-December/004946.html

Answer by Malcolm Cohen: It's invalid
http://j3-fortran.org/pipermail/j3/2012-January/004986.html
Comment 11 janus 2012-06-19 15:53:02 UTC
Here is a reduced version of comment 2, which is invalid according to comment 10:


module m_sort
  implicit none

  type, abstract :: sort_t
  contains
    generic :: assignment(=) => assign
    procedure(assign_it), deferred :: assign 
  end type

  interface assignment(=)
    subroutine assign_it (lhs, rhs)
      import
      class(sort_t), intent(out) :: lhs
      class(sort_t), intent(in) :: rhs
    end subroutine
  end interface

end module
Comment 12 janus 2012-06-23 13:54:29 UTC
(In reply to comment #11)
> Here is a reduced version of comment 2, which is invalid according to comment
> 10:

Here is a non-abstract version of the test case in comment #11, which should be equally invalid (however, in this case it is not a problem of having two ambiguous procedures, but adding the same procedure twice to the same generic assignment operator):

module m_sort
  implicit none

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

  interface assignment(=)
    module procedure :: assign
  end interface

contains

  subroutine assign (lhs, rhs)
    class(sort_t), intent(out) :: lhs
    class(sort_t), intent(in) :: rhs
  end subroutine

end module
Comment 13 janus 2012-06-27 17:38:11 UTC
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.


2012-06-27  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/41951
	PR fortran/49591
	* gfortran.dg/typebound_operator_16.f03: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/typebound_operator_16.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/interface.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog
Comment 14 janus 2012-06-27 17:54:29 UTC
Comment 12 is fixed with r189022, but comment 11 is still accepted without error.
Comment 15 Tobias Burnus 2012-06-27 18:18:07 UTC
(In reply to comment #14)
> Comment 12 is fixed with r189022, but comment 11 is still accepted without
> error.

Note that previous commit also excluded PRIVATE type-bound operators, cf. http://gcc.gnu.org/ml/fortran/2012-06/msg00149.html.

Whatever a private type-bound operators/assignments are, given that they automatically become available when use-associating the type. (As they have to have a pass argument, they won't interfere with other operator declarations.)
Comment 16 janus 2012-11-16 12:04:13 UTC
(In reply to comment #15)
> (In reply to comment #14)
> > Comment 12 is fixed with r189022, but comment 11 is still accepted without
> > error.

One way to reject the abstract case in comment 11, would be to just do the checking, but not add the procedure to the operator list:


Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 193552)
+++ gcc/fortran/resolve.c	(working copy)
@@ -11539,19 +11539,21 @@ resolve_typebound_intrinsic_op (gfc_symbol* derive
 	goto error;
 
       /* Add target to non-typebound operator list.  */
-      if (!target->specific->deferred && !derived->attr.use_assoc
-	  && p->access != ACCESS_PRIVATE)
+      if (!derived->attr.use_assoc && p->access != ACCESS_PRIVATE)
 	{
-	  gfc_interface *head, *intr;
 	  if (gfc_check_new_interface (derived->ns->op[op], target_proc,
 				       p->where) == FAILURE)
 	    return FAILURE;
-	  head = derived->ns->op[op];
-	  intr = gfc_get_interface ();
-	  intr->sym = target_proc;
-	  intr->where = p->where;
-	  intr->next = head;
-	  derived->ns->op[op] = intr;
+	  if (!target->specific->deferred)
+	    {
+	      gfc_interface *head, *intr;
+	      head = derived->ns->op[op];
+	      intr = gfc_get_interface ();
+	      intr->sym = target_proc;
+	      intr->where = p->where;
+	      intr->next = head;
+	      derived->ns->op[op] = intr;
+	    }
 	}
     }
Comment 17 janus 2012-11-16 12:33:00 UTC
(In reply to comment #16)
> One way to reject the abstract case in comment 11, would be to just do the
> checking, but not add the procedure to the operator list:

The only problem with the patch in comment 16 is that the error message it gives on comment 11 ...

generic :: assignment(=) => assign
                               1
Error: Entity 'assign_it' at (1) is already present in the interface

... is technically not correct. 'assign_it' is not added twice, but in one case is just the interface of the deferred type-boud procedure 'assign'. Instead, it might be better to complain about 'Ambiguous interfaces' (since any non-abstract extension of the type 'sort_t' will have to override/implement the type-bound 'assign' with a procedure which is ambiguous to 'assign_it').

However, such an error about 'Ambiguous interfaces' is also missing for the following case (which is still accepted):

module m_sort
  implicit none
  
  type, abstract :: sort_t
  contains
    generic :: assignment(=) => assign
    procedure(assign_ifc), deferred :: assign 
  end type

  abstract interface
    subroutine assign_ifc (lhs, rhs)
      import
      class(sort_t), intent(out) :: lhs
      class(sort_t), intent(in) :: rhs
    end subroutine
  end interface

  interface assignment(=)
    subroutine assign_it (lhs, rhs)
      import
      class(sort_t), intent(out) :: lhs
      class(sort_t), intent(in) :: rhs
    end subroutine
  end interface

end module