Bug 43291 - [4.5 regression] [OOP] Type mismatch in argument; passed CLASS(t1) to CLASS(t2)
Summary: [4.5 regression] [OOP] Type mismatch in argument; passed CLASS(t1) to CLASS(t2)
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Paul Thomas
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-08 15:48 UTC by janus
Modified: 2010-03-12 22:05 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Fix for the PR (922 bytes, patch)
2010-03-09 13:09 UTC, Paul Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description janus 2010-03-08 15:48:14 UTC
This is a spin-off from PR42769 comment #17. The test case has been extracted from comment #1 in that PR. It was working before r157272 (which fixed PR43256), but fails now:

module m1
  type  :: t1
  contains 
    procedure :: sizeof
  end type
contains
  integer function sizeof(a)
    class(t1) :: a
  end function sizeof
end module


module m2
  use m1
  type, extends(t1) :: t2    
  contains
    procedure :: sizeof => sizeof2
  end type
contains
  integer function sizeof2(a)
    class(t2) :: a
  end function
end module


module m3
  use m2
  type :: t3
    class(t1), allocatable  :: a 
  contains
    procedure :: sizeof => sizeof3
  end type
contains 
  integer function sizeof3(a)
    class(t3) :: a
    sizeof3 = a%a%sizeof()
  end function 
end module



    sizeof3 = a%a%sizeof()
              1
