[Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]

Hi all,

as the subject line says, the attached patch rejects procedure
pointers in COMMON blocks (which is forbidden in F08). Since it's
apparently legal in F03, I'm still accepting it with -std=f2003 and
add that flag to a test case where this 'feature' is used. In another
one, I'm adding the error message that one gets with -std=f2008.

As my last submission, this fixes fallout from
https://groups.google.com/forum/?fromgroups#!topic/comp.lang.fortran/AIHRQ2kJv3c.
As the last one, it is a very simple fix for an accepts-invalid
problem (which is not a regression), so I hope this one will also
still be suitable for trunk (if not, I hope the release managers, in
CC, will stop me).

It does regtest cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


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

    PR fortran/84313
    * symbol.c (check_conflict): Reject procedure pointers in common blocks.


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

    PR fortran/84313
    * gfortran.dg/proc_ptr_common_1.f90: Fix invalid test case,
    add necessary compiler options.
    * gfortran.dg/proc_ptr_common_2.f90: Add missing error message.

[-- Attachment #2: pr84313.diff --]
[-- Type: text/plain, Size: 1873 bytes --]

Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 257589)
+++ gcc/fortran/symbol.c	(working copy)
@@ -809,7 +809,9 @@ check_conflict (symbol_attribute *attr, const char
 	    conf2 (threadprivate);
 	}
 
-      if (!attr->proc_pointer)
+      /* Procedure pointers in COMMON blocks are allowed in F03,
+       * but forbidden per F08:C5100.  */
+      if (!attr->proc_pointer || (gfc_option.allow_std & GFC_STD_F2008))
 	conf2 (in_common);
 
       conf2 (omp_declare_target_link);
Index: gcc/testsuite/gfortran.dg/proc_ptr_common_1.f90
===================================================================
--- gcc/testsuite/gfortran.dg/proc_ptr_common_1.f90	(revision 257589)
+++ gcc/testsuite/gfortran.dg/proc_ptr_common_1.f90	(working copy)
@@ -1,16 +1,18 @@
 ! { dg-do run }
-
+! { dg-options "-std=f2003 -fall-intrinsics" }
+!
 ! PR fortran/36592
 !
 ! Procedure Pointers inside COMMON blocks.
+! (Allowed in F03, but forbidden in F08.)
 !
 ! Contributed by Janus Weil <janus@gcc.gnu.org>.
 
 subroutine one()
   implicit none
-  common /com/ p1,p2,a,b
   procedure(real), pointer :: p1,p2
   integer :: a,b
+  common /com/ p1,p2,a,b
   if (a/=5 .or. b/=-9 .or. p1(0.0)/=1.0 .or. p2(0.0)/=0.0) call abort()
 end subroutine one
 
Index: gcc/testsuite/gfortran.dg/proc_ptr_common_2.f90
===================================================================
--- gcc/testsuite/gfortran.dg/proc_ptr_common_2.f90	(revision 257589)
+++ gcc/testsuite/gfortran.dg/proc_ptr_common_2.f90	(working copy)
@@ -12,7 +12,7 @@ abstract interface
 end interface
 
 procedure(foo), pointer, bind(C) :: proc
-common /com/ proc,r
+common /com/ proc,r  ! { dg-error "PROCEDURE attribute conflicts with COMMON attribute" }
 
 common s
 call s()  ! { dg-error "PROCEDURE attribute conflicts with COMMON attribute" }

Re: [Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Steve Kargl]

On Tue, Feb 13, 2018 at 07:24:35PM +0100, Janus Weil wrote:
> 
> As my last submission, this fixes fallout from
> https://groups.google.com/forum/?fromgroups#!topic/comp.lang.fortran/AIHRQ2kJv3c.
> As the last one, it is a very simple fix for an accepts-invalid
> problem (which is not a regression), so I hope this one will also
> still be suitable for trunk (if not, I hope the release managers, in
> CC, will stop me).
> 
> It does regtest cleanly on x86_64-linux-gnu. Ok for trunk?
> 

Ok.  I think you can skip waiting RM approval.  To me, this patch
falls into the "too simply and obvious" fix category.

PS: Given the response I got yesterday on the #gcc IRC channel
when I asked about the current status of trunk, I'm inclined to
ignore the "regression and doc fixes only" status.
 
-- 
steve 

Re: [Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Richard Biener]

On February 13, 2018 8:14:33 PM GMT+01:00, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote:
>On Tue, Feb 13, 2018 at 07:24:35PM +0100, Janus Weil wrote:
>> 
>> As my last submission, this fixes fallout from
>>
>https://groups.google.com/forum/?fromgroups#!topic/comp.lang.fortran/AIHRQ2kJv3c.
>> As the last one, it is a very simple fix for an accepts-invalid
>> problem (which is not a regression), so I hope this one will also
>> still be suitable for trunk (if not, I hope the release managers, in
>> CC, will stop me).
>> 
>> It does regtest cleanly on x86_64-linux-gnu. Ok for trunk?
>> 
>
>Ok.  I think you can skip waiting RM approval.  To me, this patch
>falls into the "too simply and obvious" fix category.
>
>PS: Given the response I got yesterday on the #gcc IRC channel
>when I asked about the current status of trunk, I'm inclined to
>ignore the "regression and doc fixes only" status.

