#469 Removed the creation of parsable_x90 files. Fixes #458.

source/fab/parse/fortran_common.py

@@ -127,8 +127,12 @@ def run(self, fpath: Path) \
         return analysed_file, analysis_fpath
 
     def _get_analysis_fpath(self, fpath, file_hash) -> Path:
+        """
+        :returns: the name to use for the analysis file. It consists of
+            the original filename, the hash, and a `.an` suffix.
+        """
         return Path(self.config.prebuild_folder /
-                    f'{fpath.stem}.{file_hash}.an')
+                    f'{fpath.name}.{file_hash}.an')
 
     def _parse_file(self, fpath):
         """Get a node tree from a fortran file."""

source/fab/steps/psyclone.py

@@ -10,7 +10,6 @@
 """
 from dataclasses import dataclass
 import logging
-import re
 import shutil
 import warnings
 from itertools import chain
@@ -194,19 +193,15 @@ def _generate_mp_payload(config, analysed_x90, all_kernel_hashes, overrides_fold
 def _analyse_x90s(config: BuildConfig,
                   x90s: Set[Path]) -> Dict[Path, AnalysedX90]:
     """
-    Analyse parsable versions of the x90s, finding kernel dependencies.
+    Analyse the x90s, finding kernel dependencies.
     """
 
-    # make parsable - todo: fast enough not to require prebuilds?
-    with TimerLogger(f"converting {len(x90s)} x90s into parsable fortran"):
-        parsable_x90s = run_mp(config, items=x90s, func=make_parsable_x90)
-
     # Parse. Note that there is no need to ignore dependencies: the x90
     # files will be converted to algorithm layer f90 files, and then
     # properly analysed later.
     x90_analyser = X90Analyser(config=config)
-    with TimerLogger(f"analysing {len(parsable_x90s)} parsable x90 files"):
-        x90_results = run_mp(config, items=parsable_x90s, func=x90_analyser.run)
+    with TimerLogger(f"analysing {len(x90s)} x90 files"):
+        x90_results = run_mp(config, items=x90s, func=x90_analyser.run)
     log_or_dot_finish(logger)
     x90_analyses, x90_artefacts = zip(*x90_results) if x90_results else ((), ())
     check_for_errors(results=x90_analyses)
@@ -215,11 +210,11 @@ def _analyse_x90s(config: BuildConfig,
     prebuild_files = list(by_type(x90_artefacts, Path))
     config.add_current_prebuilds(prebuild_files)
 
-    # record the analysis results against the original x90 filenames (not the parsable versions we analysed)
+    # record the analysis results against the original x90 filenames
     analysed_x90 = by_type(x90_analyses, AnalysedX90)
     analysed_x90 = {result.fpath.with_suffix('.x90'): result for result in analysed_x90}
 
-    # make the hashes from the original x90s, not the parsable versions which have invoke names removed.
+    # make the hashes from the x90s
     for p in analysed_x90:
         analysed_x90[p]._file_hash = file_checksum(p).file_hash
 
@@ -259,7 +254,6 @@ def _analyse_kernels(
     file_lists = [list(file_walk(root, ignore_folders=[config.prebuild_folder])) for root in kernel_roots]
     all_kernel_files: Set[Path] = set(sum(file_lists, []))
     kernel_files: List[Path] = suffix_filter(all_kernel_files, ['.f90'])
-
     # We use the normal Fortran analyser, which records psyclone kernel metadata.
     # todo: We'd like to separate that from the general fortran analyser at some point, to reduce coupling.
     # The Analyse step also uses the same fortran analyser. It stores its results so they won't be analysed twice.
@@ -373,7 +367,7 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs):
      - cli args
 
     """
-    # We've analysed (a parsable version of) this x90.
+    # We've analysed this x90.
     analysis_result = mp_payload.analysed_x90[x90_file]  # type: ignore
 
     # include the hashes of kernels used by this x90
