User account creation filtered due to spam.

Bug 46196 - [OOP] gfortran compiles invalid generic TBP: dummy arguments are type compatible
Summary: [OOP] gfortran compiles invalid generic TBP: dummy arguments are type compatible
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: 4.6.0
Assignee: janus
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-27 09:34 UTC by Hans-Werner Boschmann
Modified: 2016-11-16 13:41 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-10-27 20:32:13


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hans-Werner Boschmann 2010-10-27 09:34:16 UTC
module generic
  type :: a_type
   contains
     procedure :: a_subroutine
  end type a_type
  type,extends(a_type) :: b_type
   contains
     procedure :: b_subroutine
     generic :: g_sub => a_subroutine,b_subroutine
  end type b_type
contains
  subroutine a_subroutine(this)
    class(a_type)::this
  end subroutine a_subroutine
  subroutine b_subroutine(this)
    class(b_type)::this
  end subroutine b_subroutine
end module generic

GNU Fortran (GCC) 4.6.0 20101022 (experimental)

This code is invalid, because the dummy arguments in a_subroutine and b_subroutine are type compatible. Therefore they must not share a generic interface.
Comment 1 janus 2010-10-27 11:58:15 UTC
Yes. Apparently a duplicate of PR 44917/44926.
Comment 2 janus 2010-10-27 20:32:13 UTC
Draft patch. It seems to fix this PR as well as all of PR44917 and PR44926. Not tested for regressions yet.


Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c     (revision 165975)
+++ gcc/fortran/interface.c     (working copy)
@@ -872,7 +872,8 @@ count_types_test (gfc_formal_arglist *f1, gfc_form
       /* Find other nonoptional arguments of the same type/rank.  */
       for (j = i + 1; j < n1; j++)
        if ((arg[j].sym == NULL || !arg[j].sym->attr.optional)
-           && compare_type_rank_if (arg[i].sym, arg[j].sym))
+           && (compare_type_rank_if (arg[i].sym, arg[j].sym)
+               || compare_type_rank_if (arg[j].sym, arg[i].sym)))
          arg[j].flag = k;
 
       k++;