Heh! Fortran is not release critical so if it's broken we'll release anyway. Which means it's up to frontend maintainers to decide what they accept at this stage. 

Extrapolating from last years I would expect GCC 8 to be released mid April. 

Richard. 


No

Re: [Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Janus Weil]

2018-02-13 20:55 GMT+01:00 Richard Biener <rguenther@suse.de>:
> On February 13, 2018 8:14:33 PM GMT+01:00, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote:
>>On Tue, Feb 13, 2018 at 07:24:35PM +0100, Janus Weil wrote:
>>>
>>> As my last submission, this fixes fallout from
>>>
>>https://groups.google.com/forum/?fromgroups#!topic/comp.lang.fortran/AIHRQ2kJv3c.
>>> As the last one, it is a very simple fix for an accepts-invalid
>>> problem (which is not a regression), so I hope this one will also
>>> still be suitable for trunk (if not, I hope the release managers, in
>>> CC, will stop me).
>>>
>>> It does regtest cleanly on x86_64-linux-gnu. Ok for trunk?
>>>
>>
>>Ok.

Thanks, Steve. Committed as r257636.


>>I think you can skip waiting RM approval.  To me, this patch
>>falls into the "too simply and obvious" fix category.
>>
>>PS: Given the response I got yesterday on the #gcc IRC channel
>>when I asked about the current status of trunk, I'm inclined to
>>ignore the "regression and doc fixes only" status.
>
> Heh! Fortran is not release critical so if it's broken we'll release anyway. Which means it's up to frontend maintainers to decide what they accept at this stage.

This is how it was handled for earlier releases also.

Nevertheless there is also quite a number of Fortran regression we
should take care of. Will try to have a look on the weekend.

Cheers,
Janus

Re: [Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Jakub Jelinek]

On Tue, Feb 13, 2018 at 07:24:35PM +0100, Janus Weil wrote:
> Hi all,
> 
> as the subject line says, the attached patch rejects procedure
> pointers in COMMON blocks (which is forbidden in F08). Since it's
> apparently legal in F03, I'm still accepting it with -std=f2003 and
> add that flag to a test case where this 'feature' is used. In another
> one, I'm adding the error message that one gets with -std=f2008.
> 
> As my last submission, this fixes fallout from
> https://groups.google.com/forum/?fromgroups#!topic/comp.lang.fortran/AIHRQ2kJv3c.
> As the last one, it is a very simple fix for an accepts-invalid
> problem (which is not a regression), so I hope this one will also
> still be suitable for trunk (if not, I hope the release managers, in
> CC, will stop me).
> 
> It does regtest cleanly on x86_64-linux-gnu. Ok for trunk?

This broke libgomp.fortran/threadprivate4.f90 test.
Adding ! { dg-additional-options "-std=f2003" }
doesn't work, because the test uses
  call abort
which is a GNU extension and I have no idea how to choose allow_std
which includes GNU but doesn't include F2008.

	Jakub

Re: [Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Jakub Jelinek]

On Wed, Feb 14, 2018 at 12:30:14PM +0100, Jakub Jelinek wrote:
> On Tue, Feb 13, 2018 at 07:24:35PM +0100, Janus Weil wrote:
> > as the subject line says, the attached patch rejects procedure
> > pointers in COMMON blocks (which is forbidden in F08). Since it's
> > apparently legal in F03, I'm still accepting it with -std=f2003 and
> > add that flag to a test case where this 'feature' is used. In another
> > one, I'm adding the error message that one gets with -std=f2008.
> > 
> > As my last submission, this fixes fallout from
> > https://groups.google.com/forum/?fromgroups#!topic/comp.lang.fortran/AIHRQ2kJv3c.
> > As the last one, it is a very simple fix for an accepts-invalid
> > problem (which is not a regression), so I hope this one will also
> > still be suitable for trunk (if not, I hope the release managers, in
> > CC, will stop me).
> > 
> > It does regtest cleanly on x86_64-linux-gnu. Ok for trunk?
> 
> This broke libgomp.fortran/threadprivate4.f90 test.
> Adding ! { dg-additional-options "-std=f2003" }
> doesn't work, because the test uses
>   call abort
> which is a GNU extension and I have no idea how to choose allow_std
> which includes GNU but doesn't include F2008.

! { dg-additional-options "-std=f2003 -fdec" }

seems to work (because -std=f2003 sets
      gfc_option.allow_std = GFC_STD_F95_OBS | GFC_STD_F77
        | GFC_STD_F2003 | GFC_STD_F95 | GFC_STD_F2008_OBS;
and -fdec adds:
      gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
        | GFC_STD_GNU | GFC_STD_LEGACY;
), but it is quite nasty.  Isn't there a better way?