@@ -393,7 +387,7 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs):
     # todo: hash the psyclone version in case the built-in kernels change?
     prebuild_hash = sum([
 
-        # the hash of the x90 (not of the parsable version, so includes invoke names)
+        # the hash of the x90
         analysis_result.file_hash,
 
         # the hashes of the kernels used by this x90
@@ -437,64 +431,3 @@ def _check_override(check_path: Path, mp_payload: MpCommonArgs):
 
     # we didn't have an override, so continue using this file
     return check_path
-
-
-# regex to convert an x90 into parsable fortran, so it can be analysed using a third party tool
-
-WHITE = r'[\s&]+'
-OPT_WHITE = r'[\s&]*'
-
-SQ_STRING = "'[^']*'"
-DQ_STRING = '"[^"]*"'
-STRING = f'({SQ_STRING}|{DQ_STRING})'
-
-NAME_KEYWORD = 'name' + OPT_WHITE + '=' + OPT_WHITE + STRING + OPT_WHITE + ',' + OPT_WHITE
-NAMED_INVOKE = 'call' + WHITE + 'invoke' + OPT_WHITE + r'\(' + OPT_WHITE + NAME_KEYWORD
-
-_x90_compliance_pattern = None
-
-
-# todo: In the future, we'd like to extend fparser to handle the leading invoke keywords. (Lots of effort.)
-def make_parsable_x90(x90_path: Path) -> Path:
-    """
-    Take out the leading name keyword in calls to invoke(), making temporary, parsable fortran from x90s.
-
-    If present it looks like this::
-
-        call invoke( name = "compute_dry_mass", ...
-
-    Returns the path of the parsable file.
-
-    This function is not slow so we're not creating prebuilds for this work.
-
-    """
-    global _x90_compliance_pattern
-    if not _x90_compliance_pattern:
-        _x90_compliance_pattern = re.compile(pattern=NAMED_INVOKE)
-
-    # src = open(x90_path, 'rt').read()
-
-    # Before we remove the name keywords to invoke, we must remove any comment lines.
-    # This is the simplest way to avoid producing bad fortran when the name keyword is followed by a comment line.
-    # I.e. The comment line doesn't have an "&", so we get "call invoke(!" with no "&", which is a syntax error.
-    src_lines = open(x90_path, 'rt').readlines()
-    no_comment_lines = [line for line in src_lines if not line.lstrip().startswith('!')]
-    src = ''.join(no_comment_lines)
-
-    replaced = []
-
-    def repl(matchobj):
-        # matchobj[0] contains the entire matching string, from "call" to the "," after the name keyword.
-        # matchobj[1] contains the single group in the search pattern, which is defined in STRING.
-        name = matchobj[1].replace('"', '').replace("'", "")
-        replaced.append(name)
-        return 'call invoke('
-
-    out = _x90_compliance_pattern.sub(repl=repl, string=src)
-
-    out_path = x90_path.with_suffix('.parsable_x90')
-    open(out_path, 'wt').write(out)
-
-    logger.debug(f'names removed from {str(x90_path)}: {replaced}')
-
-    return out_path

tests/system_tests/FortranDependencies/test_FortranDependencies.py

@@ -50,29 +50,29 @@ def test_fortran_dependencies(tmp_path):
     }
 
     # check the analysis results
-    assert AnalysedFortran.load(config.prebuild_folder / 'first.193489053.an') == AnalysedFortran(
+    assert AnalysedFortran.load(config.prebuild_folder / 'first.f90.193489053.an') == AnalysedFortran(
         fpath=config.build_output / 'first.f90', file_hash=193489053,
         program_defs={'first'},
         module_defs=None, symbol_defs={'first'},
         module_deps={'greeting_mod', 'constants_mod'}, symbol_deps={'greeting_mod', 'constants_mod', 'greet'})
 
-    assert AnalysedFortran.load(config.prebuild_folder / 'two.2557739057.an') == AnalysedFortran(
+    assert AnalysedFortran.load(config.prebuild_folder / 'two.f90.2557739057.an') == AnalysedFortran(
         fpath=config.build_output / 'two.f90', file_hash=2557739057,
         program_defs={'second'},
         module_defs=None, symbol_defs={'second'},
         module_deps={'constants_mod', 'bye_mod'}, symbol_deps={'constants_mod', 'bye_mod', 'farewell'})
 
-    assert AnalysedFortran.load(config.prebuild_folder / 'greeting_mod.62446538.an') == AnalysedFortran(
+    assert AnalysedFortran.load(config.prebuild_folder / 'greeting_mod.f90.62446538.an') == AnalysedFortran(
         fpath=config.build_output / 'greeting_mod.f90', file_hash=62446538,
         module_defs={'greeting_mod'}, symbol_defs={'greeting_mod'},
         module_deps={'constants_mod'}, symbol_deps={'constants_mod'})
 
-    assert AnalysedFortran.load(config.prebuild_folder / 'bye_mod.3332267073.an') == AnalysedFortran(
+    assert AnalysedFortran.load(config.prebuild_folder / 'bye_mod.f90.3332267073.an') == AnalysedFortran(
         fpath=config.build_output / 'bye_mod.f90', file_hash=3332267073,
         module_defs={'bye_mod'}, symbol_defs={'bye_mod'},
         module_deps={'constants_mod'}, symbol_deps={'constants_mod'})
 
-    assert AnalysedFortran.load(config.prebuild_folder / 'constants_mod.233796393.an') == AnalysedFortran(
+    assert AnalysedFortran.load(config.prebuild_folder / 'constants_mod.f90.233796393.an') == AnalysedFortran(
         fpath=config.build_output / 'constants_mod.f90', file_hash=233796393,
         module_defs={'constants_mod'}, symbol_defs={'constants_mod'},
         module_deps=None, symbol_deps=None)

tests/system_tests/psyclone/test_psyclone_system_test.py

@@ -3,10 +3,7 @@
 #  For further details please refer to the file COPYRIGHT
 #  which you should have received as part of this distribution
 # ##############################################################################
-import filecmp
-from os import unlink
 from pathlib import Path
-import shutil
 from typing import Any, Dict
 from unittest import mock
 
@@ -19,8 +16,7 @@
 from fab.steps.grab.folder import grab_folder
 from fab.steps.preprocess import preprocess_fortran
 from fab.steps.psyclone import (_analyse_x90s, _analyse_kernels,
-                                make_parsable_x90, preprocess_x90,
-                                psyclone)
+                                preprocess_x90, psyclone)
 from fab.tools.psyclone import Psyclone
 from fab.tools.tool_box import ToolBox
 from fab.util import file_checksum
@@ -41,29 +37,6 @@
 #       Make the skeleton sample call more than one kernel.
 #       Make the skeleton sample include a big X90 and a little x90.
 
-
-def test_make_parsable_x90(tmp_path):
-    # turn x90 into parsable fortran by removing the name keyword from calls to invoke
-    grab_x90_path = SAMPLE_X90
-    input_x90_path = tmp_path / grab_x90_path.name
-    shutil.copy(grab_x90_path, input_x90_path)
-
-    parsable_x90_path = make_parsable_x90(input_x90_path)
-
-    with BuildConfig('proj', ToolBox(), fab_workspace=tmp_path) as config:
-        x90_analyser = X90Analyser(config=config)
-        x90_analyser.run(parsable_x90_path)
-
-    # ensure the files are as expected
-    assert filecmp.cmp(parsable_x90_path, EXPECT_PARSABLE_X90)
-
-    # make_parsable_x90() puts its output file next to the source.
-    # Because we're reading sample code from our Fab git repos,
-    # we don't want to leave this test output in our working copies, so delete it.
-    # Otherwise, it'll appear in the output from `git status`.
-    unlink(parsable_x90_path)
-
-
 class TestX90Analyser:
 
     expected_analysis_result = AnalysedX90(
@@ -106,7 +79,7 @@ def test_analyse(self, tmp_path):
         # analysed_x90
         assert analysed_x90 == {
             SAMPLE_X90: AnalysedX90(
-                fpath=SAMPLE_X90.with_suffix('.parsable_x90'),
+                fpath=SAMPLE_X90,
                 file_hash=file_checksum(SAMPLE_X90).file_hash,
                 kernel_deps={'kernel_one_type', 'kernel_two_type'})}
 

tests/unit_tests/parse/fortran/test_fortran_analyser.py

@@ -9,7 +9,6 @@
 
 
 from pathlib import Path
-from tempfile import NamedTemporaryFile
 from unittest import mock
 
 from fparser.common.readfortran import FortranFileReader  # type: ignore
@@ -75,8 +74,9 @@ def test_module_file(self, fortran_analyser, module_fpath,
         with mock.patch('fab.parse.AnalysedFile.save'):
             analysis, artefact = fortran_analyser.run(fpath=module_fpath)
         assert analysis == module_expected
-        assert artefact == (fortran_analyser._config.prebuild_folder /
-                            f'test_fortran_analyser.{analysis.file_hash}.an')
+        assert artefact == (
+            fortran_analyser._config.prebuild_folder /
+            f'test_fortran_analyser.f90.{analysis.file_hash}.an')
 
     def test_module_file_no_openmp(self, fortran_analyser: FortranAnalyser,
                                    module_fpath: Path,
@@ -95,8 +95,9 @@ def test_module_file_no_openmp(self, fortran_analyser: FortranAnalyser,
 
         assert analysis == module_expected
         assert isinstance(analysis, AnalysedFortran)
-        assert artefact == (fortran_analyser._config.prebuild_folder /
-                            f'test_fortran_analyser.{analysis.file_hash}.an')
+        assert artefact == (
+            fortran_analyser._config.prebuild_folder /
+            f'test_fortran_analyser.f90.{analysis.file_hash}.an')
 
     def test_module_file_ignore_dependencies(
              self,
@@ -122,15 +123,18 @@ def test_module_file_ignore_dependencies(
 
         assert analysis == module_expected
         assert isinstance(analysis, AnalysedFortran)
-        assert artefact == (fortran_analyser._config.prebuild_folder /
-                            f'test_fortran_analyser.{analysis.file_hash}.an')
+        assert artefact == (
+                fortran_analyser._config.prebuild_folder /
+                f'test_fortran_analyser.f90.{analysis.file_hash}.an')
 
     def test_program_file(self,
+                          tmp_path: Path,
                           fortran_analyser: FortranAnalyser,
                           module_fpath: Path,
                           module_expected: AnalysedFortran) -> None:
         # same as test_module_file() but replacing MODULE with PROGRAM
-        with NamedTemporaryFile(mode='w+t', suffix='.f90') as tmp_file:
+        prog_path = tmp_path / "prog.f90"
+        with prog_path.open('w') as tmp_file:
             tmp_file.write(module_fpath.open().read().replace("MODULE",
                                                               "PROGRAM"))
             tmp_file.flush()
@@ -148,8 +152,9 @@ def test_program_file(self,
 
             assert analysis == module_expected
             assert isinstance(analysis, AnalysedFortran)
-            assert artefact == fortran_analyser._config.prebuild_folder \
-                   / f'{Path(tmp_file.name).stem}.{analysis.file_hash}.an'
+            assert artefact == (
+                fortran_analyser._config.prebuild_folder /
+                f'{prog_path.name}.{analysis.file_hash}.an')
 
 
 # todo: test more methods!