* [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement @ 2018-08-13 19:45 Janus Weil [not found] ` <9F1BA124-0E74-496E-9AAC-53C86052CF5F@gmail.com> 0 siblings, 1 reply; 11+ messages in thread From: Janus Weil @ 2018-08-13 19:45 UTC (permalink / raw) To: gfortran, gcc-patches Hi all, this simple patch improves some of the diagnostics for invalid ASSOCIATE statements: https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006 In particular it gives a more precise locus and improved wording, which is achieved basically by splitting a 'gfc_match' into two separate ones. Regtests cleanly on x86_64-linux-gnu. Ok for trunk? Cheers, Janus ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <9F1BA124-0E74-496E-9AAC-53C86052CF5F@gmail.com>]
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement [not found] ` <9F1BA124-0E74-496E-9AAC-53C86052CF5F@gmail.com> @ 2018-08-14 19:41 ` Janus Weil 2018-08-15 14:09 ` Fritz Reese ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Janus Weil @ 2018-08-14 19:41 UTC (permalink / raw) To: Bernhard Reutner-Fischer, gfortran [-- Attachment #1: Type: text/plain, Size: 818 bytes --] Am Di., 14. Aug. 2018 um 19:44 Uhr schrieb Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>: > > Janus, > > On 13 August 2018 21:44:47 CEST, Janus Weil <janus@gcc.gnu.org> wrote: > >Hi all, > > > >this simple patch improves some of the diagnostics for invalid > >ASSOCIATE statements: > > > >https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006 > > Please do not send external references but the patch itself for posterity. "git diff pr86935~1 pr86935 > pr86935.diff" See attachment. I think gcc would profit massively not only from having its primary repository in git, but also having a proper review system (github/gitlab give great examples, but there are lots of others by now). Sending patches to mailing lists is so archaic and cumbersome. Just my opinion, though ... Cheers, Janus [-- Attachment #2: pr86935.diff --] [-- Type: text/x-patch, Size: 2912 bytes --] diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog index 6c39d9cc5d5..fef3a021981 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,9 @@ +2018-08-13 Janus Weil <janus@gcc.gnu.org> + + PR fortran/86935 + * match.c (gfc_match_associate): Improve diagnostics for the ASSOCIATE + statement. + 2018-08-12 Paul Thomas <pault@gcc.gnu.org> PR fortran/66679 diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c index 1ab0e0fad9a..b0cf6e48f9b 100644 --- a/gcc/fortran/match.c +++ b/gcc/fortran/match.c @@ -1889,15 +1889,19 @@ gfc_match_associate (void) gfc_association_list* a; /* Match the next association. */ - if (gfc_match (" %n => %e", newAssoc->name, &newAssoc->target) - != MATCH_YES) + if (gfc_match (" %n => ", newAssoc->name) != MATCH_YES) + { + gfc_error ("Expected association at %C"); + goto assocListError; + } + + if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) { /* Have another go, allowing for procedure pointer selectors. */ gfc_matching_procptr_assignment = 1; - if (gfc_match (" %n => %e", newAssoc->name, &newAssoc->target) - != MATCH_YES) + if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) { - gfc_error ("Expected association at %C"); + gfc_error ("Invalid association target at %C"); goto assocListError; } gfc_matching_procptr_assignment = 0; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index ad2610b91b5..f7cfc447a5b 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2018-08-13 Janus Weil <janus@gcc.gnu.org> + + PR fortran/86935 + * gfortran.dg/associate_3.f90: Update error message. + * gfortran.dg/associate_39.f90: New test case. + 2018-08-13 Martin Sebor <msebor@redhat.com> PR tree-optimization/71625 diff --git a/gcc/testsuite/gfortran.dg/associate_3.f03 b/gcc/testsuite/gfortran.dg/associate_3.f03 index 20a375dcfd1..da7bec951d1 100644 --- a/gcc/testsuite/gfortran.dg/associate_3.f03 +++ b/gcc/testsuite/gfortran.dg/associate_3.f03 @@ -13,7 +13,7 @@ PROGRAM main ASSOCIATE (a => 1) 5 ! { dg-error "Junk after ASSOCIATE" } - ASSOCIATE (x =>) ! { dg-error "Expected association" } + ASSOCIATE (x =>) ! { dg-error "Invalid association target" } ASSOCIATE (=> 5) ! { dg-error "Expected association" } diff --git a/gcc/testsuite/gfortran.dg/associate_39.f90 b/gcc/testsuite/gfortran.dg/associate_39.f90 new file mode 100644 index 00000000000..16357c32777 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/associate_39.f90 @@ -0,0 +1,19 @@ +! { dg-do compile } +! +! PR 86935: Bad locus in ASSOCIATE statement +! +! Contributed by Janus Weil <janus@gcc.gnu.org> + +implicit none + +type :: t + real :: r = 0.5 + integer :: i = 3 +end type + +type(t) :: x + +associate (r => x%r, & + i => x%ii) ! { dg-error "Invalid association target" } + +end ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2018-08-14 19:41 ` Janus Weil @ 2018-08-15 14:09 ` Fritz Reese 2018-08-21 16:20 ` Janus Weil 2018-08-22 15:46 ` Thomas Koenig 2 siblings, 0 replies; 11+ messages in thread From: Fritz Reese @ 2018-08-15 14:09 UTC (permalink / raw) To: Janus Weil; +Cc: fortran On Tue, Aug 14, 2018 at 3:41 PM Janus Weil <janus@gcc.gnu.org> wrote: > [...] > I think gcc would profit massively not only from having its primary > repository in git, but also having a proper review system > (github/gitlab give great examples, but there are lots of others by > now). Sending patches to mailing lists is so archaic and cumbersome. > > Just my opinion, though ... > I agree! ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2018-08-14 19:41 ` Janus Weil 2018-08-15 14:09 ` Fritz Reese @ 2018-08-21 16:20 ` Janus Weil 2018-08-22 13:07 ` Bernhard Reutner-Fischer 2018-08-22 15:46 ` Thomas Koenig 2 siblings, 1 reply; 11+ messages in thread From: Janus Weil @ 2018-08-21 16:20 UTC (permalink / raw) To: Bernhard Reutner-Fischer, gfortran ping! Am Di., 14. Aug. 2018 um 21:40 Uhr schrieb Janus Weil <janus@gcc.gnu.org>: > > > > > > >this simple patch improves some of the diagnostics for invalid > > >ASSOCIATE statements: > > > > > >https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006 > > > > Please do not send external references but the patch itself for posterity. > > "git diff pr86935~1 pr86935 > pr86935.diff" > See attachment. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2018-08-21 16:20 ` Janus Weil @ 2018-08-22 13:07 ` Bernhard Reutner-Fischer 0 siblings, 0 replies; 11+ messages in thread From: Bernhard Reutner-Fischer @ 2018-08-22 13:07 UTC (permalink / raw) To: Janus Weil, gfortran On 21 August 2018 18:20:08 CEST, Janus Weil <janus@gcc.gnu.org> wrote: >ping! > > >Am Di., 14. Aug. 2018 um 21:40 Uhr schrieb Janus Weil ><janus@gcc.gnu.org>: >> >> > > >> > >this simple patch improves some of the diagnostics for invalid >> > >ASSOCIATE statements: >> > > >> > >>https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006 >> > >> > Please do not send external references but the patch itself for >posterity. >> >> "git diff pr86935~1 pr86935 > pr86935.diff" >> See attachment. +++ b/gcc/fortran/match.c @@ -1889,15 +1889,19 @@ gfc_match_associate (void) gfc_association_list* a; /* Match the next association. */ - if (gfc_match (" %n => %e", newAssoc->name, &newAssoc->target) - != MATCH_YES) + if (gfc_match (" %n => ", newAssoc->name) != MATCH_YES) Since you match " %e", I.e. with leading optional space, I'd omit the trailing blank in " %n => ". With that change the patch looks ok to me, but I cannot approve it. Cheers, + { + gfc_error ("Expected association at %C"); + goto assocListError; + } + + if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) { ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2018-08-14 19:41 ` Janus Weil 2018-08-15 14:09 ` Fritz Reese 2018-08-21 16:20 ` Janus Weil @ 2018-08-22 15:46 ` Thomas Koenig 2018-08-22 19:37 ` Janus Weil 2 siblings, 1 reply; 11+ messages in thread From: Thomas Koenig @ 2018-08-22 15:46 UTC (permalink / raw) To: Janus Weil, Bernhard Reutner-Fischer, gfortran Hi Janus, >> Janus, >> >> On 13 August 2018 21:44:47 CEST, Janus Weil <janus@gcc.gnu.org> wrote: >>> Hi all, >>> >>> this simple patch improves some of the diagnostics for invalid >>> ASSOCIATE statements: >>> >>> https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006 >> >> Please do not send external references but the patch itself for posterity. > > "git diff pr86935~1 pr86935 > pr86935.diff" > See attachment. The patch is OK; you might want to take Bernhard's remark about the trailing space after "%e" into account. Regards Thomas ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2018-08-22 15:46 ` Thomas Koenig @ 2018-08-22 19:37 ` Janus Weil 2018-09-02 15:16 ` Bernhard Reutner-Fischer 0 siblings, 1 reply; 11+ messages in thread From: Janus Weil @ 2018-08-22 19:37 UTC (permalink / raw) To: Thomas Koenig; +Cc: Bernhard Reutner-Fischer, gfortran Am Mi., 22. Aug. 2018 um 17:46 Uhr schrieb Thomas Koenig <tkoenig@netcologne.de>: > > Hi Janus, > > >> Janus, > >> > >> On 13 August 2018 21:44:47 CEST, Janus Weil <janus@gcc.gnu.org> wrote: > >>> Hi all, > >>> > >>> this simple patch improves some of the diagnostics for invalid > >>> ASSOCIATE statements: > >>> > >>> https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006 > >> > >> Please do not send external references but the patch itself for posterity. > > > > "git diff pr86935~1 pr86935 > pr86935.diff" > > See attachment. > > The patch is OK; you might want to take Bernhard's remark about > the trailing space after "%e" into account. I have incorporated Bernhard's remark and committed as r263787. Thanks everyone! Cheers, Janus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2018-08-22 19:37 ` Janus Weil @ 2018-09-02 15:16 ` Bernhard Reutner-Fischer 2021-10-27 17:40 ` Bernhard Reutner-Fischer 0 siblings, 1 reply; 11+ messages in thread From: Bernhard Reutner-Fischer @ 2018-09-02 15:16 UTC (permalink / raw) To: Janus Weil; +Cc: Thomas Koenig, gfortran, GCC Patches On Wed, 22 Aug 2018 at 21:37, Janus Weil <janus@gcc.gnu.org> wrote: > > Am Mi., 22. Aug. 2018 um 17:46 Uhr schrieb Thomas Koenig > <tkoenig@netcologne.de>: > > > > Hi Janus, > > > > >> Janus, > > >> > > >> On 13 August 2018 21:44:47 CEST, Janus Weil <janus@gcc.gnu.org> wrote: > > >>> Hi all, > > >>> > > >>> this simple patch improves some of the diagnostics for invalid > > >>> ASSOCIATE statements: > > >>> > > >>> https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006 > > >> > > >> Please do not send external references but the patch itself for posterity. > > > > > > "git diff pr86935~1 pr86935 > pr86935.diff" > > > See attachment. > > > > The patch is OK; you might want to take Bernhard's remark about > > the trailing space after "%e" into account. > > I have incorporated Bernhard's remark and committed as r263787. While rebasing my fortran-fe-stringpool branch i spotted one (pre-existing) possible inconsistency that i did overlook back then: gfc_match_associate () reads ... if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) { /* Have another go, allowing for procedure pointer selectors. */ gfc_matching_procptr_assignment = 1; if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) { gfc_error ("Invalid association target at %C"); goto assocListError; } gfc_matching_procptr_assignment = 0; } i.e. we retry a match, but in the second attempt we turn on procptr assignment matching and if that works, we turn procptr assignment matching off again. But if we fail that retry, we forget to turn it off again. I suppose we should: $ svn diff -x -p gcc/fortran/match.c Index: gcc/fortran/match.c =================================================================== --- gcc/fortran/match.c (revision 264040) +++ gcc/fortran/match.c (working copy) @@ -1898,13 +1898,16 @@ gfc_match_associate (void) if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) { /* Have another go, allowing for procedure pointer selectors. */ + match m; + gfc_matching_procptr_assignment = 1; - if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) + m = gfc_match (" %e", &newAssoc->target); + gfc_matching_procptr_assignment = 0; + if (m != MATCH_YES) { gfc_error ("Invalid association target at %C"); goto assocListError; } - gfc_matching_procptr_assignment = 0; } newAssoc->where = gfc_current_locus; Untested. Maybe someone wants to give it a whirl... If it wrecks havoc then leaving it set deliberately deserves at least a comment. PS: It would be nice to get rid of gfc_matching_procptr_assignment, gfc_matching_ptr_assignment, gfc_matching_prefix, FWIW. cheers, > > Thanks everyone! > > Cheers, > Janus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2018-09-02 15:16 ` Bernhard Reutner-Fischer @ 2021-10-27 17:40 ` Bernhard Reutner-Fischer 2021-10-27 19:44 ` Harald Anlauf 0 siblings, 1 reply; 11+ messages in thread From: Bernhard Reutner-Fischer @ 2021-10-27 17:40 UTC (permalink / raw) To: Janus Weil; +Cc: rep.dot.nop, Thomas Koenig, gfortran, GCC Patches AFAICS current trunk still has this issue. Any takers? thanks, On Sun, 2 Sep 2018 17:16:07 +0200 Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > i spotted one > (pre-existing) possible inconsistency that i did overlook back then: > > gfc_match_associate () reads > ... > if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) > { > /* Have another go, allowing for procedure pointer selectors. */ > gfc_matching_procptr_assignment = 1; > if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) > { > gfc_error ("Invalid association target at %C"); > goto assocListError; > } > gfc_matching_procptr_assignment = 0; > } > > i.e. we retry a match, but in the second attempt we turn on procptr > assignment matching and if that works, we turn procptr assignment > matching off again. > But if we fail that retry, we forget to turn it off again. > I suppose we should: > > $ svn diff -x -p gcc/fortran/match.c > Index: gcc/fortran/match.c > =================================================================== > --- gcc/fortran/match.c (revision 264040) > +++ gcc/fortran/match.c (working copy) > @@ -1898,13 +1898,16 @@ gfc_match_associate (void) > if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) > { > /* Have another go, allowing for procedure pointer selectors. */ > + match m; > + > gfc_matching_procptr_assignment = 1; > - if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) > + m = gfc_match (" %e", &newAssoc->target); > + gfc_matching_procptr_assignment = 0; > + if (m != MATCH_YES) > { > gfc_error ("Invalid association target at %C"); > goto assocListError; > } > - gfc_matching_procptr_assignment = 0; > } > newAssoc->where = gfc_current_locus; > > > Untested. Maybe someone wants to give it a whirl... > If it wrecks havoc then leaving it set deliberately deserves at least a comment. > > PS: It would be nice to get rid of gfc_matching_procptr_assignment, > gfc_matching_ptr_assignment, gfc_matching_prefix, FWIW. > cheers, > > > > Thanks everyone! > > > > Cheers, > > Janus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2021-10-27 17:40 ` Bernhard Reutner-Fischer @ 2021-10-27 19:44 ` Harald Anlauf 2021-10-27 20:46 ` Bernhard Reutner-Fischer 0 siblings, 1 reply; 11+ messages in thread From: Harald Anlauf @ 2021-10-27 19:44 UTC (permalink / raw) To: fortran; +Cc: gcc-patches Hi Bernhard, Am 27.10.21 um 19:40 schrieb Bernhard Reutner-Fischer via Fortran: > AFAICS current trunk still has this issue. > Any takers? > thanks, can you create a PR tracking this issue? AFAICS PR86935 has been fixed for gcc-9+. Harald > On Sun, 2 Sep 2018 17:16:07 +0200 > Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > >> i spotted one >> (pre-existing) possible inconsistency that i did overlook back then: >> >> gfc_match_associate () reads >> ... >> if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) >> { >> /* Have another go, allowing for procedure pointer selectors. */ >> gfc_matching_procptr_assignment = 1; >> if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) >> { >> gfc_error ("Invalid association target at %C"); >> goto assocListError; >> } >> gfc_matching_procptr_assignment = 0; >> } >> >> i.e. we retry a match, but in the second attempt we turn on procptr >> assignment matching and if that works, we turn procptr assignment >> matching off again. >> But if we fail that retry, we forget to turn it off again. >> I suppose we should: >> >> $ svn diff -x -p gcc/fortran/match.c >> Index: gcc/fortran/match.c >> =================================================================== >> --- gcc/fortran/match.c (revision 264040) >> +++ gcc/fortran/match.c (working copy) >> @@ -1898,13 +1898,16 @@ gfc_match_associate (void) >> if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) >> { >> /* Have another go, allowing for procedure pointer selectors. */ >> + match m; >> + >> gfc_matching_procptr_assignment = 1; >> - if (gfc_match (" %e", &newAssoc->target) != MATCH_YES) >> + m = gfc_match (" %e", &newAssoc->target); >> + gfc_matching_procptr_assignment = 0; >> + if (m != MATCH_YES) >> { >> gfc_error ("Invalid association target at %C"); >> goto assocListError; >> } >> - gfc_matching_procptr_assignment = 0; >> } >> newAssoc->where = gfc_current_locus; >> >> >> Untested. Maybe someone wants to give it a whirl... >> If it wrecks havoc then leaving it set deliberately deserves at least a comment. >> >> PS: It would be nice to get rid of gfc_matching_procptr_assignment, >> gfc_matching_ptr_assignment, gfc_matching_prefix, FWIW. >> cheers, >>> >>> Thanks everyone! >>> >>> Cheers, >>> Janus > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement 2021-10-27 19:44 ` Harald Anlauf @ 2021-10-27 20:46 ` Bernhard Reutner-Fischer 0 siblings, 0 replies; 11+ messages in thread From: Bernhard Reutner-Fischer @ 2021-10-27 20:46 UTC (permalink / raw) To: Harald Anlauf via Fortran; +Cc: rep.dot.nop, Harald Anlauf, gcc-patches On Wed, 27 Oct 2021 21:44:52 +0200 Harald Anlauf via Fortran <fortran@gcc.gnu.org> wrote: > Hi Bernhard, > > Am 27.10.21 um 19:40 schrieb Bernhard Reutner-Fischer via Fortran: > > AFAICS current trunk still has this issue. > > Any takers? > > thanks, > > can you create a PR tracking this issue? now https://gcc.gnu.org/PR102973 > > AFAICS PR86935 has been fixed for gcc-9+. Yes, it is a pre existing possible bug that caught my eye when i looked at that patch, so admittedly unrelated to PR86935. thanks, ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-27 20:46 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-13 19:45 [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement Janus Weil [not found] ` <9F1BA124-0E74-496E-9AAC-53C86052CF5F@gmail.com> 2018-08-14 19:41 ` Janus Weil 2018-08-15 14:09 ` Fritz Reese 2018-08-21 16:20 ` Janus Weil 2018-08-22 13:07 ` Bernhard Reutner-Fischer 2018-08-22 15:46 ` Thomas Koenig 2018-08-22 19:37 ` Janus Weil 2018-09-02 15:16 ` Bernhard Reutner-Fischer 2021-10-27 17:40 ` Bernhard Reutner-Fischer 2021-10-27 19:44 ` Harald Anlauf 2021-10-27 20:46 ` Bernhard Reutner-Fischer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).