[Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement

[Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement

[Janus Weil]

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

Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement

[Bernhard Reutner-Fischer]

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

Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement

[Bernhard Reutner-Fischer]

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  

Re: [Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement

[Bernhard Reutner-Fischer]

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,