Error: Type mismatch in argument 'a' at (1); passed CLASS(t1) to CLASS(t2)
Comment 1 Dominique d'Humieres 2010-03-08 15:57:34 UTC
As noted in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43256#c9, the third hunk of the patch for resolve.c in revision 157272 does not apply to fortran-dev. The test in comment #0 passes with fortran-dev and my patched trunk in which the first two hunks of the patch have been applied (so the cause of this pr is probably comming from the third one).
Comment 2 janus 2010-03-09 09:44:44 UTC
The problem here is that when resolving a polymorphic TBP call, we resolve the call for each member of the CLASS (i.e. the declared type and all its children, cf. 'resolve_class_compcall'). In 'check_class_members' we set the correct type for the passed object. However, this does not work if the passed object is a component, because the ts we set is overridden when the expr is resolved (e.g. in resolve_actual_arglist).
Comment 3 janus 2010-03-09 09:49:46 UTC
(In reply to comment #2)
> The problem here is that when resolving a polymorphic TBP call, we resolve the
> call for each member of the CLASS (i.e. the declared type and all its children,
> cf. 'resolve_class_compcall').

Paul, since you were the one who implemented this: Could you me remind me why this is needed at all? Shouldn't it be enough to resolve the call as is, i.e. just for the base class? Checking the actual arguments for every descendant of the base class seems unnecessary to me.
Comment 4 Paul Thomas 2010-03-09 11:33:07 UTC
(In reply to comment #3)

> 
> Paul, since you were the one who implemented this: Could you me remind me why
> this is needed at all? Shouldn't it be enough to resolve the call as is, i.e.
> just for the base class? Checking the actual arguments for every descendant of
> the base class seems unnecessary to me.
> 
Dear Janus,

I believe that it is not necessary. The code reuses the original resolve_compcall for class hierarchies in order to get the specific instance of the procedure for the given class member.  You will note that resolution for each class members is foregone in the case of subroutines. Perhaps the most straightforward thing to do would be to add a further static boolean flag that signals that the derived type is the declared type?

I have just updated my tree to take the fixes in that you mention below. I'll try to check out what I say later but don wait for me - the painkillers that I am taking after my operation are leaving me very prone to nodding off to sleep.

Paul
Comment 5 Paul Thomas 2010-03-09 13:09:02 UTC
Created attachment 20056 [details]
Fix for the PR

This is just now regtesting.  I believe that it is OK, since it works for gfortran.dg/class* and gfortran.dg/select* :-)

If all appears to be well with the regtest and I do not hear back from you, I will commit the patch to trunk tonight.

Cheers

Paul
Comment 6 Paul Thomas 2010-03-09 13:34:30 UTC
> If all appears to be well with the regtest and I do not hear back from you, I
> will commit the patch to trunk tonight.

Confidence before the fall and all that.....

The patch clobbers dynamic_dispatch_4.f03.  Remember it Salvatore?

OK, back to the drawing board!

Paul

Comment 7 Paul Thomas 2010-03-09 13:46:51 UTC
(In reply to comment #6)
....
> The patch clobbers dynamic_dispatch_4.f03.  Remember it Salvatore?

I should have said that this is for -O1 and higher.

/svn/trunk/gcc/testsuite/gfortran.dg/dynamic_dispatch_4.f03: In function ‘getit’:
/svn/trunk/gcc/testsuite/gfortran.dg/dynamic_dispatch_4.f03:79:0: error: statement makes a memory store, but has no VDEFS
a_3.$vptr = D.1823_2;
/svn/trunk/gcc/testsuite/gfortran.dg/dynamic_dispatch_4.f03:79:0: internal compiler error: verify_ssa failed

which means a whole lot to me!

Paul
Comment 8 Salvatore Filippone 2010-03-09 14:14:28 UTC
(In reply to comment #6)
> > If all appears to be well with the regtest and I do not hear back from you, I
> > will commit the patch to trunk tonight.
> 
> Confidence before the fall and all that.....
> 
> The patch clobbers dynamic_dispatch_4.f03.  Remember it Salvatore?
> 
Ah, yes, how nice to see old friends again :-) 

> OK, back to the drawing board!
> 
> Paul
> 

Good luck 
Salvatore....
.....who would obviously like to see the segfault 42274 in the -dev branch solved, but knows better than to ask forcefully during regression-only fix frenzy.. 

Comment 9 Dominique d'Humieres 2010-03-09 15:20:55 UTC
For the record, the patch in comment #5 won't apply on fortran-dev (no class_try in the branch).
Comment 10 Paul Thomas 2010-03-10 09:29:11 UTC
In preparing a testcase, I foolishly decided to check the original for correct execution.

The following gives the wrong result:
module m1
  type  :: t1
  contains 
    procedure :: sizeof
  end type
contains
  integer function sizeof(a)
    class(t1) :: a
    sizeof = 1
  end function sizeof
end module


module m2
  use m1
  type, extends(t1) :: t2    
  contains
    procedure :: sizeof => sizeof2
  end type
contains
  integer function sizeof2(a)
    class(t2) :: a
    sizeof2 = 2
  end function
end module


module m3
  use m2
  type :: t3
    class(t1), pointer  :: a 
  contains
    procedure :: sizeof => sizeof3
  end type
contains 
  integer function sizeof3(a)
    class(t3) :: a
    sizeof3 = a%a%sizeof()
  end function 
end module

  use m1
  use m2
  use m3
  class(t1), pointer :: a, ptr
  type(t1), target :: x
  type(t2), target :: y
  type(t3) :: z
  a => x
  print *, a%sizeof()
  a => y
  print *, a%sizeof()
  z%a => x
  print *, z%sizeof(), z%a%sizeof()
  z%a => y
  print *, z%sizeof(), z%a%sizeof()

end

gives
           1
           2
           1           1
           2           1

The last line should read
           2           2

of course.

The logic in calling resolve_class_compcall is wrong.

Paul




Comment 11 janus 2010-03-10 20:46:04 UTC
(In reply to comment #6)
> > If all appears to be well with the regtest and I do not hear back from you, I
> > will commit the patch to trunk tonight.
> 
> The patch clobbers dynamic_dispatch_4.f03. 

Hm, funny. For me the patch from comment #5 survives a full regtest. Ok to commit from my side.
Comment 12 janus 2010-03-10 21:04:27 UTC
(In reply to comment #10)
> In preparing a testcase, I foolishly decided to check the original for correct
> execution.
> 
> The following gives the wrong result:

I guess this is worth a separate PR. It's PR43326 now.
Comment 13 Paul Thomas 2010-03-12 22:01:11 UTC
Subject: Bug 43291

Author: pault
Date: Fri Mar 12 22:00:52 2010
New Revision: 157411

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157411
Log:
2010-03-12  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/43291
	PR fortran/43326
	* resolve.c (resolve_compcall): Add new boolean dummy argument
	'class_members'. Only resolve expression at end if false.
	Remove redundant, static variable 'class_object'.
	(check_class_members): Add extra argument to call of
	resolve_compcall.
	(resolve_typebound_function): Renamed resolve_class_compcall.
	Do all the detection of class references here. Correct calls to
	resolve_compcall for extra argument.
	(resolve_typebound_subroutine): resolve_class_typebound_call
	renamed. Otherwise same as resolve_typebound_function.
	(gfc_resolve_expr): Call resolve_typebound_function.
	(resolve_code): Call resolve_typebound_subroutine.

2010-03-12  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/43291
	PR fortran/43326
	* gfortran.dg/dynamic_dispatch_7.f03: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/dynamic_dispatch_7.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog

Comment 14 Paul Thomas 2010-03-12 22:05:57 UTC
Fixed on trunk.

I´ll keep 43326 open for a bit, until I figure out what to do with fortran-dev.

Thanks for the report.

Cheers

Paul