[Fortran] PR 84273: Reject allocatable passed-object dummy argument (proc_ptr_47.f90)

Message ID CAKwh3qj7h9WFAvMs5o1CnUbBEkM1pmUQtNPOLXUhD+YREdRe+w@mail.gmail.com
State New
Headers show
Series
  • [Fortran] PR 84273: Reject allocatable passed-object dummy argument (proc_ptr_47.f90)
Related show

Commit Message

Janus Weil Feb. 9, 2018, 5:13 p.m.
Hi all,

the attached patch fixes some checking code for PASS arguments in
procedure-pointer components, which does not properly account for the
fact that the PASS argument needs to be polymorphic.

[The reason for this issue is probably that PPCs were mostly
implemented before polymorphism was available. The corresponding
pass-arg checks for TBPs are ok.]

The patch also fixes an invalid test case (which was detected thanks
to Neil Carlson). It regtests cleanly on x86_64-linux-gnu. Ok for
trunk?

Cheers,
Janus



2018-02-09  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/84273
    * resolve.c (resolve_component): Fix checks of passed argument in
    procedure-pointer components.


2018-02-09  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/84273
    * gfortran.dg/proc_ptr_47.f90: Fix invalid test case.
    * gfortran.dg/proc_ptr_comp_pass_4.f90: Fix and extend test case.

Comments

Steve Kargl Feb. 9, 2018, 11:21 p.m. | #1
On Fri, Feb 09, 2018 at 06:13:34PM +0100, Janus Weil wrote:
> 
> the attached patch fixes some checking code for PASS arguments in
> procedure-pointer components, which does not properly account for the
> fact that the PASS argument needs to be polymorphic.
> 
> [The reason for this issue is probably that PPCs were mostly
> implemented before polymorphism was available. The corresponding
> pass-arg checks for TBPs are ok.]
> 
> The patch also fixes an invalid test case (which was detected thanks
> to Neil Carlson). It regtests cleanly on x86_64-linux-gnu. Ok for
> trunk?

Janus,

The patch looks ok to me.  Trunk is in regression and doc
fixes only mode, so you'll probably need to ping Jakub or
Richard (ie., release engineer) for an ok. 

PS: Welcome back!  We can definitely use your talent.
Paul Richard Thomas Feb. 10, 2018, 11:46 a.m. | #2
Hi Janus,

As Steve said, welcome back!

I hope that you will post the news of this fix and the correction of
the testcases on clf. Talking of which, have you posted the problems
that others have found as PRs?

It was one of my long deferred tasks to make a start on validating the
testsuite and to remove non-standard features.

Thanks for the patch.

Paul


On 9 February 2018 at 23:21, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
> On Fri, Feb 09, 2018 at 06:13:34PM +0100, Janus Weil wrote:
>>
>> the attached patch fixes some checking code for PASS arguments in
>> procedure-pointer components, which does not properly account for the
>> fact that the PASS argument needs to be polymorphic.
>>
>> [The reason for this issue is probably that PPCs were mostly
>> implemented before polymorphism was available. The corresponding
>> pass-arg checks for TBPs are ok.]
>>
>> The patch also fixes an invalid test case (which was detected thanks
>> to Neil Carlson). It regtests cleanly on x86_64-linux-gnu. Ok for
>> trunk?
>
> Janus,
>
> The patch looks ok to me.  Trunk is in regression and doc
> fixes only mode, so you'll probably need to ping Jakub or
> Richard (ie., release engineer) for an ok.
>
> PS: Welcome back!  We can definitely use your talent.
>
> --
> Steve
Janus Weil Feb. 11, 2018, 1:42 p.m. | #3
Hi guys,

2018-02-10 12:46 GMT+01:00 Paul Richard Thomas <paul.richard.thomas@gmail.com>:
> As Steve said, welcome back!

thanks for the warm welcome :)


> I hope that you will post the news of this fix and the correction of
> the testcases on clf. Talking of which, have you posted the problems
> that others have found as PRs?