Kind like -std=gnu++17 vs. -std=c++17 where the latter is standard
and former standard + GNU extensions (which would roughly be
"| GFC_STD_GNU | GFC_STD_LEGACY" in the fortran world).

	Jakub

Re: [Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Janus Weil]

2018-02-14 12:47 GMT+01:00 Jakub Jelinek <jakub@redhat.com>:
> On Wed, Feb 14, 2018 at 12:30:14PM +0100, Jakub Jelinek wrote:
>> On Tue, Feb 13, 2018 at 07:24:35PM +0100, Janus Weil wrote:
>> > as the subject line says, the attached patch rejects procedure
>> > pointers in COMMON blocks (which is forbidden in F08). Since it's
>> > apparently legal in F03, I'm still accepting it with -std=f2003 and
>> > add that flag to a test case where this 'feature' is used. In another
>> > one, I'm adding the error message that one gets with -std=f2008.
>> >
>> > As my last submission, this fixes fallout from
>> > https://groups.google.com/forum/?fromgroups#!topic/comp.lang.fortran/AIHRQ2kJv3c.
>> > As the last one, it is a very simple fix for an accepts-invalid
>> > problem (which is not a regression), so I hope this one will also
>> > still be suitable for trunk (if not, I hope the release managers, in
>> > CC, will stop me).
>> >
>> > It does regtest cleanly on x86_64-linux-gnu. Ok for trunk?
>>
>> This broke libgomp.fortran/threadprivate4.f90 test.

Sorry about that! I regtested with "make check-gfortran", which
doesn't run the libgomp tests, I guess?


>> Adding ! { dg-additional-options "-std=f2003" }
>> doesn't work, because the test uses
>>   call abort

I actually think we should get rid of such extensions in the
testsuite, where possible. This particular one is used all over the
place, but could be easily replaces by something like "stop 1", which
is standard Fortran.


>> which is a GNU extension and I have no idea how to choose allow_std
>> which includes GNU but doesn't include F2008.
>
> ! { dg-additional-options "-std=f2003 -fdec" }
>
> seems to work (because -std=f2003 sets
>       gfc_option.allow_std = GFC_STD_F95_OBS | GFC_STD_F77
>         | GFC_STD_F2003 | GFC_STD_F95 | GFC_STD_F2008_OBS;
> and -fdec adds:
>       gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
>         | GFC_STD_GNU | GFC_STD_LEGACY;
> ), but it is quite nasty.  Isn't there a better way?

Yes, there is "-std=f2003 -fall-intrinsics", which is a little better
at least. It's what I did with proc_ptr_common1.f90 as well ...

https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/testsuite/gfortran.dg/proc_ptr_common_1.f90?r1=257636&r2=257635&pathrev=257636


> Kind like -std=gnu++17 vs. -std=c++17 where the latter is standard
> and former standard + GNU extensions (which would roughly be
> "| GFC_STD_GNU | GFC_STD_LEGACY" in the fortran world).

Huh, I suppose it would be nice to have options like -std=gnu2003 and
-std=gnu2008, in analogy to those C++ options ...

Cheers,
Janus

Re: [Patch, Fortran, F08] PR 84313: reject procedure pointers in COMMON blocks

[Janus Weil]

>>> Adding ! { dg-additional-options "-std=f2003" }
>>> doesn't work, because the test uses
>>>   call abort
>
> I actually think we should get rid of such extensions in the
> testsuite, where possible. This particular one is used all over the
> place, but could be easily replaces by something like "stop 1", which
> is standard Fortran.

Just opened a PR for this:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84381


>>> which is a GNU extension and I have no idea how to choose allow_std
>>> which includes GNU but doesn't include F2008.
>>
>> ! { dg-additional-options "-std=f2003 -fdec" }
>>
>> seems to work (because -std=f2003 sets
>>       gfc_option.allow_std = GFC_STD_F95_OBS | GFC_STD_F77
>>         | GFC_STD_F2003 | GFC_STD_F95 | GFC_STD_F2008_OBS;
>> and -fdec adds:
>>       gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
>>         | GFC_STD_GNU | GFC_STD_LEGACY;
>> ), but it is quite nasty.  Isn't there a better way?
>
> Yes, there is "-std=f2003 -fall-intrinsics", which is a little better
> at least. It's what I did with proc_ptr_common1.f90 as well ...
>
> https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/testsuite/gfortran.dg/proc_ptr_common_1.f90?r1=257636&r2=257635&pathrev=257636

Thanks for taking care of fixing this so quickly, Jakub (and for
noticing it in the first place)!


>> Kind like -std=gnu++17 vs. -std=c++17 where the latter is standard
>> and former standard + GNU extensions (which would roughly be
>> "| GFC_STD_GNU | GFC_STD_LEGACY" in the fortran world).
>
> Huh, I suppose it would be nice to have options like -std=gnu2003 and
> -std=gnu2008, in analogy to those C++ options ...

This is now:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84382

Cheers,
Janus