Bug 48786 - [OOP] Generic ambiguity check too strict for polymorphic dummies
Summary: [OOP] Generic ambiguity check too strict for polymorphic dummies
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.7.0
: P4 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code, wrong-code
Depends on:
Blocks:
 
Reported: 2011-04-27 07:31 UTC by Tobias Burnus
Modified: 2013-08-21 14:48 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.5.3
Known to fail: 4.6.0, 4.7.0
Last reconfirmed: 2013-07-02 00:00:00


Attachments
Example program (738 bytes, text/plain)
2011-04-27 07:31 UTC, Tobias Burnus
Details
Example 2, cf. comment 3, third item (1.49 KB, text/plain)
2011-04-29 09:24 UTC, Tobias Burnus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2011-04-27 07:31:48 UTC
Created attachment 24110 [details]
Example program

Reported at c.l.f by Arjen Markus
http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/4a4e885f68b660bd

I have not really thought about the issue, but I believe gfortran wrongly rejects the program.


One has:
  type point2d
with
  generic, public :: operator(+) => add_vector
and
  type, extends(point2d) :: point3d
with
  generic, public :: operator(+) => add_vector_3dversion

As one can pass any "point3d" data type to either add_vector_3dversion or add_vector, gfortran complains:
  Error: 'add_vector_3d' and 'add_vector_2d' for GENERIC '+' at (1) are ambiguous

However, the TKI is different, and thus I believe the error message is wrong - in calls, add_vector_3d should be preferred and only if not available, one should fall back to add_vector_2d.

"A dummy argument is type, kind, and rank compatible, or TKR compatible, with another dummy argument if the first is type compatible with the second, the kind type parameters of the first have the same values as the corresponding kind type parameters of the second, and both have the same rank." (F2008, 12.4.3.4.5)

-> I believe the arguments have a different (declared) type.

It should also be somewhat compatible with: "12.5.5.2 Resolving procedure references to names established to be generic" - but there it does not tell which function one should call. Admittedly, I am a bit lost.
Comment 1 Tobias Burnus 2011-04-27 13:55:00 UTC
It's a regression; with GCC 4.5.3 20110421 one gets the correct error:

            point = point + vector
            1
Error: Variable must not be polymorphic in assignment at (1)


While 4.6.1 20110422 and 4.7 regard 'add_vector_3d' and 'add_vector_2d' as ambiguous. (Thus, the above diagnostic will not be reached.)
Comment 2 Tobias Burnus 2011-04-28 08:04:27 UTC
See http://gcc.gnu.org/ml/fortran/2011-04/msg00293.html for a variant which ICE (segfaults) after printing the error.

Regarding the initial program: Except for the assignment (cf. comment 1), the program compiles also with Cray's ftn (incl. run time of a modified program) and with ifort 12; it also compiles w/o assignment error (!?) with PGI.
Comment 3 Tobias Burnus 2011-04-29 09:24:30 UTC
Created attachment 24141 [details]
Example 2, cf. comment 3, third item

