Bug 54286 - [4.8 Regression] Accepts invalid proc-pointer assignments involving proc-ptr function result
Summary: [4.8 Regression] Accepts invalid proc-pointer assignments involving proc-ptr ...
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: Paul Thomas
URL:
Keywords: accepts-invalid
Depends on:
Blocks:
 
Reported: 2012-08-16 15:05 UTC by janus
Modified: 2013-01-15 05:34 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2013-01-08 00:00:00


Attachments
A patch that recovers the original error (334 bytes, patch)
2013-01-08 21:43 UTC, Paul Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description janus 2012-08-16 15:05:21 UTC
The following test is probably invalid, but not rejected by recent trunk versions:


type :: t
  procedure(a), pointer, nopass :: p
end type

type(t) :: x
procedure(iabs), pointer :: pp

x%p => a     ! ok
x%p => a(1)  ! invalid, but not rejected by 4.8

pp => a(2)   ! ok
pp => a      ! invalid, but not rejected by 4.8

contains

  function a (c) result (b)
    integer, intent(in) :: c
    procedure(iabs), pointer :: b
    b => iabs
  end function

end



gfortran 4.7 rejects it with:

x%p => a(1)
       1
Error: Interface mismatch in procedure pointer assignment at (1): Type/rank mismatch in return value of 'b'

pp => a
      1
Error: Interface mismatch in procedure pointer assignment at (1): Type/rank mismatch in return value of 'a'


However, I wonder why one of the errors refers to 'a' and the other to 'b'. Also I'm not sure whether 'type/rank mismatch' is the correct diagnostic. I would rather say there is a mismatch in the procedure pointer attribute of the result (which is not checked for yet), cf. also PR 35831.
Comment 1 Tobias Burnus 2012-09-19 12:28:50 UTC
(When you look into this problem: I think the current handling of assumed-rank is also wrong (sorry, my mistake). For array passing, assumed-rank dummies take any rank (including scalar). However, for dummy procedures or proc-pointers, if the dummy/proc-pointer has an assumed rank dummy, the actual/proc/target has to have one too.)
Comment 2 Paul Thomas 2013-01-08 21:43:46 UTC
Created attachment 29113 [details]
A patch that recovers the original error

Tobias' remarks about the appropriateness of the error are justified.  I will look into this tomorrow.

Paul
Comment 3 Paul Thomas 2013-01-09 09:44:07 UTC
Please find below my interpretation of the validity or not of the testcase for this PR.

> The following test is probably invalid, but not rejected by recent trunk
> versions:
> 
> 
> type :: t
>   procedure(a), pointer, nopass :: p
> end type
> 
> type(t) :: x
> procedure(iabs), pointer :: pp
> 
> x%p => a     ! ok

[A] I believe that this is invalid because the interfaces do not match
procedure(iabs), pointer  =>  integer pointer procedure (integer formal)

It strikes me as being a moot point as to whetehr or not the processor is required to report it though, on the gorunds that "anything goes" with pointers! That said, the interface check is easy to implement.

> x%p => a(1)  ! invalid, but not rejected by 4.8

[B] This is valid:
procedure(iabs), pointer  =>  pointer to integer procedure(iabs)

It has the pleasing property of providing the correct result for
print *, x%p(-99) .eq. iabs(-99)

> 
> pp => a(2)   ! ok

[B] applies - it also gives the correct result.

> pp => a      ! invalid, but not rejected by 4.8

[A] applies - invalid.

> 
> contains
> 
>   function a (c) result (b)
>     integer, intent(in) :: c
>     procedure(iabs), pointer :: b
>     b => iabs
>   end function
> 
> end

Interestingly, both invalid lines run and provide results that are consistent with INT (loc (iabs)) :-)

Paul
Comment 4 Mikael Morin 2013-01-09 12:06:53 UTC
(In reply to comment #3)
> > type :: t
> >   procedure(a), pointer, nopass :: p
> > end type
> > 
> > type(t) :: x
> > procedure(iabs), pointer :: pp
> > 
> > x%p => a     ! ok
> 
> [A] I believe that this is invalid because the interfaces do not match
> procedure(iabs), pointer  =>  integer pointer procedure (integer formal)

[...] 
> > x%p => a(1)  ! invalid, but not rejected by 4.8
> 
> [B] This is valid:
> procedure(iabs), pointer  =>  pointer to integer procedure(iabs)

Huh? the interface of `x%p' is `a', not `iabs'.
I think the comments are correct; the former is valid, the latter invalid.
Comment 5 Paul Thomas 2013-01-09 13:07:31 UTC
(In reply to comment #4)
> (In reply to comment #3)
....snip....
> Huh? the interface of `x%p' is `a', not `iabs'.
> I think the comments are correct; the former is valid, the latter invalid.

Quite right - thanks, Mikael.  I missed/forgot/ignored the first interface.  That makes life a lot easier because the patch does the right thing :-)

Cheers

Paul
Comment 6 Paul Thomas 2013-01-13 08:57:51 UTC
Author: pault
Date: Sun Jan 13 08:57:46 2013
New Revision: 195133

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195133
Log:
2013-01-13  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/54286
	* expr.c (gfc_check_pointer_assign): Ensure that both lvalue
	and rvalue interfaces are presented to gfc_compare_interfaces.
	Simplify references to interface names by using the symbols
	themselves. Call gfc_compare_interfaces with s1 and s2 inter-
	changed to overcome the asymmetry of this function. Do not
	repeat the check for the presence of s1 and s2.

2013-01-13  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/54286
	* gfortran.dg/proc_ptr_result_8.f90 : New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/proc_ptr_result_8.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/expr.c
    trunk/gcc/testsuite/ChangeLog
Comment 7 Paul Thomas 2013-01-14 05:52:48 UTC
Fixed on trunk.

Thanks for the report

Paul
Comment 8 Paul Thomas 2013-01-14 20:32:04 UTC
ICE noted by Dominique d'Humieres with

module m
  type :: foobar
    real, pointer :: array(:)
    procedure (), pointer, nopass :: f
  end type
contains
  elemental subroutine fooAssgn (a1, a2)
    type(foobar), intent(out) :: a1
    type(foobar), intent(in) :: a2
    allocate (a1%array(size(a2%array)))
    a1%array = a2%array
    a1%f => a2%f
  end subroutine
end module m

Dominique also provided the fix on #gfortran:
in gfc_check_pointer_assign of ../../gcc/fortran/expr.c:3540
replacing if (s2->attr.proc_pointer with if (s2 && s2->attr.proc_pointer
lets the code compile.

Will commit as 'obvious' as soon as it has bootstrapped and regtested.

Paul
Comment 9 Dominique d'Humieres 2013-01-14 22:35:20 UTC
Additional comment from #gfortran:

AFAIU this kind of changes, they cannot cause a problem for anything that did not triggered the ICE.

So they are basically harmless, excepted that they can trun an ICE into a wrong code, which is worse (IMO).
 
Too bad I cannot remember the origin of the code.
Comment 10 Paul Thomas 2013-01-15 05:29:07 UTC
Author: pault
Date: Tue Jan 15 05:29:01 2013
New Revision: 195185

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195185
Log:
2013-01-15  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/54286
	* expr.c (gfc_check_pointer_assign): Check for presence of
	's2' before using it.

2013-01-15  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/54286
	* gfortran.dg/proc_ptr_result_8.f90 : Add module 'm' to check
	case where interface is null.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/expr.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/proc_ptr_result_8.f90
Comment 11 Paul Thomas 2013-01-15 05:34:25 UTC
Hopefully, it will stay fixed this time!

Thanks Dominique.

Paul