@@ -897,7 +898,8 @@ count_types_test (gfc_formal_arglist *f1, gfc_form
       ac2 = 0;
 
       for (f = f2; f; f = f->next)
-       if (compare_type_rank_if (arg[i].sym, f->sym))
+       if (compare_type_rank_if (arg[i].sym, f->sym)
+           || compare_type_rank_if (f->sym, arg[i].sym))
          ac2++;
 
       if (ac1 > ac2)
@@ -948,7 +950,8 @@ generic_correspondence (gfc_formal_arglist *f1, gf
       if (f1->sym->attr.optional)
        goto next;
 
-      if (f2 != NULL && compare_type_rank (f1->sym, f2->sym))
+      if (f2 != NULL && (compare_type_rank (f1->sym, f2->sym)
+                        || compare_type_rank (f2->sym, f1->sym)))
        goto next;
 
       /* Now search for a disambiguating keyword argument starting at
Comment 3 Dominique d'Humieres 2010-10-27 21:51:23 UTC
With the patch in comment #2, several of my codelets are rejected: for instance the test in comment #24 of pr42274 is rejected with:

[macbook] f90/bug% gfc pr42274_5.f90
pr42274_5.f90:14.33:

    generic, public :: extract => make_integer_2
                                 1
Error: 'make_integer_2' and 'make_integer' for GENERIC 'extract' at (1) are ambiguous
pr42274_5.f90:33.7:

  use m
       1
Fatal Error: Can't open module file 'm.mod' for reading at (1): No such file or directory

If you need I can sort out the other similar new errors tomorrow.
Comment 4 janus 2010-10-28 15:32:44 UTC
(In reply to comment #3)
> With the patch in comment #2, several of my codelets are rejected: for instance
> the test in comment #24 of pr42274 is rejected with:
> 
> [macbook] f90/bug% gfc pr42274_5.f90
> pr42274_5.f90:14.33:
> 
>     generic, public :: extract => make_integer_2
>                                  1
> Error: 'make_integer_2' and 'make_integer' for GENERIC 'extract' at (1) are
> ambiguous

Yes, this is expected. The test case is invalid.


> If you need I can sort out the other similar new errors tomorrow.

Maybe you could check if there are any 'false alarm' errors among that ...
Comment 5 Dominique d'Humieres 2010-10-28 21:06:36 UTC
> Maybe you could check if there are any 'false alarm' errors among that ...

I'll do it. Meanwhile I am puzzled by the patch. My understanding of compare_type_rank and compare_type_rank_if is that these tests should be symmetric: test(A,B)==test(B,A). So if test(A,B)||test(B,A) changes something it means that the tests are actually asymmetric. This asymmetry seems buggy. Looking at gcc/fortran/interface.c, I see at lines 447 to 458:

      if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
            && !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
            && gfc_compare_types (&dt1->ts, &dt2->ts) == 0)
        return 0;

      else if ((dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
                && !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived))
        return 0;

      else if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
                && (dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived))
        return 0;

Should not this be

      if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
            && !(dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived)
            && gfc_compare_types (&dt1->ts, &dt2->ts) == 0)
        return 0;

      else if ((dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
                && !(dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived))
        return 0;

      else if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
                && (dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived))
        return 0;

???
Comment 6 Mikael Morin 2010-10-28 21:58:48 UTC
(In reply to comment #5)
> I'll do it. Meanwhile I am puzzled by the patch. My understanding of
> compare_type_rank and compare_type_rank_if is that these tests should be
> symmetric: test(A,B)==test(B,A). So if test(A,B)||test(B,A) changes something
> it means that the tests are actually asymmetric. This asymmetry seems buggy.
> Looking at gcc/fortran/interface.c, I see at lines 447 to 458:
>   [...]
> Should not this be
>   [...]
> ???

The code is not executed here (it should not at least ;-) ): types without the sequence attribute. But there is indeed something to fix here. You can propose your patch to the list.
 
For the asymmetry, I see gfc_compare_types and gfc_type_compatible as possible causes too.
Comment 7 janus 2010-10-29 08:31:41 UTC
(In reply to comment #5)
> Meanwhile I am puzzled by the patch. My understanding of
> compare_type_rank and compare_type_rank_if is that these tests should be
> symmetric: test(A,B)==test(B,A). So if test(A,B)||test(B,A) changes something
> it means that the tests are actually asymmetric. This asymmetry seems buggy.

Well, this symmetry is exactly what the patch is about. For F95 derived types the functions you mention are indeed symmetric (and they should be!). However, for polymorphic arguments this symmetry no longer holds (cf. gfc_type_compatible), because TYPE(t) (and all its extensions) are compatible with CLASS(t) (e.g. you can pass an actual TYPE(t) argument to a CLASS(t) formal), but not the other way around.

Now, for comparing specifics in a generic interface, one needs to symmetrize this again by adding a call with switched arguments.


> Looking at gcc/fortran/interface.c, I see at lines 447 to 458:
> 
>       if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
>             && !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
>             && gfc_compare_types (&dt1->ts, &dt2->ts) == 0)
>         return 0;
> 
>       else if ((dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
>                 && !(dt1->ts.type == BT_DERIVED && derived1 ==
> dt1->ts.u.derived))
>         return 0;
> 
>       else if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
>                 && (dt1->ts.type == BT_DERIVED && derived1 ==
> dt1->ts.u.derived))
>         return 0;
> 
> Should not this be
> 
>       if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
>             && !(dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived)
>             && gfc_compare_types (&dt1->ts, &dt2->ts) == 0)
>         return 0;
> 
>       else if ((dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
>                 && !(dt2->ts.type == BT_DERIVED && derived2 ==
> dt2->ts.u.derived))
>         return 0;
> 
>       else if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
>                 && (dt2->ts.type == BT_DERIVED && derived2 ==
> dt2->ts.u.derived))
>         return 0;
> 
> ???

As already noted by Mikael, this piece of code is not important for this PR. Still I guess you're right that it is bogus.
Comment 8 Dominique d'Humieres 2010-10-29 16:00:57 UTC
I have successfully regtested the following patch, i.e., the patch in
comment #2 and the fix for the typos reported in comment #5, on top of
revision 166058 (plus a few unrelated patches). I also noticed that the 
fix for pr46067 use the asymmetry of gfc_compare_interfaces!-)

------------------------------------------------------------------------

--- ../_clean/gcc/fortran/interface.c	2010-10-27 23:47:20.000000000 +0200
+++ gcc/fortran/interface.c	2010-10-29 10:55:07.000000000 +0200
@@ -445,16 +445,16 @@ gfc_compare_derived_types (gfc_symbol *d
       /* Make sure that link lists do not put this function into an 
	 endless recursive loop!  */
       if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
-	    && !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
+	    && !(dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived)
	    && gfc_compare_types (&dt1->ts, &dt2->ts) == 0)
	return 0;
 
       else if ((dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
-		&& !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived))
+		&& !(dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived))
	return 0;
 
       else if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