(In reply to comment #2)
> See http://gcc.gnu.org/ml/fortran/2011-04/msg00293.html for a variant
> which ICE (segfaults) after printing the error.

(With 4.7.0 20110428 [compiled with release checking], I do not see the ICE.)

( Note: The thread continues at http://gcc.gnu.org/ml/fortran/2011-04/msg00294.html )

 * * *

I have thought about the question again - and now I think that gfortran 4.6/4.7 correctly rejects the program as ambiguous, even though ifort 12.0, crayftn 7.3, PGI and gfortran 4.5 accept it.

For:
  function add_vector_2d( point, vector )
    class(point2d), intent(in)  :: point
    class(point2d), intent(in)  :: vector

  function add_vector_3d( point, vector )
    class(point3d), intent(in)  :: point
    type(point3d), intent(in)   :: vector

I believe the interfaces are ambiguous as:

"Two dummy arguments are distinguishable if [...] neither is TKR compatible with the other" (F2008, 12.4.3.4.5)

For an actual argument of the type "class(point3d),type(point3d)", either function has the correct interface. More precisely, class(point2d) is type compatible* with class(point3d) and class(point2d) is type compatible with type(point3d). -- The crucial word is "neither": While add_vector_3d's dummies are not type compatible to add_vector_2d - the reverse is not true. Thus, the arguments are not distinguishable.

(* A polymorphic entity that is not an unlimited polymorphic entity is type compatible with entities of the same declared type or any of its extensions.", F2008, 4.3.1.3.)

 * * *

If one modifies the program (cf. attachment 24110 [details]) as follows, gfortran 4.7 segfaults at

==30150== Invalid read of size 8
==30150==    at 0x4CB104: gfc_compare_derived_types (interface.c:409)
==30150==    by 0x52ACCE: gfc_type_is_extension_of (symbol.c:4700)
==30150==    by 0x52AD8E: gfc_type_compatible (symbol.c:4728)


--- hj44.f90    2011-04-29 10:38:53.663239782 +0200
+++ hj444.f90   2011-04-29 10:39:39.111745236 +0200
@@ -17,3 +17 @@
-        procedure               :: add_vector_3dversion      => add_vector_3d
-        generic, public         :: add_vector_new            => add_vector_3dversion
-        generic, public         :: operator(+) => add_vector_3dversion
+        procedure               :: add_vector      => add_vector_3d
@@ -38 +36 @@
-    type(point2d)               :: add_vector_2d
+    class(point2d)               :: add_vector_2d
@@ -47 +45 @@
-    type(point3d), intent(in)   :: vector
+    class(point3d), intent(in)   :: vector
@@ -49 +47 @@
-    type(point3d)               :: add_vector_3d
+    class(point3d)               :: add_vector_3d

 * * *

Regarding a second program at
http://gcc.gnu.org/ml/fortran/2011-04/msg00302.html (see example 2 attachment): The program compiles and segfault at run time for

function add_vector_2d( point, vector )
    class(point2d), intent(in)  :: point, vector
    class(point2d), allocatable :: add_vector_2d

    if ( allocated(add_vector_2d) ) then
        deallocate( add_vector_2d )
    endif
    allocate( add_vector_2d )

The issue is that the result variable is not properly initialized; from the dump:

  struct __class_points2d3d_Point2d_a __result_add_vector_2d;
  if (__result_add_vector_2d._data != 0B)
Comment 4 Tobias Burnus 2011-04-29 16:04:45 UTC
Mini analysis:

(In reply to comment #3)
> ==30150== Invalid read of size 8
> ==30150==    at 0x4CB104: gfc_compare_derived_types (interface.c:409)

The problem is that the second argument of gfc_compare_derived_types is derived2 == NULL.


> The issue is that the result variable is not properly initialized; from the
> dump:
> 
>   struct __class_points2d3d_Point2d_a __result_add_vector_2d;
>   if (__result_add_vector_2d._data != 0B)

That's odd as it should be handled in trans-decl.c around line 3524 (gfc_trans_deferred_vars).
Comment 5 Tobias Burnus 2011-04-30 10:04:59 UTC
(In reply to comment #3)
> If one modifies the program (cf. attachment 24110 [details]) as follows,
> gfortran 4.7 segfaults

The reason is that in gfc_type_compatible, the ts1 and ts2 are both BT_CLASS,
with ts1->u.derived->name == point3d
and  ts2->u.derived->name == point2d

As point3d is derived from point2d, the first component is the parent type:
  ts1->u.derived->components->name == point2d
while point2d is not derived and thus the first component is the "real :: x" component:
  ts2->u.derived->components->name == x

Thus, it is not surprising that accessing "x"'s ts.u.derived is NULL:
  ts2->u.derived->components->ts.u.derived

gfc_type_compatible is called via gfc_compare_types, which is called by resolve.c's check_typebound_override

      if (!gfc_compare_types (&proc_target->result->ts,
                              &old_target->result->ts))

 * * *

> Regarding a second program at
> http://gcc.gnu.org/ml/fortran/2011-04/msg00302.html (see example 2 attachment):

Draft patch at: http://gcc.gnu.org/ml/fortran/2011-04/msg00318.html
Comment 6 Tobias Burnus 2011-05-11 13:45:52 UTC
A) Regarding the ambiguity issue (cf. comment 0 and comment 3): As written, I believe the interface is indeed ambiguous - if that's the case, there is no regression in gfortran and comment 0 of this bug is INVALID. I have asked at J3: http://j3-fortran.org/pipermail/j3/2011-May/004366.html

Hence: Regression but diagnostic is probably correct -> INVALID. Asked J3 for confirmation.


B) Regarding the wrong-code bug (example 2, attachment 24141 [details]): See patch at http://gcc.gnu.org/ml/fortran/2011-04/msg00318.html
The patch fixes the issue but as the review comment suggests, one should factorize the code into a different function to make it easier to maintain and to read, cf. http://gcc.gnu.org/ml/fortran/2011-05/msg00001.html
(Note: You might need valgrind or malloc perturb to run into the bug.)

Hence: OOP wrong-code issue but no regression; patch exists, but needs to be cleaned up.


C) Regarding the segfault (test case: attachment 24110 [details] with the patch of comment 3), see comment 5

Hence: No regression but an OOP ice-on-valid-code (segfault) bug, which still needs to be investigated and fixed.
Comment 7 Tobias Burnus 2011-05-11 14:30:36 UTC
(In reply to comment #6)
> I have asked at J3: http://j3-fortran.org/pipermail/j3/2011-May/004366.html

IBM's xlf and someone at IBM also regard it as invalid. Thus, I have now removed the [Regression] marker - and downgraded it to a normal OOP bug.
Comment 8 Dominique d'Humieres 2013-07-02 09:13:00 UTC
For the record, the link in comment #6 is now
http://mailman.j3-fortran.org/pipermail/j3/2011-May/004366.html

This PR is quite difficult to follow. AFAICT the main issue has been fixed almost two years ago:
(1) The errors of the kind

Error: 'add_vector_3d' and 'add_vector_2d' for GENERIC '+' at (1) are ambiguous

are valid.
(2) The test in  attachment 24141 [details] compiles and run without error (however valgrind reports a memory leak) since revision 178509 (4.7, 2011-09-04).
(3) Up to revision 177649 (2011-08-11), the test compiled but aborted at run time with

 Two-dimensional walk:
a.out(48088) malloc: *** error for object 0x7fff5fbfd690: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

(4) The original test at https://groups.google.com/forum/?hl=it#!topic/comp.lang.fortran/4Y0ENMRlmOI

gives the result expected by Salvatore Filippone and if I remove the commented lines, I get
pr48786_5_db.f90:30.29:

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

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

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

I have only one issue left: the test by Ian Harvey at https://groups.google.com/forum/#!topic/comp.lang.fortran/Sk6IX2i2YL0
gives an ICE when compiled with a clean revision 200078:

pr48786_1.f90:4.43:

     CHARACTER(:), ALLOCATABLE :: some_text 
                                           1
Error: Deferred-length character component 'some_text' at (1) is not yet supported
... repeated several times ...
f951: internal compiler error: in matching_typebound_op, at fortran/interface.c:3535

f951: internal compiler error: Abort trap
gfcc: internal compiler error: Abort trap (program f951)
Abort

My proposal is to close this PR as fixed and to open a new one for the remaining issue. Any objection?
Comment 9 janus 2013-08-21 08:48:34 UTC
(In reply to Dominique d'Humieres from comment #8)
> AFAICT the main issue has been fixed almost two years ago:
> (1) The errors of the kind
> 
> Error: 'add_vector_3d' and 'add_vector_2d' for GENERIC '+' at (1) are
> ambiguous
> 
> are valid.

After this interpretation has been established here, it is a bit unfortunate to see that current trunk (4.9.0 20130820, r201883) does not show the error any more (in contrast to 4.8 and 4.7). If we still believe that the error is correct, then this is a regression (would be interesting to know which revision caused it).
Comment 10 janus 2013-08-21 09:10:54 UTC
(In reply to janus from comment #9)
> After this interpretation has been established here, it is a bit unfortunate
> to see that current trunk (4.9.0 20130820, r201883) does not show the error
> any more (in contrast to 4.8 and 4.7).

However, the variant in comment 2 still shows the error. The difference is that in comment 0 one has an argument with type(point3d) instead of class(3d), so maybe this is again an issue with symmetrization.
Comment 11 janus 2013-08-21 10:25:00 UTC
(In reply to janus from comment #9)
> If we still believe that the error is
> correct, then this is a regression (would be interesting to know which
> revision caused it).

Apparently it is due to r201329 (for PR 57530), in particular the interface.c part.
Comment 12 Dominique d'Humieres 2013-08-21 12:19:22 UTC
I still get the errors

pr48786_2.f90:132.46:

     generic, public         :: operator(+) => add_vector
                                              1
Error: 'add_vector_3d' and 'add_vector_2d' for GENERIC '+' at (1) are ambiguous
pr48786_2.f90:133.50:

     generic, public         :: assignment(=)   => assign
                                                  1
Error: 'assign_3d' and 'assign_2d' for GENERIC '=' at (1) are ambiguous
pr48786_2.f90:249.6:

  use points2d3d   ! Both 2D and 3D points available
      1
Fatal Error: Can't open module file 'points2d3d.mod' for reading at (1): No such file or directory

with the test in http://gcc.gnu.org/ml/fortran/2011-04/msg00293.html
at r201817 or 4.8.1.

The errors are gone with the following change

--- pr48786_2.f90	2013-07-01 22:53:35.000000000 +0200
+++ pr48786_2_db.f90	2013-08-21 14:15:35.000000000 +0200
@@ -129,8 +129,8 @@ module points2d3d
      procedure               :: add_vector      => add_vector_3d
      procedure               :: random          => random_vector_3d
      procedure               :: assign          => assign_3d
-     generic, public         :: operator(+) => add_vector
-     generic, public         :: assignment(=)   => assign
+!     generic, public         :: operator(+) => add_vector
+!     generic, public         :: assignment(=)   => assign
   end type point3d
 
 contains

in type, extends(point2d) :: point3d. Is this a bug or the expected behavior?
Comment 13 janus 2013-08-21 14:43:16 UTC
(In reply to janus from comment #11)
> Apparently it is due to r201329 (for PR 57530), in particular the
> interface.c part.

However, reverting this via

Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(revision 201883)
+++ gcc/fortran/interface.c	(working copy)
@@ -514,12 +514,6 @@ compare_type (gfc_symbol *s1, gfc_symbol *s2)
   if (s2->attr.ext_attr & (1 << EXT_ATTR_NO_ARG_CHECK))
     return 1;
 
-  /* TYPE and CLASS of the same declared type are type compatible,
-     but have different characteristics.  */
-  if ((s1->ts.type == BT_CLASS && s2->ts.type == BT_DERIVED)
-      || (s1->ts.type == BT_DERIVED && s2->ts.type == BT_CLASS))
-    return 0;
-
   return gfc_compare_types (&s1->ts, &s2->ts) || s2->ts.type == BT_ASSUMED;
 }
 

results in these testsuite failures:

FAIL: gfortran.dg/proc_ptr_30.f90  -O   (test for errors, line 25)
FAIL: gfortran.dg/proc_ptr_comp_32.f90  -O   (test for errors, line 34)
FAIL: gfortran.dg/proc_ptr_comp_33.f90  -O   (test for errors, line 14)
FAIL: gfortran.dg/proc_ptr_comp_33.f90  -O   (test for errors, line 54)
FAIL: gfortran.dg/dummy_procedure_4.f90  -O   (test for errors, line 28)
Comment 14 janus 2013-08-21 14:48:08 UTC
(In reply to Dominique d'Humieres from comment #12)
> I still get the errors
> 
> pr48786_2.f90:132.46:
> 
>      generic, public         :: operator(+) => add_vector
>                                               1
> Error: 'add_vector_3d' and 'add_vector_2d' for GENERIC '+' at (1) are
> ambiguous
> 
> [...]
> 
> with the test in http://gcc.gnu.org/ml/fortran/2011-04/msg00293.html
> at r201817 or 4.8.1.
> 
> [...]
> 
> Is this a bug or the expected behavior?

The test case you refer to is the one from comment 2 and, as I mentioned, the behavior did not change for this one (according to previous evaluations in this PR the behavior is correct).