fortran-src icon indicating copy to clipboard operation
fortran-src copied to clipboard

Construct SrcSpans in Blocks

Open burz opened this issue 5 years ago • 7 comments

Note: I am going to present this issue in the context of BlIf but I am pretty sure it applies to other blocks as well.

Currently BlIf construct has a few SrcSpans that we can utilize

  1. The overall SrcSpan of the block from if to endif.
  2. The SrcSpan of each individual condition via the Maybe (Expression a)s so long as the conditions are actually present.
  3. The SrcSpans of the inner blocks that correspond to these conditions via [[Block a]].

This covers 99% of the cases where SrcSpans would be needed, but there is a case where these are not sufficient.

Suppose I am creating a tool to rewrite BlIf code using fortran-src. First consider a simple example

      if (cond) then
       print *, 'yes'
      else
       print *, 'no'
       print *, 'maybe so'
      endif

If I know that cond is false, I would like to rewrite this as

       print *, 'no'
       print *, 'maybe so'

This is simple enough to do with the SrcSpan of the overall BlIf -- call this ifSS -- combined with the SrcSpan of the else branch (i.e. the SrcSpan the final element in [[Block a]]) that we'll call blockSS, by rewriting from the beginning of ifSS to the beginning of blockSS and then from the end of blockSS to the end of ifSS.

On the other hand if we have comments in our code

      if (cond) then
c     A comment for this branch
       print *, 'yes'
      else
c     A very important and illuminating comment
       print *, 'no'
       print *, 'maybe so'
      endif

we would certainly like to keep these comments around!

c     A very important and illuminating comment
       print *, 'no'
       print *, 'maybe so'

But our previous method of rewriting no longer works since comments are not included within [[Block a]]. Furthermore, there there does not seem to be a reasonable way to determine where the constructs like if (cond) then, else if (cond2) then, else, and endif begin and end, due to comments, line continuations, or similar.

In order for this sort of thing to work correctly it would seem that the SrcSpans of the actual constructs mentioned above to be included somehow within the types of the blocks since they are certainly known to the parser.

I am creating this issue to determine what the best course of action is. I would be willing to do the work to make this work (and perhaps not in this way if there is some better/more elegant way), but what I think might work would be to add another field [SrcSpan] to BlIf where length [[Block a]] + 1 = length [SrcSpan], i.e. [SrcSpan] includes one additional span corresponding to endif. Regardless of how we see fit to proceed, I believe this will be a rather large code change so I want to ensure I am going in the right direction before starting off.

@mrd @ruoso

burz avatar Apr 16 '19 16:04 burz

That sounds reasonable, I'm away for the next 5 days so I'll have to take a closer look next week; sorry!

mrd avatar Apr 17 '19 13:04 mrd

Can you clarify what you meant by

since comments are not included within [[Block a]]

They should be, as AST node BlComment.

What may not be included is any line-comments that share the same line with a piece of code such as if or endif, however that is not an issue in Fortran 77 at all unless you are using non-standard comments.

mrd avatar Apr 22 '19 19:04 mrd

Through my own experience, I haven't been seeing the comments included in the [[Block a]] within the BlIf, so I created a test! https://github.com/camfort/fortran-src/compare/master...burz:77legacy-comments

Indeed, the comments are included for the fortran77Parser, but not for the legacy77Parser. I believe that this is due to them being free and fixed form, respectively.

Any thoughts on a way to tackle this? Or will this likely never be handled due to continuation lines?

I can understand that it would be difficult to pull out a comment like

      a =
c     Some comment I decided to put here for some reason
     + b - 43543

but I'm surprised that comments like

c     I am a strong, independent comment
      a = b - 43543

wouldn't be included.

burz avatar May 03 '19 19:05 burz

The reason is that there is a case test in the fixed form lexer that explicitly drops all comments if the legacy parser is running. I do not know the reason this test was inserted so I've preserved it for the time being.

On Fri, 3 May 2019, 20:29 Anthony Burzillo, [email protected] wrote:

Through my own experience, I haven't been seeing the comments included in the [[Block a]] within the BlIf, so I created a test! master...burz:77legacy-comments https://github.com/camfort/fortran-src/compare/master...burz:77legacy-comments

Indeed, the comments are included for the fortran77Parser, but not for the legacy77Parser. I believe that this is due to them being free and fixed form, respectively.

Any thoughts on a way to tackle this? Or will this likely never be handled due to continuation lines?

I can understand that it would be difficult to pull out a comment like

  a =

c Some comment I decided to put here for some reason + b - 43543

but I'm surprised that comments like

c I am a strong, independent comment a = b - 43543

wouldn't be included.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/camfort/fortran-src/issues/102#issuecomment-489213069, or mute the thread https://github.com/notifications/unsubscribe-auth/AACCMOECGOCEIGUZN2SXSYDPTSHAJANCNFSM4HGLXFDQ .

mrd avatar May 03 '19 23:05 mrd

@mrd @burz any progress with a coming up with a proposal for addressing this? I could also use this information in a refactoring tool .. specifically in my case to determine the SrcSpan of the else.

ccotter avatar Jul 23 '19 03:07 ccotter

Does https://github.com/camfort/fortran-src/pull/103 help you, @ccotter?

mrd avatar Jul 23 '19 13:07 mrd

ya - it does help my case. thanks!

ccotter avatar Aug 05 '19 00:08 ccotter