| Message ID | CAKwh3qhbdSontmz3XmNO9r9ec4OUfod+hk9U-k57Sv-o009Uvw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Fortran,F08] PR 84313: reject procedure pointers in COMMON blocks | expand |
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.
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
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
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
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
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
>>> 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
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" }
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.