search

stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
105 stars 28 forks source link

Basic support for comments in the PSyIR #1247

Open sergisiso opened 3 years ago

sergisiso commented 3 years ago

Currently PSyIR comments are added with expressions like:

block = Comment(FortranStringReader("! My comment\n", ignore_comments=False))
codeblock = CodeBlock([block], CodeBlock.Structure.STATEMENT)

This expression have 2 limitations:

First I thought of creating a new Node, but I don't like this idea because comments are syntax elements and shouln't be part of the AST. Also transformations very easily leave them behind and end up in the wrong place.

So, I think the right place for them is as an attribute to all statement sub-class nodes Statement(..., comment="My comment"), this way it is moved around with the relevant statement.

Note that the Fortran front-end ignores comments, I think this is good as when parsing code it may be ambiguous to which statement they relate to and propose to keep it this way. But it works to create equivalent comments to the ones added in gen_code because with direct PSyIR creation this approach works well.

@rupertford, @arporter, @hiker, @TeranIvy What do you think?

rupertford commented 3 years ago

That seems like a nice solution. I do think that we will need to have the option of keeping comments when parsing existing Fortran, but don't see a problem in having rules about how to associate comments with statement, even if they may not always be what the code writer meant. I also wonder whether it would make sense to capture whether a comment is inline or on a separate-line as languages typically (always?) support both.

arporter commented 3 years ago

I second the need to have the option to keep comments from existing code. That's going to become more important now that the re-generated code (in the case of NEMO) may well be formatted quite differently to the original. Users will get twitchy :-)

sergisiso commented 3 years ago

I agree, the frontend could associate comments to its first following statement if they are a full line, or lhs statement if they are inline. But in the first PR I will just focus on adding the support to the PSyIR/backend and leave the front-end for another PR.

arporter commented 2 years ago

In the NEMOVAR hackathon it's become clear that it would be really useful to preserve the manually-added OpenACC directives. I think preserving comments is going to be the easiest way to do this in the short term although I worry that we may break things when we alter the code structure (e.g. WHERE statements).

arporter commented 2 years ago

I've created 1247_allow_comments which simply alters the frontend so that the parser is told not to ignore comments. Any comments then end up in CodeBlocks and are preserved in the output. This is a temporary workaround sufficient (I think) to support the use case of adding routine-level profiling to a code that already has directives in it.