-		&& (dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived))
+		&& (dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived))
	return 0;
 
       dt1 = dt1->next;
@@ -872,7 +872,8 @@ count_types_test (gfc_formal_arglist *f1
       /* Find other nonoptional arguments of the same type/rank.  */
       for (j = i + 1; j < n1; j++)
	if ((arg[j].sym == NULL || !arg[j].sym->attr.optional)
-	    && compare_type_rank_if (arg[i].sym, arg[j].sym))
+	    && (compare_type_rank_if (arg[i].sym, arg[j].sym)
+ 		|| compare_type_rank_if (arg[j].sym, arg[i].sym)))
	  arg[j].flag = k;
 
       k++;
@@ -897,7 +898,8 @@ count_types_test (gfc_formal_arglist *f1
       ac2 = 0;
 
       for (f = f2; f; f = f->next)
-	if (compare_type_rank_if (arg[i].sym, f->sym))
+	if (compare_type_rank_if (arg[i].sym, f->sym)
+	    || compare_type_rank_if (f->sym, arg[i].sym))
	  ac2++;
 
       if (ac1 > ac2)
@@ -948,7 +950,8 @@ generic_correspondence (gfc_formal_argli
       if (f1->sym->attr.optional)
	goto next;
 
-      if (f2 != NULL && compare_type_rank (f1->sym, f2->sym))
+      if (f2 != NULL && (compare_type_rank (f1->sym, f2->sym)
+ 			 || compare_type_rank (f2->sym, f1->sym)))
	goto next;
 
       /* Now search for a disambiguating keyword argument starting at

------------------------------------------------------------------------

This has disturbed my bug collection for the following tests:

*Code in (1)                             pr41951_3.f90:21.32:

    generic :: operator(.gt.) => gt_cmp_int
				1
Error: 'gt_cmp_int' and 'gt_cmp' for GENERIC '.gt.' at (1) are ambiguous


*Code in comment #24                    pr42274_5.f90:14.33:

    generic, public :: extract => make_integer_2
				 1
Error: 'make_integer_2' and 'make_integer' for GENERIC 'extract' at (1) are ambiguous

*Code in comment #0 with !!$ removed    pr43945_2.f90:30.29:

    generic, public :: do  => doit 
			     1
Error: 'doit2' and 'doit' for GENERIC 'do' at (1) are ambiguous
pr43945_2.f90:31.29:

    generic, public :: get => getit 
			     1
Error: 'getit2' and 'getit' for GENERIC 'get' at (1) are ambiguous

*Expected                               pr44917.f90, pr44917_1.f90, oop.f90
oop.f90:22.33:

    generic, public :: extract => real, make_integer_2
				 1
Error: 'make_integer_2' and 'make_integer' for GENERIC 'extract' at (1) are ambiguous

*Expected                               pr44926.f90:33.35:

    generic, public    :: csget  => d_get
				   1
Error: 'd_get' and 'base_csgetp' for GENERIC 'csget' at (1) are ambiguous

*Expected                               pr46196.f90:9.24:

     generic :: g_sub => a_subroutine,b_subroutine
			1
Error: 'a_subroutine' and 'b_subroutine' for GENERIC 'g_sub' at (1) are ambiguous


------------------------------------------------------------------------

(1) code for pr41951_3.f90:

module m_sort
  implicit none
  type, abstract :: sort_t
  contains
    generic :: operator(.gt.) => gt_cmp
    procedure :: gt_cmp
    end type sort_t
contains
  logical function gt_cmp(a,b)
    class(sort_t), intent(in) :: a, b
    gt_cmp = .true.
  end function gt_cmp
end module

module test
  use m_sort
  implicit none
  type, extends(sort_t) :: sort_int_t
    integer :: i
  contains
    generic :: operator(.gt.) => gt_cmp_int
    procedure :: gt_cmp_int
  end type
contains
  logical function gt_cmp_int(a,b) result(cmp)
    class(sort_int_t), intent(in) :: a, b
    if (a%i > b%i) then
      cmp = .true.
     else
      cmp = .false.
     end if
  end function gt_cmp_int
end module

end

I am pretty bad with such ambiguous stuff, especially with classes, so I 
let the experts check that there is no false positive.
Comment 9 janus 2010-10-30 09:33:12 UTC
(In reply to comment #8)
> I have successfully regtested the following patch

Thanks.

> I also noticed that the 
> fix for pr46067 use the asymmetry of gfc_compare_interfaces!-)

Right.



> --- ../_clean/gcc/fortran/interface.c    2010-10-27 23:47:20.000000000 +0200
> +++ gcc/fortran/interface.c    2010-10-29 10:55:07.000000000 +0200
> @@ -445,16 +445,16 @@ gfc_compare_derived_types (gfc_symbol *d
>        /* Make sure that link lists do not put this function into an 
>      endless recursive loop!  */
>        if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
> -        && !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
> +        && !(dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived)
>         && gfc_compare_types (&dt1->ts, &dt2->ts) == 0)
>     return 0;
> 
>        else if ((dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
> -        && !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived))
> +        && !(dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived))
>     return 0;
> 
>        else if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
> -        && (dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived))
> +        && (dt2->ts.type == BT_DERIVED && derived2 == dt2->ts.u.derived))
>     return 0;


