search

camfort / fortran-src

Fortran parsing and static analysis infrastructure
https://hackage.haskell.org/package/fortran-src
Other
48 stars 20 forks source link

pair IF/CASE conditions with their blocks #217

Closed raehik closed 2 years ago

raehik commented 2 years ago

Previously, block IF and CASE constructs stored their conditions and blocks separately. To use them, you had to zip them together, but also handle the edge cases (ELSE, CASE DEFAULT). This change merges them into a list of tuples. It should be easier to consume them safely, and we get to simplify some code.

In some places, we do only want the blocks or the conditions e.g. the basic block analysis processes IF/ELSE IF conditions while ignoring the inner blocks. In such cases, we now have to write fmap {fst,snd} tuples. On the other hand, printing for these block constructs is now much easier to define, and we restrict our representation a bit more to what valid Fortran can look like.

raehik commented 2 years ago

I also de-duplicated more parser tests by moving into Parser.Free.Common.

raehik commented 2 years ago

@RaoulHC

Thanks for pointing this out & the help, the representation is much nicer than before. Can I get your thoughts?

raehik commented 2 years ago

During a meeting last month, it was noted that the split/interleaved representation used currently breaks "sequentiality": the bodies field comes after the conditions field, but the SrcSpans overlap (if one were to calculate them for the lists). This is relevant for Haskell Data generics, and the property is required by our reprinter algorithm.

It'd be nice to write tests for this, but the reprinter isn't bundled with fortran-src, and CamFort uses a fork. I'll look at reconciling them again, then I could add tests over in fortran-src-extras .

RaoulHC commented 2 years ago

Forgot to look over this before, but looks all sensible to me. Should hopefully simplify usage in various places.

Can't remember if it was mentioned but does the camfort version use the reprintSort alternative algorithm? Else I would have expected some pretty broken refactors if it ever tries to change if and case blocks, though maybe it just doesn't do that.

raehik commented 2 years ago

I don't think CamFort's reprinter has the alternative algorithm (it seems like a terse monomorphic version for fortran-src). I think it's as you say, the problematic constructs weren't reprinted / in ways where the problem surfaced.

raehik commented 2 years ago

Tracking reprinting-related work & tests elsewhere, merging this.