(closes #3178) Fixes to handle Directives from fparser

.github/workflows/compilation.yml

@@ -87,7 +87,7 @@ jobs:
         python -m pip install --upgrade pip
         # Uncomment the below to use the submodule version of fparser rather
         # than the latest release from pypi.
-        # pip install external/fparser
+        pip install external/fparser
         pip install .[test,psydata,doc]
     - name: Unit tests with compilation - gfortran
       run: |

.github/workflows/extraction_test.yml

@@ -74,7 +74,7 @@ jobs:
         python -m pip install --upgrade pip
         # Uncomment the below to use the submodule version of fparser rather
         # than the latest release from pypi.
-        # pip install external/fparser
+        pip install external/fparser
         pip install .[test]
         # Install BAF, and its version of Fab:
         # ------------------------------------

.github/workflows/lfric_test.yml

@@ -86,7 +86,7 @@ jobs:
         python -m pip install --upgrade pip
         # Uncomment the below to use the submodule version of fparser rather
         # than the latest release from pypi.
-        # pip install external/fparser
+        pip install external/fparser
         pip install .[test]
         # Fetch the specified version of LFRic apps
         source /archive/psyclone-spack/psyclone-spack-Jun25/spack-repo/share/spack/setup-env.sh

.github/workflows/nemo_tests.yml

@@ -85,7 +85,7 @@ jobs:
         python -m pip install --upgrade pip
         # Uncomment the below to use the submodule version of fparser rather
         # than the latest release from pypi.
-        # pip install external/fparser
+        pip install external/fparser
         pip install .[test]
         # Compile nvidia profiling tools
         module load nvidia-hpcsdk/${NVFORTRAN_VERSION}

.github/workflows/nemo_v5_tests.yml

@@ -82,7 +82,7 @@ jobs:
         python -m pip install --upgrade pip
         # Uncomment the below to use the submodule version of fparser rather
         # than the latest release from pypi.
-        # pip install external/fparser
+        pip install external/fparser
         pip install .
     - name: Reset working directory
       run: |

.github/workflows/python-package.yml

@@ -105,7 +105,7 @@ jobs:
           # We need to install sphinx to get correct doc testing
           # Uncomment the below to use the submodule version of fparser rather
           # than the latest release from pypi.
-          # pip install external/fparser
+          pip install external/fparser
           pip install .[doc]
           pip install .[test]
       - name: Lint with flake8

.gitmodules

@@ -1,6 +1,6 @@
 [submodule "external/fparser"]
 	path = external/fparser
-	url = https://github.com/stfc/fparser.git
+    url = https://github.com/stfc/fparser.git
 [submodule "external/dl_esm_inf"]
 	path = external/dl_esm_inf
 	url = https://github.com/stfc/dl_esm_inf.git

changelog

@@ -1,3 +1,5 @@
+   4) PR #3178 for #3196. Improves fparser2 reader directive handling.
+
    3) PR #3210 for #3203. Removes outstanding references to the old,
    pared-down LFRic infrastructure.
 

doc/conf.py

@@ -413,7 +413,9 @@
     # the links to anchors to the main README.
     r'^https://github.com/stfc/PSyclone#',
     # Requires authentication.
-    r'^https://code.metoffice.gov.uk/trac'
+    r'^https://code.metoffice.gov.uk/trac',
+    # This often fails, but the link exists
+    r'^https://www.khronos.org'
 ]
 
 # -- Autodoc configuration ---------------------------------------------------

doc/user_guide/psyclone_command.rst

@@ -520,6 +520,9 @@ some limitations:
      nodes. Also PSyclone will not know any details about these nodes
      (including that they contain directives) but this functionality will
      be improved over time.