Well, I have opened PR 84094 as a collector for such things (and a few
dependencies). Neil Carson has put a great amount of work into
analyzing some of the issues that came up and compiled a very helpful
list at:

https://github.com/nncarlson/gfortran.dg/issues

I have not sifted through the extensive c.l.f. thread (which may
contain further possible issues posted by people who lack the skills
to use any kind of bug tracker), any I don't have the intention
(read:time) to do that.


> It was one of my long deferred tasks to make a start on validating the
> testsuite and to remove non-standard features.

The biggest issue in this respect is probably the frequent use of the
non-std "call abort". All of those could simply be replaced by "stop
1" AFAICS, and if there is consensus that this is a useful thing to
do, I'd be willing to take care of it. (Feedback welcome here).

Cheers,
Janus
Janus Weil Feb. 11, 2018, 9:44 p.m. | #4
Dear release managers,

2018-02-10 0:21 GMT+01:00 Steve Kargl <sgk@troutmask.apl.washington.edu>:
> On Fri, Feb 09, 2018 at 06:13:34PM +0100, Janus Weil wrote:
>>
>> the attached patch fixes some checking code for PASS arguments in
>> procedure-pointer components, which does not properly account for the
>> fact that the PASS argument needs to be polymorphic.
>>
>> [The reason for this issue is probably that PPCs were mostly
>> implemented before polymorphism was available. The corresponding
>> pass-arg checks for TBPs are ok.]
>>
>> The patch also fixes an invalid test case (which was detected thanks
>> to Neil Carlson). It regtests cleanly on x86_64-linux-gnu. Ok for
>> trunk?
>
> The patch looks ok to me.  Trunk is in regression and doc
> fixes only mode, so you'll probably need to ping Jakub or
> Richard (ie., release engineer) for an ok.

would you mind if I applied this patch to trunk at the current stage?
It was approved by Steve and Paul, is rather simple and low-risk ...

Cheers,
Janus
Richard Biener Feb. 12, 2018, 7:22 a.m. | #5
On Sun, 11 Feb 2018, Janus Weil wrote:

> Dear release managers,
> 
> 2018-02-10 0:21 GMT+01:00 Steve Kargl <sgk@troutmask.apl.washington.edu>:
> > On Fri, Feb 09, 2018 at 06:13:34PM +0100, Janus Weil wrote:
> >>
> >> the attached patch fixes some checking code for PASS arguments in
> >> procedure-pointer components, which does not properly account for the
> >> fact that the PASS argument needs to be polymorphic.
> >>
> >> [The reason for this issue is probably that PPCs were mostly
> >> implemented before polymorphism was available. The corresponding
> >> pass-arg checks for TBPs are ok.]
> >>
> >> The patch also fixes an invalid test case (which was detected thanks
> >> to Neil Carlson). It regtests cleanly on x86_64-linux-gnu. Ok for
> >> trunk?
> >
> > The patch looks ok to me.  Trunk is in regression and doc
> > fixes only mode, so you'll probably need to ping Jakub or
> > Richard (ie., release engineer) for an ok.
> 
> would you mind if I applied this patch to trunk at the current stage?
> It was approved by Steve and Paul, is rather simple and low-risk ...

Go ahead.

Richard.
Janus Weil Feb. 12, 2018, 5:14 p.m. | #6
2018-02-12 8:22 GMT+01:00 Richard Biener <rguenther@suse.de>:
>> 2018-02-10 0:21 GMT+01:00 Steve Kargl <sgk@troutmask.apl.washington.edu>:
>> > On Fri, Feb 09, 2018 at 06:13:34PM +0100, Janus Weil wrote:
>> >>
>> >> the attached patch fixes some checking code for PASS arguments in
>> >> procedure-pointer components, which does not properly account for the
>> >> fact that the PASS argument needs to be polymorphic.
>> >>
>> >> [The reason for this issue is probably that PPCs were mostly
>> >> implemented before polymorphism was available. The corresponding
>> >> pass-arg checks for TBPs are ok.]
>> >>
>> >> The patch also fixes an invalid test case (which was detected thanks
>> >> to Neil Carlson). It regtests cleanly on x86_64-linux-gnu. Ok for
>> >> trunk?
>> >
>> > The patch looks ok to me.  Trunk is in regression and doc
>> > fixes only mode, so you'll probably need to ping Jakub or
>> > Richard (ie., release engineer) for an ok.
>>
>> would you mind if I applied this patch to trunk at the current stage?
>> It was approved by Steve and Paul, is rather simple and low-risk ...
>
> Go ahead.


