Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maybe AList confuses representation and limits precise reprinting #231

Open
raehik opened this issue Jul 13, 2022 · 2 comments
Open

Maybe AList confuses representation and limits precise reprinting #231

raehik opened this issue Jul 13, 2022 · 2 comments
Labels
area:ast Related to Fortran AST representation. Status: Discussion

Comments

@raehik
Copy link
Collaborator

raehik commented Jul 13, 2022

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
Copy link
Collaborator Author

raehik commented Jul 13, 2022

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 5d9e46b

@RaoulHC
Copy link
Collaborator

RaoulHC commented Oct 13, 2022

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?

@raehik raehik added area:ast Related to Fortran AST representation. Status: Discussion labels May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ast Related to Fortran AST representation. Status: Discussion
Projects
None yet
Development

No branches or pull requests

2 participants