Can we find any test case which is sensitive to this change?



> This has disturbed my bug collection for the following tests:

AFAICS all of the error messages are correct. Thanks for checking.
Comment 10 Dominique d'Humieres 2010-10-30 11:52:46 UTC
> Can we find any test case which is sensitive to this change?

I di not find one in store!-(
Am I correct to expect that such test will be of the "reject valid" kind?

Clearly
(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
                && !(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
is always false and the third test is the same. So these tests are just nops and the 'return 0' will never be reached. Then the if-else block reduces to

      if (!(dt1->ts.type == BT_DERIVED && derived1 == dt1->ts.u.derived)
            && gfc_compare_types (&dt1->ts, &dt2->ts) == 0)
        return 0;

For this case we need a test with 'dt1->ts.type == BT_DERIVED' and 'dt1->ts.u.derived == derived1' and one of these conditions false for dt2. My understanding of the code is not good enough to reach a valid test from that (I don't know where derived* come from.
Comment 11 Mikael Morin 2010-10-30 12:51:54 UTC
Another bug, we don't check that components are non-null before entering the loop. 


      module type_a
      type a
              sequence
      end type a
      end module type_a

      module type_a_bis
      type a
              sequence
      end type a
      end module type_a_bis

      use type_a
      use type_a_bis, only: b => a

      type(b) :: y
      call bar(y)

    contains

      subroutine bar (x)
        type(a) :: x
      end subroutine bar

  end

PS: Dominique, you can start hacking with this testcase. gfc_compare_derived_types is called with it (when comparing type of `y' vs type of `x'). 

PS2: we will need a new PR in the end.
Comment 12 janus 2010-10-30 13:52:44 UTC
Author: janus
Date: Sat Oct 30 13:52:39 2010
New Revision: 166089

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166089
Log:
2010-10-30  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44917
	PR fortran/44926
	PR fortran/46196
	* interface.c (count_types_test): Symmetrize type check.
	(generic_correspondence): Ditto.

2010-10-30  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44917
	PR fortran/44926
	PR fortran/46196
	* gfortran.dg/typebound_generic_10.f03: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/typebound_generic_10.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/interface.c
    trunk/gcc/testsuite/ChangeLog
Comment 13 janus 2010-10-30 14:31:49 UTC
The original test case is fixed with r166089. The problem mentioned in comment #5 will be tracked by PR 46244.