Thanks! Committed as r257590.

Cheers,
Janus

Patch

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 257498)
+++ gcc/fortran/resolve.c	(working copy)
@@ -13703,8 +13703,8 @@  resolve_component (gfc_component *c, gfc_symbol *s
           return false;
         }
 
-      /* Check for C453.  */
-      if (me_arg->attr.dimension)
+      /* Check for F03:C453.  */
+      if (CLASS_DATA (me_arg)->attr.dimension)
         {
           gfc_error ("Argument %qs of %qs with PASS(%s) at %L "
                      "must be scalar", me_arg->name, c->name, me_arg->name,
@@ -13713,7 +13713,7 @@  resolve_component (gfc_component *c, gfc_symbol *s
           return false;
         }
 
-      if (me_arg->attr.pointer)
+      if (CLASS_DATA (me_arg)->attr.class_pointer)
         {
           gfc_error ("Argument %qs of %qs with PASS(%s) at %L "
                      "may not have the POINTER attribute", me_arg->name,
@@ -13722,7 +13722,7 @@  resolve_component (gfc_component *c, gfc_symbol *s
           return false;
         }
 
-      if (me_arg->attr.allocatable)
+      if (CLASS_DATA (me_arg)->attr.allocatable)
         {
           gfc_error ("Argument %qs of %qs with PASS(%s) at %L "
                      "may not be ALLOCATABLE", me_arg->name, c->name,
Index: gcc/testsuite/gfortran.dg/proc_ptr_47.f90
===================================================================
--- gcc/testsuite/gfortran.dg/proc_ptr_47.f90	(revision 257498)
+++ gcc/testsuite/gfortran.dg/proc_ptr_47.f90	(working copy)
@@ -21,13 +21,9 @@ 
 
 contains
   function foo(A)
-    class(AA), allocatable :: A
+    class(AA) :: A
     type(AA) foo
 
-    if (.not.allocated (A)) then
-      allocate (A, source = AA (2, foo))
-    endif
-
     select type (A)
       type is (AA)
         foo = AA (3, foo)
Index: gcc/testsuite/gfortran.dg/proc_ptr_comp_pass_4.f90
===================================================================
--- gcc/testsuite/gfortran.dg/proc_ptr_comp_pass_4.f90	(revision 257498)
+++ gcc/testsuite/gfortran.dg/proc_ptr_comp_pass_4.f90	(working copy)
@@ -37,22 +37,23 @@  module m
 
  type :: t8
    procedure(foo8), pass, pointer :: f8  ! { dg-error "must be of the derived type" }
+   procedure(foo9), pass, pointer :: f9  ! { dg-error "Non-polymorphic passed-object dummy argument" }
  end type
 
 contains
 
  subroutine foo1 (x1,y1)
-  type(t1) :: x1(:)
+  class(t1) :: x1(:)
   type(t1) :: y1
  end subroutine
 
  subroutine foo2 (x2,y2)
-  type(t2),pointer :: x2
+  class(t2),pointer :: x2
   type(t2) :: y2
  end subroutine
 
  subroutine foo3 (x3,y3)
-  type(t3),allocatable :: x3
+  class(t3),allocatable :: x3
   type(t3) :: y3
  end subroutine
 
@@ -69,4 +70,8 @@  contains
    integer :: i
  end function
 
+ subroutine foo9(x)
+   type(t8) :: x
+ end subroutine
+
 end module m