+  3. Directives that appear before or inside the declaration section of a
+     Subroutine, Program or Module are converted to comments (the default
+     output add a space after the ``!``, effectively disabling the directive.
 
-Note that using the ``keep-comments`` option alone means that any comments
-that PSyclone interprets as directives will be lost from the input.
+Note that using the ``--keep-comments`` option without the ``--keep-directives``
+option means that any comments that PSyclone interprets as directives will be excluded.

setup.py

@@ -169,7 +169,7 @@ def get_files(directory, install_path, valid_suffixes):
         classifiers=CLASSIFIERS,
         packages=PACKAGES,
         package_dir={"": "src"},
-        install_requires=['pyparsing', 'fparser==0.2.1', 'configparser',
+        install_requires=['pyparsing', 'fparser>=0.2.1', 'configparser',
                           'sympy', "Jinja2", 'termcolor', 'graphviz'],
         extras_require={
             'doc': ["sphinx", "sphinxcontrib.bibtex", "sphinx_design",

src/psyclone/psyir/frontend/fortran.py

@@ -94,6 +94,7 @@ def __init__(self, free_form: bool = True, ignore_comments: bool = True,
                 " only have an effect if ignore_comments is also set to False."
             )
         self._ignore_comments = ignore_comments
+        self._ignore_directives = ignore_directives
         self._processor = Fparser2Reader(ignore_directives,
                                          last_comments_as_codeblocks,
                                          resolve_modules)
@@ -132,7 +133,8 @@ def psyir_from_source(self, source_code: str) -> Node:
         SYMBOL_TABLES.clear()
         string_reader = FortranStringReader(
             source_code, include_dirs=Config.get().include_paths,
-            ignore_comments=self._ignore_comments)
+            ignore_comments=self._ignore_comments,
+            process_directives=not self._ignore_directives)
         # Set reader to free format.
         string_reader.set_format(FortranFormat(self._free_form, False))
 
@@ -258,9 +260,12 @@ def psyir_from_file(self, file_path):
 
         # Using the FortranFileReader instead of manually open the file allows
         # fparser to keep the filename information in the tree
-        reader = FortranFileReader(file_path,
-                                   include_dirs=Config.get().include_paths,
-                                   ignore_comments=self._ignore_comments)
+        reader = FortranFileReader(
+            file_path,
+            include_dirs=Config.get().include_paths,
+            ignore_comments=self._ignore_comments,
+            process_directives=not self._ignore_directives
+        )
         reader.set_format(FortranFormat(self._free_form, False))
         try:
             parse_tree = self._parser(reader)

src/psyclone/psyir/frontend/fparser2.py

@@ -42,9 +42,10 @@
 
 from collections import OrderedDict
 from dataclasses import dataclass, field
+import re
 import os
 import sys
-from typing import Iterable, Optional
+from typing import Iterable, Optional, Union
 
 from fparser.common.readfortran import FortranStringReader
 from fparser.two import C99Preprocessor, Fortran2003, utils
@@ -1040,6 +1041,7 @@ def __init__(self, ignore_directives: bool = True,
             Fortran2003.Module: self._module_handler,
             Fortran2003.Main_Program: self._main_program_handler,
             Fortran2003.Program: self._program_handler,
+            Fortran2003.Directive: self._directive_handler,
         }
         # Used to attach inline comments to the PSyIR symbols and nodes
         self._last_psyir_parsed_and_span = None
@@ -2432,7 +2434,8 @@ def process_declarations(self, parent, nodes, arg_list,
         preceding_comments = []
         for node in nodes:
             if isinstance(node, Fortran2003.Implicit_Part):
-                for comment in walk(node, Fortran2003.Comment):
+                for comment in walk(node, (Fortran2003.Comment,
+                                           Fortran2003.Directive)):
                     self.process_comment(comment, preceding_comments)
             elif isinstance(node, Fortran2003.Derived_Type_Def):
                 sym = self._process_derived_type_decln(parent, node,
@@ -2460,7 +2463,8 @@ def process_declarations(self, parent, nodes, arg_list,
         for node in nodes:
 
             if isinstance(node, Fortran2003.Implicit_Part):
-                for comment in walk(node, Fortran2003.Comment):
+                for comment in walk(node, (Fortran2003.Comment,
+                                           Fortran2003.Directive)):
                     self.process_comment(comment, preceding_comments)
                     continue
                 # Anything other than a PARAMETER statement or an
@@ -2891,24 +2895,6 @@ def _add_comments_to_tree(self, parent: Node, preceding_comments,
         :type preceding_comments: list[:py:class:`fparser.two.utils.Base`]
         :param psy_child: The current PSyIR node being constructed.
         '''
-        for comment in preceding_comments[:]:
-            # If the comment is a directive and we
-            # keep_directives then create a CodeBlock for
-            # the directive.
-
-            # TODO: fparser #469. This only captures some free-form
-            # directives.
-            if (not self._ignore_directives and
-                    comment.tostr().startswith("!$")):
-                block = self.nodes_to_code_block(parent, [comment])
-                # Attach any comments that came before this directive to this
-                # CodeBlock node.
-                if comment is not preceding_comments[0]:
-                    index = preceding_comments.index(comment)
-                    block.preceding_comment += self._comments_list_to_string(
-                        preceding_comments[0:index])
-                    preceding_comments = preceding_comments[index:]
-                preceding_comments.remove(comment)
         # Leftover comments are added to the provided PSyIR node.
         psy_child.preceding_comment += self._comments_list_to_string(
             preceding_comments
@@ -3018,7 +3004,9 @@ def _create_child(self, child, parent=None):
             # there), so we have to examine the first statement within it. We
             # must allow for the case where the block is empty though.
             if (child.content and child.content[0] and
-                    (not isinstance(child.content[0], Fortran2003.Comment)) and
+                    (not isinstance(child.content[0],
+                                    (Fortran2003.Comment,
+                                     Fortran2003.Directive))) and
                     child.content[0].item and child.content[0].item.label):
                 raise NotImplementedError("Unsupported labelled statement")
         elif isinstance(child, StmtBase):
@@ -3370,14 +3358,19 @@ def _do_construct_handler(self, node, parent):
             raise NotImplementedError("Unsupported Loop")
 
         # Process loop body (ignore 'do' and 'end do' statements)
-        # Keep track of the comments before the 'do' statement
+        # Keep track of the comments and directives before the 'do' statement
         loop_body_nodes = []
         preceding_comments = []
         found_do_stmt = False
         for child in node.content:
             if isinstance(child, Fortran2003.Comment) and not found_do_stmt:
                 self.process_comment(child, preceding_comments)
                 continue
+            if isinstance(child, Fortran2003.Directive) and not found_do_stmt:
+                directive = self._directive_handler(child, None)
+                # Add the directive before the loop.
+                loop.parent.addchild(directive)
+                continue
             if isinstance(child, Fortran2003.Nonlabel_Do_Stmt):
                 found_do_stmt = True
                 continue
@@ -3430,6 +3423,10 @@ def _if_construct_handler(self, node, parent):
         for child in node.content[:clause_indices[0]]:
             if isinstance(child, Fortran2003.Comment):
                 self.process_comment(child, preceding_comments)
+            if isinstance(child, Fortran2003.Directive):
+                direc = self._directive_handler(child, None)
+                parent.addchild(direc)
+
         # NOTE: The comments are added to the IfBlock node.
         # NOTE: Comments before the 'else[if]' statements are not handled.
 
@@ -5394,9 +5391,9 @@ def _process_args(self, node, call, canonicalise=None):
 
         return call
 
-    def _get_lost_declaration_comments(self, decl_list,
-                                       attach_trailing_symbol: bool = True)\
-            -> list[Fortran2003.Comment]:
+    def _get_lost_declaration_comments_and_directives(
+        self, decl_list, attach_trailing_symbol: bool = True
+    ) -> list[Union[Fortran2003.Comment, Fortran2003.Directive]]:
         '''Finds comments from the variable declaration that the default
         declaration handler doesn't keep. Any comments that appear after
         the final declaration but before the first PSyIR node created are
@@ -5410,14 +5407,16 @@ def _get_lost_declaration_comments(self, decl_list,
                                        the last symbol to the tree or not.
                                        Defaults to True
 
-        :returns: a list of comments that have been missed.
+        :returns: a list of comments and directives that have been
+                  missed.
         '''
         lost_comments = []
         if len(decl_list) != 0 and isinstance(decl_list[-1],
                                               Fortran2003.Implicit_Part):
             # fparser puts all comments after the end of the last declaration
             # in the tree of the last declaration.
-            for comment in walk(decl_list[-1], Fortran2003.Comment):
+            for comment in walk(decl_list[-1], (Fortran2003.Comment,
+                                                Fortran2003.Directive)):
                 if len(comment.tostr()) == 0:
                     continue
                 if self._last_psyir_parsed_and_span is not None:
@@ -5552,7 +5551,8 @@ def _subroutine_handler(self, node, parent):
             # declarations as part of the declarations, but in PSyclone
             # they need to be a preceding_comment unless it's an inline
             # comment on the last declaration.
-            lost_comments = self._get_lost_declaration_comments(decl_list)
+            lost_comments = \
+                self._get_lost_declaration_comments_and_directives(decl_list)
 
             # Check whether the function-stmt has a prefix specifying the
             # return type (other prefixes are handled in
@@ -5707,7 +5707,9 @@ def _main_program_handler(self, node, parent):
         # fparser puts comments at the end of the declarations
         # whereas as preceding comments they belong in the execution part
         # except if it's an inline comment on the last declaration.
-        lost_comments = self._get_lost_declaration_comments(decl_list, False)
+        lost_comments = \
+            self._get_lost_declaration_comments_and_directives(decl_list,
+                                                               False)
 
         try:
             prog_exec = _first_type_match(node.content,
@@ -5840,8 +5842,25 @@ def process_comment(self, comment, preceding_comments):
         '''
         if len(comment.tostr()) == 0:
             return
-        if self._ignore_directives and comment.tostr().startswith("!$"):
-            return
+        if self._ignore_directives:
+            # When directive detection is disabled in fparser, but we still
+            # request comments the directive will be part of the comments. This
+            # is typically not an issue because current psyir backends add an
+            # space between the comment character and the start of the comment,
+            # effectively disabling the directive. However, for clarity, we
+            # still try to clean them up using the regex below.
+            _directive_formats = [
+                r"\!\$[a-z]",  # Generic directive
+                r"c\$[a-z]",  # Generic directive
+                r"\*\$[a-z]",  # Generic directive
+                r"\!dir\$",  # flang, ifx, ifort directives.
+                r"cdir\$",  # flang, ifx, ifort fixed format directive.
+                r"\!gcc\$",  # GCC compiler directive
+            ]
+            comment_str = comment.tostr().lower()
+            for dir_form in _directive_formats:
+                if re.match(dir_form, comment_str):
+                    return
         if self._last_psyir_parsed_and_span is not None:
             last_psyir, last_span = self._last_psyir_parsed_and_span
             if (last_span[1] is not None
@@ -5851,6 +5870,25 @@ def process_comment(self, comment, preceding_comments):
 
         preceding_comments.append(comment)
 
+    def _directive_handler(
+        self, node: Fortran2003.Directive, parent: Node
+    ) -> CodeBlock:
+        '''
+        Process a directive and add it to the tree. The current behaviour
+        places the directive into a CodeBlock.
+
+        :param node: Directive to process.
+        :param parent: The parent to add the PSyIR node to.
+
+        :returns: a CodeBlock containing the input Directive.
+        '''
+        code_block = CodeBlock(
+            [node],
+            CodeBlock.Structure.STATEMENT,
+            parent=parent
+        )
+        return code_block
+
 
 # For Sphinx AutoAPI documentation generation
 __all__ = ["Fparser2Reader"]

src/psyclone/psyir/nodes/codeblock.py

@@ -211,7 +211,7 @@ def get_symbol_names(self) -> List[str]:
         # For directives, we need to analyse all alphanumeric* parts of the
         # comment string and return any names that match a symbol in the
         # symbol table.
-        for node in walk(parse_tree, Fortran2003.Comment):
+        for node in walk(parse_tree, Fortran2003.Directive):
             string_rep = node.tostr()
             # Directives start with a $
             if string_rep.lstrip()[0:2] != "!$":