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

`Maybe AList` confuses representation and limits precise reprinting

Open raehik opened this issue 2 years ago • 2 comments

The Fortran AST makes heavy use of the AList type to attach metadata to a list as well as each of its elements. However, it often gets used as Maybe AList to provide an easy out for the empty case (where you could just as well use an empty list inside the AList). This has a few effects:

  • Constructing for the empty list case is easier: Nothing rather than adding all the AList metadata
  • Ambiguous empty representation: both Nothing and Just [] now exist
  • Can't reprint because the Nothing case doesn't store a SrcSpan
  • Can't treat as a plain list without unwrapping the Maybe (results in lots of fromMaybe [])

ALists are essentially a common piece of AST factored out - in particular, they don't map to any one piece of syntax. It would be possible to refactor ALists (or rather, add a bunch more) so that they store all their relevant syntactic information. e.g. some start with ,, some may not be bracketed when empty. That way, the type tells us more, and the pretty printing and reprinting typeclass instances can be simplified.

A sketch would be:

data AListX ext t a = AListX a SrcSpan [t a] ext

where ext would be instantiated as a Bool-like (e.g. data Brackets = Brackets | OmitBrackets) or something else.

raehik avatar Jul 13 '22 15:07 raehik

As a first step, I've done this for StCall and ExpFunctionCall, being two especially common AST nodes. The parsing is more fiddly, because you have to think about the empty space where the empty list "is" (would be?). I feel like that might be why the Maybe route was taken. But the end-user experience is better, since you don't have to unwrap a Maybe to get to a regular list.

2023-05-04 edit: the relevant commit is https://github.com/camfort/fortran-src/commit/5d9e46b99a98fd7ff26fc447ee66e513df343685

raehik avatar Jul 13 '22 15:07 raehik

I think this suggestion would make sense, I guess for a lot of these cases the SrcSpan will just be zero width immediately after whatever node contains the list?

RaoulHC avatar Oct 13 '22 10:10 RaoulHC