From 5cdf59aeacc6eb3d6b20c147783f5dd8b96e27af Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 4 Jun 2025 09:48:56 +0100 Subject: [PATCH 01/16] Initial commit to convert directives into codeblocks --- src/psyclone/psyir/frontend/fparser2.py | 27 +++++++++++++++++++ .../tests/psyir/frontend/fortran_test.py | 13 ++++----- .../psyir/frontend/fparser2_comment_test.py | 11 +++----- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 465232898b..b0d3f746cd 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2666,6 +2666,7 @@ def process_declarations(self, parent, nodes, arg_list, f"Could not process '{stmtfn}'. Statement Function " f"declarations are not supported.") from err + @staticmethod def _process_data_statements(nodes, psyir_parent): '''Limited support for data statements: they will be converted @@ -2892,6 +2893,24 @@ def process_nodes(self, parent, nodes): # Add the comments to nodes that support it and reset the # list of comments if isinstance(psy_child, CommentableMixin): + for comment in preceding_comments[:]: + # If the comment is a directive and we + # keep_directives then create a CodeBlock for + # the directive. + if (not self._ignore_directives and + comment.tostr().startswith("!$")): + block = self.nodes_to_code_block(parent, + [comment]) + # Precede this with any comments as relevant. + 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) psy_child.preceding_comment\ += self._comments_list_to_string( preceding_comments) @@ -2919,6 +2938,14 @@ def process_nodes(self, parent, nodes): # Complete any unfinished code-block self.nodes_to_code_block(parent, code_block_nodes, message) + # If there are any directives at the end we create code blocks for + # them. + if not self._ignore_directives and len(preceding_comments) != 0: + for comment in preceding_comments[:]: + if comment.tostr().startswith("!$"): + self.nodes_to_code_block(parent, [comment]) + preceding_comments.remove(comment) + if self._last_comments_as_codeblocks and len(preceding_comments) != 0: self.nodes_to_code_block(parent, preceding_comments) diff --git a/src/psyclone/tests/psyir/frontend/fortran_test.py b/src/psyclone/tests/psyir/frontend/fortran_test.py index e04b876e5b..406082d9ae 100644 --- a/src/psyclone/tests/psyir/frontend/fortran_test.py +++ b/src/psyclone/tests/psyir/frontend/fortran_test.py @@ -267,13 +267,14 @@ def test_fortran_psyir_from_file(fortran_reader, tmpdir_factory): ignore_directives=False) file_container = fortran_reader.psyir_from_file(filename) assert isinstance(file_container, FileContainer) + assignment = file_container.walk(Assignment)[0] + assert assignment.preceding_comment == "Comment on assignment" + # When keeping directives a comment before the directive + # goes on the directive. + par_direc = file_container.walk(CodeBlock)[0] + assert par_direc.preceding_comment == "Comment on do loop" for node in file_container.walk(CommentableMixin): - if isinstance(node, Loop): - assert node.preceding_comment == ("Comment on do loop\n" - "$omp parallel do") - elif isinstance(node, Assignment): - assert node.preceding_comment == "Comment on assignment" - else: + if node not in (assignment, par_direc): assert node.preceding_comment == "" # Check that the following combination raises an error diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index e343041b7c..a37546ad2f 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -509,9 +509,10 @@ def test_directives(): psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE) loop = psyir.walk(Loop)[0] - assert ( - loop.preceding_comment - == "Comment on loop 'do i = 1, 10'\n$omp parallel do" + directive = loop.preceding(reverse=True)[0] + assert isinstance(directive, CodeBlock) + assert (directive.debug_string() == + "! Comment on loop 'do i = 1, 10'\n!$omp parallel do\n" ) @@ -529,10 +530,6 @@ def test_directives(): """ -@pytest.mark.xfail( - reason="Directive is written back as '! $omp parallel do'" - "instead of '!$omp parallel do'" -) def test_write_directives(): """Test that the directives are written back to the code""" reader = FortranReader(ignore_comments=False, ignore_directives=False) From fdf212b6a44fe59dd41acd86d1baf712e0f07dbb Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 4 Jun 2025 09:50:05 +0100 Subject: [PATCH 02/16] linting --- src/psyclone/psyir/frontend/fparser2.py | 2 -- src/psyclone/tests/psyir/frontend/fparser2_comment_test.py | 5 ++--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index b0d3f746cd..8744c0b592 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2666,7 +2666,6 @@ def process_declarations(self, parent, nodes, arg_list, f"Could not process '{stmtfn}'. Statement Function " f"declarations are not supported.") from err - @staticmethod def _process_data_statements(nodes, psyir_parent): '''Limited support for data statements: they will be converted @@ -2909,7 +2908,6 @@ def process_nodes(self, parent, nodes): preceding_comments[0:index]) preceding_comments = preceding_comments[ index:] - preceding_comments.remove(comment) psy_child.preceding_comment\ += self._comments_list_to_string( diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index a37546ad2f..e4f32aa57d 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -511,9 +511,8 @@ def test_directives(): loop = psyir.walk(Loop)[0] directive = loop.preceding(reverse=True)[0] assert isinstance(directive, CodeBlock) - assert (directive.debug_string() == - "! Comment on loop 'do i = 1, 10'\n!$omp parallel do\n" - ) + assert (directive.debug_string() == + "! Comment on loop 'do i = 1, 10'\n!$omp parallel do\n") EXPECTED_WITH_DIRECTIVES = """subroutine test_sub() From f0de8b799d285ce8f2b5135ab0d54378dd3eb2b8 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 4 Jun 2025 13:14:17 +0100 Subject: [PATCH 03/16] Added to comment functionality to generator and enabled support in LFRic and GOCean --- .../alg_invoke_2_psy_call_trans.py | 5 + .../raise_psyir_2_gocean_kern_trans.py | 4 + .../raise_psyir_2_lfric_alg_trans.py | 6 ++ src/psyclone/generator.py | 41 ++++++-- src/psyclone/parse/utils.py | 8 +- src/psyclone/psyir/frontend/fparser2.py | 31 +++++- src/psyclone/tests/generator_test.py | 94 +++++++++++++++++++ 7 files changed, 178 insertions(+), 11 deletions(-) diff --git a/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py b/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py index 414fcbe1f6..64ecaed199 100644 --- a/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py +++ b/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py @@ -231,6 +231,11 @@ def apply(self, node, options=None): interface=interface) psy_call = Call.create(routine_symbol, arguments) + # Copy over the comments. + # FIXME Unit test. + psy_call.preceding_comment = node.preceding_comment + psy_call.inline_comment = node.inline_comment + node.replace_with(psy_call) # Remove original 'invoke' symbol if there are no other diff --git a/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py b/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py index 7c400ed56c..c006478210 100644 --- a/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py +++ b/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py @@ -244,6 +244,10 @@ def apply(self, node, options=None): symbol_table=gotable.detach()) for child in routine.pop_all_children(): gokernsched.addchild(child) + # Copy across any comments. + # FIXME Unit test. + gokernsched.preceding_comment = routine.preceding_comment + gokernsched.inline_comment = routine.inline_comment routine.replace_with(gokernsched) diff --git a/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py b/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py index 056a02315a..ec8c2eaf68 100644 --- a/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py +++ b/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py @@ -104,6 +104,12 @@ def apply(self, call, index, options=None): invoke_call = LFRicAlgorithmInvokeCall.create( call.routine.symbol, calls, index, name=call_name) + + # Copy across any comments. + # FIXME Unit test. + invoke_call.preceding_comment = call.preceding_comment + invoke_call.inline_comment = call.inline_comment + call.replace_with(invoke_call) diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index 9a7b3b835d..3abe7c634b 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -175,7 +175,9 @@ def generate(filename, api="", kernel_paths=None, script_name=None, line_length=False, distributed_memory=None, kern_out_path="", - kern_naming="multiple"): + kern_naming="multiple", + keep_comments: bool = False, + keep_directives: bool = False): # pylint: disable=too-many-arguments, too-many-statements # pylint: disable=too-many-branches, too-many-locals '''Takes a PSyclone algorithm specification as input and outputs the @@ -209,6 +211,8 @@ def generate(filename, api="", kernel_paths=None, script_name=None, :rtype: Tuple[:py:class:`fparser.one.block_statements.BeginSource`, :py:class:`fparser.one.block_statements.Module`] | Tuple[:py:class:`fparser.one.block_statements.BeginSource`, str] + :param keep_comments: whether to keep comments from the original source. + :param keep_directives: whether to keep directives from the original :raises GenerationError: if an invalid API is specified. :raises GenerationError: if an invalid kernel-renaming scheme is specified. @@ -274,17 +278,19 @@ def generate(filename, api="", kernel_paths=None, script_name=None, elif api in GOCEAN_API_NAMES or (api in LFRIC_API_NAMES and LFRIC_TESTING): # Create language-level PSyIR from the Algorithm file - reader = FortranReader() + reader = FortranReader(ignore_comments=not keep_comments, + ignore_directives=not keep_directives) if api in LFRIC_API_NAMES: # avoid undeclared builtin errors in PSyIR by adding a # wildcard use statement. - fp2_tree = parse_fp2(filename) + fp2_tree = parse_fp2(filename, ignore_comments=not keep_comments) # Choose a module name that is invalid Fortran so that it # does not clash with any existing names in the algorithm # layer. builtins_module_name = "_psyclone_builtins" add_builtins_use(fp2_tree, builtins_module_name) - psyir = Fparser2Reader().generate_psyir(fp2_tree) + psyir = Fparser2Reader(ignore_directives=not keep_directives).\ + generate_psyir(fp2_tree) # Check that there is only one module/program per file. check_psyir(psyir, filename) else: @@ -512,6 +518,14 @@ def main(arguments): "--log-file", default=None, help="sets the output file to use for logging (defaults to stderr)." ) + parser.add_argument( + "--keep-comments", default=False, action="store_true", + help="keeps comments from the original code (defaults to False)." + ) + parser.add_argument( + "--keep-directives", default=False, action="store_true", + help="keeps directives from the original code (defaults to False)." + ) args = parser.parse_args(arguments) @@ -588,10 +602,17 @@ def main(arguments): print(str(err), file=sys.stderr) sys.exit(1) + if args.keep_directives and not args.keep_comments: + logger.warning("keep_directives requires keep_comments so " + "PSyclone enabled keep_comments.") + args.keep_comments = True + if not args.psykal_dsl: code_transformation_mode(input_file=args.filename, recipe_file=args.script, output_file=args.o, + keep_comments=args.keep_comments, + keep_directives=args.keep_directives, line_length=args.limit) else: # PSyKAl-DSL mode @@ -619,7 +640,9 @@ def main(arguments): line_length=(args.limit == 'all'), distributed_memory=args.dist_mem, kern_out_path=kern_out_path, - kern_naming=args.kernel_renaming) + kern_naming=args.kernel_renaming, + keep_comments=args.keep_comments, + keep_directives=args.keep_directives) except NoInvokesError: _, exc_value, _ = sys.exc_info() print(f"Warning: {exc_value}") @@ -729,6 +752,7 @@ def add_builtins_use(fp2_tree, name): def code_transformation_mode(input_file, recipe_file, output_file, + keep_comments: bool, keep_directives: bool, line_length="off"): ''' Process the input_file with the recipe_file instructions and store it in the output_file. @@ -743,6 +767,9 @@ def code_transformation_mode(input_file, recipe_file, output_file, :type input_file: Optional[str | os.PathLike] :param output_file: the output file where to store the resulting code. :type output_file: Optional[str | os.PathLike] + :param keep_comments: whether to keep comments from the original source. + :param keep_directives: whether to keep directives from the original + source. :param str line_length: set to "output" to break the output into lines of 123 chars, and to "all", to additionally check the input code. @@ -768,7 +795,9 @@ def code_transformation_mode(input_file, recipe_file, output_file, sys.exit(1) # Parse file - psyir = FortranReader(resolve_modules=resolve_mods)\ + psyir = FortranReader(resolve_modules=resolve_mods, + ignore_comments=not keep_comments, + ignore_directives=not keep_directives)\ .psyir_from_file(input_file) # Modify file diff --git a/src/psyclone/parse/utils.py b/src/psyclone/parse/utils.py index 474c78a2c5..81c0e12259 100644 --- a/src/psyclone/parse/utils.py +++ b/src/psyclone/parse/utils.py @@ -104,11 +104,13 @@ def check_line_length(filename): f"'-l/--limit' setting on the PSyclone command line.") -def parse_fp2(filename): +def parse_fp2(filename, ignore_comments: bool = True): '''Parse a Fortran source file contained in the file 'filename' using fparser2. :param str filename: source file (including path) to read. + :param ignore_comments: whether to remove the comments from the input + file. Default is True. :returns: fparser2 AST for the source file. :rtype: :py:class:`fparser.two.Fortran2003.Program` :raises ParseError: if the file could not be parsed. @@ -117,8 +119,10 @@ def parse_fp2(filename): # We get the directories to search for any Fortran include files from # our configuration object. config = Config.get() + # FIXME Unit test. try: - reader = FortranFileReader(filename, include_dirs=config.include_paths) + reader = FortranFileReader(filename, include_dirs=config.include_paths, + ignore_comments=ignore_comments) except IOError as error: raise ParseError( f"algorithm.py:parse_fp2: Failed to parse file '{filename}'. " diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 8744c0b592..eb413fd238 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2531,7 +2531,9 @@ def process_declarations(self, parent, nodes, arg_list, visibility=vis, initial_value=init) new_symbol.preceding_comment \ - = '\n'.join(preceding_comments) + = self._comments_list_to_string( + preceding_comments) + preceding_comments = [] self._last_psyir_parsed_and_span\ = (new_symbol, node.item.span) @@ -2573,6 +2575,11 @@ def process_declarations(self, parent, nodes, arg_list, f"Error processing declarations: fparser2 node of type " f"'{type(node).__name__}' not supported") + # Any comments remaining in preceding_comments at this point are + # not added to the code. + lost_comments = [] + lost_comments.extend(preceding_comments) + # Process the nodes again, looking for PARAMETER statements. This is # done after the main declarations loop because they modify existing # symbols and can appear in any order. @@ -2932,7 +2939,6 @@ def process_nodes(self, parent, nodes): parent.addchild(psy_child) # If psy_child is not initialised but it didn't produce a # NotImplementedError, it means it is safe to ignore it. - # Complete any unfinished code-block self.nodes_to_code_block(parent, code_block_nodes, message) @@ -5625,6 +5631,25 @@ def _main_program_handler(self, node, parent): decl_list = [] self.process_declarations(routine, decl_list, []) + # 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 = [] + if len(decl_list) != 0 and isinstance(decl_list[-1], + Fortran2003.Implicit_Part): + for comment in walk(decl_list[-1], Fortran2003.Comment): + if len(comment.tostr()) == 0: + continue + if self._last_psyir_parsed_and_span is not None: + last_symbol, last_span \ + = self._last_psyir_parsed_and_span + if (last_span is not None + and last_span[1] == comment.item.span[0]): + last_symbol.inline_comment\ + = self._comment_to_string(comment) + continue + lost_comments.append(comment) + try: prog_exec = _first_type_match(node.content, Fortran2003.Execution_Part) @@ -5633,7 +5658,7 @@ def _main_program_handler(self, node, parent): # valid. pass else: - self.process_nodes(routine, prog_exec.content) + self.process_nodes(routine, lost_comments + prog_exec.content) return routine diff --git a/src/psyclone/tests/generator_test.py b/src/psyclone/tests/generator_test.py index 272e52e578..5e9eb1a0f5 100644 --- a/src/psyclone/tests/generator_test.py +++ b/src/psyclone/tests/generator_test.py @@ -807,6 +807,100 @@ def test_main_api(capsys, caplog): assert "Logging system initialised" in caplog.record_tuples[0][2] +def test_keep_comments_and_keep_directives(capsys, caplog, tmpdir_factory, + monkeypatch): + ''' Test the keep comments and keep directives arguments to main. ''' + filename = str(tmpdir_factory.mktemp('psyclone_test').join("test.f90")) + code = """subroutine a() + ! Here is a comment + integer :: a + + !comment 1 + !$omp parallel + !$omp do + !comment 2 + do a = 1, 100 + end do + !$omp end do + !$omp end parallel + end subroutine""" + with open(filename, "w", encoding='utf-8') as wfile: + wfile.write(code) + + main([filename, "--keep-comments"]) + output, _ = capsys.readouterr() + + correct = """subroutine a() + ! Here is a comment + integer :: a + + ! comment 1 + ! comment 2 + do a = 1, 100, 1 + enddo + +end subroutine a + +""" + assert output == correct + + main([filename, "--keep-comments", "--keep-directives"]) + output, _ = capsys.readouterr() + + correct = """subroutine a() + ! Here is a comment + integer :: a + + ! comment 1 + !$omp parallel + !$omp do + + ! comment 2 + do a = 1, 100, 1 + enddo + !$omp end do + !$omp end parallel + +end subroutine a + +""" + assert output == correct + + with caplog.at_level(logging.WARNING): + main([filename, "--keep-directives"]) + assert caplog.records[0].levelname == "WARNING" + assert ("keep_directives requires keep_comments so " + "PSyclone enabled keep_comments." + in caplog.record_tuples[0][2]) + output, _ = capsys.readouterr() + + # Test this for LFRIC algorithm domain. + monkeypatch.setattr(generator, "LFRIC_TESTING", True) + filename = os.path.join(LFRIC_BASE_PATH, + "1_single_invoke_with_omp_dir.f90") + main([filename, "-api", "lfric", "--keep-comments"]) + output, _ = capsys.readouterr() + assert "! Here is a comment" in output + assert "!$omp barrier" not in output + + filename = os.path.join(LFRIC_BASE_PATH, + "1_single_invoke_with_omp_dir.f90") + main([filename, "-api", "lfric", "--keep-comments", "--keep-directives"]) + output, _ = capsys.readouterr() + assert "! Here is a comment" in output + assert "!$omp barrier" in output + + # FIXME Test Gocean + filename = os.path.join(GOCEAN_BASE_PATH, "single_invoke.f90") + main([filename, "-api", "gocean"]) + output, _ = capsys.readouterr() + assert "! Create fields on this grid" not in output + + main([filename, "-api", "gocean", "--keep-comments"]) + output, _ = capsys.readouterr() + assert "! Create fields on this grid" in output + + def test_config_flag(): ''' Test that -c/--config take precedence over the configuration file references in the environment variable. From 83e2116b8bd67cf5c162dccc3b540763dffc2aaf Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 4 Jun 2025 13:34:20 +0100 Subject: [PATCH 04/16] Missing file --- .../lfric/1_single_invoke_with_omp_dir.f90 | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 diff --git a/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 b/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 new file mode 100644 index 0000000000..51c8e9a9ac --- /dev/null +++ b/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 @@ -0,0 +1,55 @@ +! ----------------------------------------------------------------------------- +! BSD 3-Clause License +! +! Copyright (c) 2017-2025, Science and Technology Facilities Council +! All rights reserved. +! +! Redistribution and use in source and binary forms, with or without +! modification, are permitted provided that the following conditions are met: +! +! * Redistributions of source code must retain the above copyright notice, this +! list of conditions and the following disclaimer. +! +! * Redistributions in binary form must reproduce the above copyright notice, +! this list of conditions and the following disclaimer in the documentation +! and/or other materials provided with the distribution. +! +! * Neither the name of the copyright holder nor the names of its +! contributors may be used to endorse or promote products derived from +! this software without specific prior written permission. +! +! THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +! "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +! LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +! FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +! COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +! INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +! BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +! LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +! CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +! LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +! ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +! POSSIBILITY OF SUCH DAMAGE. +! ----------------------------------------------------------------------------- +! Author R. W. Ford, STFC Daresbury Lab +! Modified I. Kavcic, Met Office + +program single_invoke + + ! Description: single function specified in an invoke call + use constants_mod, only: r_def + use field_mod, only: field_type + use testkern_mod, only: testkern_type + + implicit none + + type(field_type) :: f1, f2, m1, m2 + real(r_def) :: a + + !Here is a comment + call invoke( & + testkern_type(a, f1, f2, m1, m2) & + ) + !$omp barrier + +end program single_invoke From f053324cab83b037fe99bc1310ce0d7200cd6a97 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 4 Jun 2025 13:56:28 +0100 Subject: [PATCH 05/16] Unit tests --- .../raise_psyir_2_alg_trans.py | 5 +++ .../raise_psyir_2_gocean_kern_trans.py | 5 +-- .../raise_psyir_2_lfric_alg_trans.py | 1 - .../alg_invoke_2_psy_call_trans_test.py | 40 +++++++++++++++++++ .../raise_psyir_2_alg_trans_test.py | 29 ++++++++++++++ .../raise_psyir_2_lfric_alg_trans_test.py | 25 ++++++++++++ 6 files changed, 100 insertions(+), 5 deletions(-) diff --git a/src/psyclone/domain/common/transformations/raise_psyir_2_alg_trans.py b/src/psyclone/domain/common/transformations/raise_psyir_2_alg_trans.py index a72119b58d..2f6689ea73 100644 --- a/src/psyclone/domain/common/transformations/raise_psyir_2_alg_trans.py +++ b/src/psyclone/domain/common/transformations/raise_psyir_2_alg_trans.py @@ -267,6 +267,11 @@ def apply(self, call, index, options=None): invoke_call = AlgorithmInvokeCall.create( call.routine.symbol, calls, index, name=call_name) + + # Keep comments + invoke_call.preceding_comment = call.preceding_comment + invoke_call.inline_comment = call.inline_comment + call.replace_with(invoke_call) diff --git a/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py b/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py index c006478210..7a3715bd5f 100644 --- a/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py +++ b/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py @@ -244,10 +244,7 @@ def apply(self, node, options=None): symbol_table=gotable.detach()) for child in routine.pop_all_children(): gokernsched.addchild(child) - # Copy across any comments. - # FIXME Unit test. - gokernsched.preceding_comment = routine.preceding_comment - gokernsched.inline_comment = routine.inline_comment + routine.replace_with(gokernsched) diff --git a/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py b/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py index ec8c2eaf68..98a1d5fca0 100644 --- a/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py +++ b/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py @@ -106,7 +106,6 @@ def apply(self, call, index, options=None): call.routine.symbol, calls, index, name=call_name) # Copy across any comments. - # FIXME Unit test. invoke_call.preceding_comment = call.preceding_comment invoke_call.inline_comment = call.inline_comment diff --git a/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py b/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py index 6c9aec4b94..d9091ad414 100644 --- a/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py +++ b/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py @@ -44,6 +44,7 @@ from psyclone.domain.common.transformations import AlgTrans from psyclone.domain.common.transformations import AlgInvoke2PSyCallTrans from psyclone.domain.gocean.transformations import GOceanAlgInvoke2PSyCallTrans +from psyclone.psyir.frontend.fortran import FortranReader from psyclone.psyir.nodes import ( Call, Loop, Literal, Container, Reference, ArrayReference, BinaryOperation, CodeBlock, UnaryOperation) @@ -588,3 +589,42 @@ def test_ai2psycall_remove_imported_symbols(fortran_reader): trans.apply(invokes[1]) assert "kern" not in module.symbol_table._symbols assert "kern_mod" not in module.symbol_table._symbols + + +def test_ai2psycall_keep_comments(): + '''Check that the apply() method works as expected when the invoke has + a single kernel with multiple fields of the same name. Also check + that the apply() method creates the required routine and container + symbols if they have not already been created. Also check that the + apply() method removes the invoke symbol from the appropriate + symbol table. Use GOceanAlgInvoke2PSyCallTrans as + AlgInvoke2PSyCallTrans is abstract. + + ''' + code = ( + "subroutine alg1()\n" + " use kern_mod\n" + " use field_mod, only : field_type\n" + " integer :: i,j\n" + " type(field_type) :: field1, field2(10)\n" + " !preceding comment\n" + " call invoke(kern1(field1, field1, field2(i), field2( j )))" + " !inline comment\n" + "end subroutine alg1\n") + fortran_reader = FortranReader(ignore_comments=False) + psyir = fortran_reader.psyir_from_source(code) + AlgTrans().apply(psyir) + invoke = psyir.children[0][0] + + assert isinstance(invoke, AlgorithmInvokeCall) + assert len(psyir.walk(AlgorithmInvokeCall)) == 1 + assert len(psyir.walk(KernelFunctor)) == 1 + assert "invoke" in invoke.scope.symbol_table._symbols + + # Don't call create_psylayer_symbol_root_names() here. This is to + # check that the transformation creates the names if needed. + trans = GOceanAlgInvoke2PSyCallTrans() + trans.apply(invoke) + invoke = psyir.children[0][0] + assert invoke.preceding_comment == "preceding comment" + assert invoke.inline_comment == "inline comment" diff --git a/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py b/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py index 69d4d5ab63..8303f51ecc 100644 --- a/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py +++ b/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py @@ -536,3 +536,32 @@ def test_multi_name(): assert ("There should be at most one named argument in an invoke, but " "there are 2 in 'call invoke(name='Sancho', name2='Fernandes')\n'." in str(info.value)) + + +def test_apply_keep_comments(): + '''Test that an invoke with an array reference argument is transformed + into PSyclone-specific AlgorithmInvokeCall and KernelFunctor + classes. + + ''' + code = ( + "subroutine alg()\n" + " use kern_mod, only: kern\n" + " use field_mod, only: r2d_field\n" + " type(r2d_field) :: field\n" + " ! preceding comment\n" + " call invoke(kern(field)) !inline comment\n" + "end subroutine alg\n") + + fortran_reader = FortranReader(ignore_comments=False) + psyir = fortran_reader.psyir_from_source(code) + subroutine = psyir.children[0] + assert len(subroutine[0].arguments) == 1 + assert isinstance(subroutine[0].arguments[0], ArrayReference) + + invoke_trans = RaisePSyIR2AlgTrans() + invoke_trans.apply(subroutine[0], 1) + + invoke = subroutine[0] + assert invoke.preceding_comment == "preceding comment" + assert invoke.inline_comment == "inline comment" diff --git a/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py b/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py index 63c2700646..fde181e7f1 100644 --- a/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py +++ b/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py @@ -375,3 +375,28 @@ def test_apply_mixed(fortran_reader): check_args(args, [(Reference, "field1"), (Literal, "1.0")]) args = subroutine[0].arguments[3].children check_args(args, [(Reference, "field1"), (Reference, "value")]) + +def test_apply_keeps_comments(fortran_reader): + '''Test the the comments are kept when applying the transformation.''' + code = ( + "subroutine alg()\n" + " use kern_mod\n" + " use field_mod, only : field\n" + " type(field) :: field1\n" + " integer :: value\n" + " call invoke(kern(field1), setval_c(field1, 1.0), name='test', " + "setval_c(field1, 1.0), setval_c(field1, value))\n" + "end subroutine alg\n") + + psyir = fortran_reader.psyir_from_source(code) + subroutine = psyir.children[0] + lfric_invoke_trans = RaisePSyIR2LFRicAlgTrans() + subroutine[0].preceding_comment = "My comment" + subroutine[0].inline_comment = "Inline comment" + + lfric_invoke_trans.apply(subroutine[0], 5) + + assert isinstance(subroutine[0], LFRicAlgorithmInvokeCall) + assert subroutine[0].preceding_comment == "My comment" + assert subroutine[0].inline_comment == "Inline comment" + From 8ea6d9593b9155260c8dbbc1fdd1224d8a53a07f Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 4 Jun 2025 13:57:15 +0100 Subject: [PATCH 06/16] linting --- .../transformations/raise_psyir_2_lfric_alg_trans_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py b/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py index fde181e7f1..b44bc2d763 100644 --- a/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py +++ b/src/psyclone/tests/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans_test.py @@ -376,6 +376,7 @@ def test_apply_mixed(fortran_reader): args = subroutine[0].arguments[3].children check_args(args, [(Reference, "field1"), (Reference, "value")]) + def test_apply_keeps_comments(fortran_reader): '''Test the the comments are kept when applying the transformation.''' code = ( @@ -395,8 +396,7 @@ def test_apply_keeps_comments(fortran_reader): subroutine[0].inline_comment = "Inline comment" lfric_invoke_trans.apply(subroutine[0], 5) - + assert isinstance(subroutine[0], LFRicAlgorithmInvokeCall) assert subroutine[0].preceding_comment == "My comment" assert subroutine[0].inline_comment == "Inline comment" - From d4eb89c6ec3a5320ccf824732dbd27a574dfcdda Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 4 Jun 2025 15:27:20 +0100 Subject: [PATCH 07/16] Fixed missing coverage and removed unnneccessary comment update on declarations from Program --- .../transformations/alg_invoke_2_psy_call_trans.py | 1 - src/psyclone/psyir/frontend/fparser2.py | 10 +++------- .../tests/psyir/frontend/fparser2_comment_test.py | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py b/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py index 64ecaed199..78103f8d98 100644 --- a/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py +++ b/src/psyclone/domain/common/transformations/alg_invoke_2_psy_call_trans.py @@ -232,7 +232,6 @@ def apply(self, node, options=None): psy_call = Call.create(routine_symbol, arguments) # Copy over the comments. - # FIXME Unit test. psy_call.preceding_comment = node.preceding_comment psy_call.inline_comment = node.inline_comment diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index eb413fd238..b7f2449c1b 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2575,11 +2575,6 @@ def process_declarations(self, parent, nodes, arg_list, f"Error processing declarations: fparser2 node of type " f"'{type(node).__name__}' not supported") - # Any comments remaining in preceding_comments at this point are - # not added to the code. - lost_comments = [] - lost_comments.extend(preceding_comments) - # Process the nodes again, looking for PARAMETER statements. This is # done after the main declarations loop because they modify existing # symbols and can appear in any order. @@ -5643,10 +5638,11 @@ def _main_program_handler(self, node, parent): if self._last_psyir_parsed_and_span is not None: last_symbol, last_span \ = self._last_psyir_parsed_and_span + # If the comment goes on the last parsed psyir node + # or symbol, then we don't need to do anything with + # it as its handled by process_declarations. if (last_span is not None and last_span[1] == comment.item.span[0]): - last_symbol.inline_comment\ - = self._comment_to_string(comment) continue lost_comments.append(comment) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index e4f32aa57d..785294b81a 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -587,3 +587,17 @@ def test_inline_comment(): assert "a = i + 1" in assignment.debug_string() assert assignment.preceding_comment == "" assert assignment.inline_comment == "Third line of inline comment" + + +def test_lost_program_comments(): + """Test that the FortranReader doesn't lose comments after the + declarations when reading a Program.""" + reader = FortranReader(ignore_comments=False) + code = """program a + integer :: i ! inline here + ! Comment here + i = 1 + end program""" + psyir = reader.psyir_from_source(code) + assignment = psyir.walk(Assignment)[0] + assert assignment.preceding_comment == "Comment here" From f0e41f059d9dc7be5b2e1dfb4fbc252d70a4f87a Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 11 Jun 2025 16:34:08 +0100 Subject: [PATCH 08/16] Next step code, implementation is TODO --- src/psyclone/psyir/nodes/codeblock.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/psyclone/psyir/nodes/codeblock.py b/src/psyclone/psyir/nodes/codeblock.py index 53942e8bd6..c2995fc681 100644 --- a/src/psyclone/psyir/nodes/codeblock.py +++ b/src/psyclone/psyir/nodes/codeblock.py @@ -207,6 +207,13 @@ def get_symbol_names(self) -> List[str]: for part in node.items: if part.items[1]: result.append(part.items[1]) + # 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): + # FIXME Check if this comment is a directive. + assert False + return result def reference_accesses(self) -> VariablesAccessMap: From 20d19585283c2120643070427a1d648f8ea24cd1 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 18 Jun 2025 15:05:15 +0100 Subject: [PATCH 09/16] Codeblocks can now report symbols used inside directives --- src/psyclone/psyir/nodes/codeblock.py | 17 +++++++++-- .../tests/psyir/nodes/codeblock_test.py | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/psyclone/psyir/nodes/codeblock.py b/src/psyclone/psyir/nodes/codeblock.py index c2995fc681..fa60987b39 100644 --- a/src/psyclone/psyir/nodes/codeblock.py +++ b/src/psyclone/psyir/nodes/codeblock.py @@ -38,10 +38,11 @@ ''' This module contains the CodeBlock node implementation.''' +import re from enum import Enum from typing import List -from fparser.two import Fortran2003 +from fparser.two import Fortran2003, pattern_tools from fparser.two.utils import walk from psyclone.core import AccessType, Signature, VariablesAccessMap from psyclone.psyir.nodes.statement import Statement @@ -211,8 +212,18 @@ def get_symbol_names(self) -> List[str]: # comment string and return any names that match a symbol in the # symbol table. for node in walk(parse_tree, Fortran2003.Comment): - # FIXME Check if this comment is a directive. - assert False + string_rep = node.tostr() + # Directives start with a $ + if string_rep.lstrip()[0:2] != "!$": + continue + string_rep = string_rep[2:] + pattern = pattern_tools.name.get_compiled() + matches = re.findall(pattern, string_rep) + scope = self.scope + for match in matches: + sym = scope.symbol_table.lookup(match, otherwise=None) + if sym: + result.append(sym.name) return result diff --git a/src/psyclone/tests/psyir/nodes/codeblock_test.py b/src/psyclone/tests/psyir/nodes/codeblock_test.py index 599c753ec6..4cd3716cab 100644 --- a/src/psyclone/tests/psyir/nodes/codeblock_test.py +++ b/src/psyclone/tests/psyir/nodes/codeblock_test.py @@ -40,6 +40,7 @@ import pytest from fparser.common.readfortran import FortranStringReader +from psyclone.psyir.frontend.fortran import FortranReader from psyclone.psyir.nodes import CodeBlock from psyclone.psyir.nodes.node import colored from psyclone.errors import GenerationError @@ -133,6 +134,34 @@ def test_codeblock_get_symbol_names(parser): assert "that_is_true" in sym_names +def test_codeblock_get_symbol_names_comments_and_directives(): + ''' + Test that Codeblock.get_symbol_names returns any symbols in directives. + ''' + code = """ + subroutine mytest + integer :: i + + ! Here is a comment + !$omp dir private(i) + i = i + 1 + end subroutine""" + + reader = FortranReader(ignore_comments=False, + ignore_directives=False) + psyir = reader.psyir_from_source(code) + block = psyir.walk(CodeBlock)[0] + sym_names = block.get_symbol_names() + assert "i" in sym_names + assert "omp" not in sym_names + assert "dir" not in sym_names + assert "private" not in sym_names + assert "Here" not in sym_names + assert "is" not in sym_names + assert "a" not in sym_names + assert "comment" not in sym_names + + def test_codeblock_ref_accesses(parser): '''Test that the reference_accesses() method works as expected. From a3a5ead5ea0bd079213a09a19a977f5f03bea9cd Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 25 Jun 2025 10:13:00 +0100 Subject: [PATCH 10/16] Fixed missing coverage of comments that are codeblocks - this only happens for the final comment in a scope if its kept --- src/psyclone/tests/psyir/nodes/codeblock_test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/psyclone/tests/psyir/nodes/codeblock_test.py b/src/psyclone/tests/psyir/nodes/codeblock_test.py index 4cd3716cab..07a266e8de 100644 --- a/src/psyclone/tests/psyir/nodes/codeblock_test.py +++ b/src/psyclone/tests/psyir/nodes/codeblock_test.py @@ -140,15 +140,16 @@ def test_codeblock_get_symbol_names_comments_and_directives(): ''' code = """ subroutine mytest - integer :: i + integer :: i, is - ! Here is a comment !$omp dir private(i) i = i + 1 + ! Here is a comment end subroutine""" reader = FortranReader(ignore_comments=False, - ignore_directives=False) + ignore_directives=False, + last_comments_as_codeblocks=True) psyir = reader.psyir_from_source(code) block = psyir.walk(CodeBlock)[0] sym_names = block.get_symbol_names() From 510bb60c9d37cc688e7d7299ac8dc03861d0417a Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Mon, 30 Jun 2025 13:17:46 +0100 Subject: [PATCH 11/16] Review fixes --- doc/user_guide/psyclone_command.rst | 17 ++- src/psyclone/generator.py | 2 + src/psyclone/parse/utils.py | 2 +- src/psyclone/psyir/frontend/fparser2.py | 101 +++++++++++------- .../alg_invoke_2_psy_call_trans_test.py | 20 +--- src/psyclone/tests/parse/utils_test.py | 23 ++++ 6 files changed, 109 insertions(+), 56 deletions(-) diff --git a/doc/user_guide/psyclone_command.rst b/doc/user_guide/psyclone_command.rst index 1d542ac0e9..6eb36e8565 100644 --- a/doc/user_guide/psyclone_command.rst +++ b/doc/user_guide/psyclone_command.rst @@ -54,7 +54,8 @@ by the command: usage: psyclone [-h] [-v] [-c CONFIG] [-s SCRIPT] [-I INCLUDE] [-l {off,all,output}] [-p {invokes,routines,kernels}] [--backend {enable-validation,disable-validation}] [-o OUTPUT_FILE] [-api DSL] [-oalg OUTPUT_ALGORITHM_FILE] [-opsy OUTPUT_PSY_FILE] [-okern OUTPUT_KERNEL_PATH] [-d DIRECTORY] [-dm] [-nodm] - [--kernel-renaming {multiple,single}] [--log-level {OFF,DEBUG,INFO,WARNING,ERROR,CRITICAL}] [--log-file LOG_FILE] + [--kernel-renaming {multiple,single}] [--log-level {OFF,DEBUG,INFO,WARNING,ERROR,CRITICAL}] + [--log-file LOG_FILE] [--keep-comments] [--keep-directives] filename Transform a file using the PSyclone source-to-source Fortran compiler @@ -103,6 +104,8 @@ by the command: --log-level {OFF,DEBUG,INFO,WARNING,ERROR,CRITICAL} sets the level of the logging (defaults to OFF). --log-file LOG_FILE sets the output file to use for logging (defaults to stderr). + --keep-comments keeps comments from the original code (defaults to False). + --keep-directives keeps directives from the original code (defaults to False). Basic Use --------- @@ -438,3 +441,15 @@ By default the output from the logging goes into stderr. To control the logging output, PSyclone provides the ``--log-file`` option. If this is set, the logging output will instead be directed to the provided file. + +Keeping Comments and Directives +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +PSyclone can now keep comments and directives from the original code, with +some limitations: + + 1. Comments that appear after all statements in a routine are not currently + kept. + 2. Directives are kept as ``CodeBlock`` nodes in the PSyIR which means + some transformations will be unavailable on regions containing these + nodes. diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index 3abe7c634b..082b890b6f 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -35,6 +35,7 @@ # Modified A. J. Voysey, Met Office # Modified J. Henrichs, Bureau of Meteorology # Modified A. R. Pirrie, Met Office +# Modified A. B. G. Chalk, STFC Daresbury Lab ''' This module provides the PSyclone 'main' routine which is intended @@ -213,6 +214,7 @@ def generate(filename, api="", kernel_paths=None, script_name=None, Tuple[:py:class:`fparser.one.block_statements.BeginSource`, str] :param keep_comments: whether to keep comments from the original source. :param keep_directives: whether to keep directives from the original + source. :raises GenerationError: if an invalid API is specified. :raises GenerationError: if an invalid kernel-renaming scheme is specified. diff --git a/src/psyclone/parse/utils.py b/src/psyclone/parse/utils.py index 81c0e12259..5c28df72d5 100644 --- a/src/psyclone/parse/utils.py +++ b/src/psyclone/parse/utils.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Authors: R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab +# Modified: A. B. G. Chalk, STFC Daresbury Lab '''Utility module containing classes and functions that are used by the parser modules. @@ -119,7 +120,6 @@ def parse_fp2(filename, ignore_comments: bool = True): # We get the directories to search for any Fortran include files from # our configuration object. config = Config.get() - # FIXME Unit test. try: reader = FortranFileReader(filename, include_dirs=config.include_paths, ignore_comments=ignore_comments) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 99cf516046..fa59a39f1e 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2849,6 +2849,36 @@ def _process_precision(type_spec, psyir_parent): return _kind_find_or_create(str(kind_names[0]), symbol_table) + def _add_comments_to_tree(self, parent: Node, preceding_comments, + psy_child: Node) -> None: + ''' + Add the comments in preceding_comments into the PsyIR tree. + + :param parent: Parent node in the PSyIR we are constructing. + :param preceding_comments: List of comment nodes to add to the tree. + :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. + 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) + psy_child.preceding_comment += self._comments_list_to_string( + preceding_comments + ) + + def process_nodes(self, parent, nodes): ''' Create the PSyIR of the supplied list of nodes in the @@ -2891,26 +2921,8 @@ def process_nodes(self, parent, nodes): # Add the comments to nodes that support it and reset the # list of comments if isinstance(psy_child, CommentableMixin): - for comment in preceding_comments[:]: - # If the comment is a directive and we - # keep_directives then create a CodeBlock for - # the directive. - if (not self._ignore_directives and - comment.tostr().startswith("!$")): - block = self.nodes_to_code_block(parent, - [comment]) - # Precede this with any comments as relevant. - 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) - psy_child.preceding_comment\ - += self._comments_list_to_string( - preceding_comments) + self._add_comments_to_tree(parent, preceding_comments, + psy_child) preceding_comments = [] if isinstance(psy_child, CommentableMixin): if child.item is not None: @@ -5345,6 +5357,32 @@ def _process_args(self, node, call, canonicalise=None): return call + def _get_lost_declaration_comments(self, decl_list)\ + -> list[Fortran2003.Comment]: + '''Finds comments from the variable declaration that the default + declaration handler doesn't keep. + + :param decl_list: The declaration list being processed. + + :returns: a list of comments that have been missed. + ''' + lost_comments = [] + if len(decl_list) != 0 and isinstance(decl_list[-1], + Fortran2003.Implicit_Part): + for comment in walk(decl_list[-1], Fortran2003.Comment): + if len(comment.tostr()) == 0: + continue + if self._last_psyir_parsed_and_span is not None: + last_symbol, last_span \ + = self._last_psyir_parsed_and_span + if (last_span is not None + and last_span[1] == comment.item.span[0]): + last_symbol.inline_comment\ + = self._comment_to_string(comment) + continue + lost_comments.append(comment) + return lost_comments + def _subroutine_handler(self, node, parent): '''Transforms an fparser2 Subroutine_Subprogram or Function_Subprogram statement into a PSyIR Routine node. @@ -5456,24 +5494,11 @@ def _subroutine_handler(self, node, parent): arg_list = [] self.process_declarations(routine, decl_list, arg_list) - # 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 = [] - if len(decl_list) != 0 and isinstance(decl_list[-1], - Fortran2003.Implicit_Part): - for comment in walk(decl_list[-1], Fortran2003.Comment): - if len(comment.tostr()) == 0: - continue - if self._last_psyir_parsed_and_span is not None: - last_symbol, last_span \ - = self._last_psyir_parsed_and_span - if (last_span is not None - and last_span[1] == comment.item.span[0]): - last_symbol.inline_comment\ - = self._comment_to_string(comment) - continue - lost_comments.append(comment) + # fparser puts comments after the 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) # Check whether the function-stmt has a prefix specifying the # return type (other prefixes are handled in diff --git a/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py b/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py index d9091ad414..48ce526eb9 100644 --- a/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py +++ b/src/psyclone/tests/domain/common/transformations/alg_invoke_2_psy_call_trans_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Authors: R. W. Ford and A. R. Porter, STFC Daresbury Laboratory. +# Modified: A. B. G. Chalk, STFC Daresbury Laboratory. ''' Module containing pytest unit tests for the AlgInvoke2PSyCallTrans transformation. @@ -592,14 +593,9 @@ def test_ai2psycall_remove_imported_symbols(fortran_reader): def test_ai2psycall_keep_comments(): - '''Check that the apply() method works as expected when the invoke has - a single kernel with multiple fields of the same name. Also check - that the apply() method creates the required routine and container - symbols if they have not already been created. Also check that the - apply() method removes the invoke symbol from the appropriate - symbol table. Use GOceanAlgInvoke2PSyCallTrans as - AlgInvoke2PSyCallTrans is abstract. - + '''Check that the apply method doesn't strip out the comments when + FortranReader had ignore_comments=False, and that the comments appear in + the expected place. ''' code = ( "subroutine alg1()\n" @@ -615,14 +611,6 @@ def test_ai2psycall_keep_comments(): psyir = fortran_reader.psyir_from_source(code) AlgTrans().apply(psyir) invoke = psyir.children[0][0] - - assert isinstance(invoke, AlgorithmInvokeCall) - assert len(psyir.walk(AlgorithmInvokeCall)) == 1 - assert len(psyir.walk(KernelFunctor)) == 1 - assert "invoke" in invoke.scope.symbol_table._symbols - - # Don't call create_psylayer_symbol_root_names() here. This is to - # check that the transformation creates the names if needed. trans = GOceanAlgInvoke2PSyCallTrans() trans.apply(invoke) invoke = psyir.children[0][0] diff --git a/src/psyclone/tests/parse/utils_test.py b/src/psyclone/tests/parse/utils_test.py index 61b50fd51e..7f6e0ce584 100644 --- a/src/psyclone/tests/parse/utils_test.py +++ b/src/psyclone/tests/parse/utils_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Authors R. W. Ford, A. R. Porter and N. Nobre, STFC Daresbury Lab +# Modified A. B. G. Chalk, STFC Daresbury Lab '''A module to perform pytest unit tests on the parse/utils.py file. @@ -41,6 +42,9 @@ import pytest +from fparser.two import Fortran2003 +from fparser.two.utils import walk + from psyclone.parse.utils import check_line_length, parse_fp2, ParseError from psyclone.errors import InternalError @@ -119,3 +123,22 @@ def test_parsefp2_invalid_fortran(tmpdir): with pytest.raises(ParseError) as excinfo: _ = parse_fp2(my_file) assert "Syntax error in file" in str(excinfo.value) + + +def test_parsefp2_ignore_comments(tmpdir): + '''Test that ignore_comments=False option works for parse_fp2.''' + code = """subroutine test + integer :: i + + ! Here is a comment + i = 1 + end subroutine""" + my_file = str(tmpdir.join("comment.f90")) + with open(my_file, "w", encoding="utf-8") as ffile: + ffile.write(code) + ffile.close() + + out = parse_fp2(my_file, ignore_comments=False) + comments = walk(out, Fortran2003.Comment) + assert len(comments) == 2 + assert str(comments[1]) == "! Here is a comment" From 19c312916fc637e8930065ed7878d698eb04ef9d Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Mon, 30 Jun 2025 13:18:15 +0100 Subject: [PATCH 12/16] linting --- src/psyclone/psyir/frontend/fparser2.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index fa59a39f1e..5d2106e2a5 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2878,7 +2878,6 @@ def _add_comments_to_tree(self, parent: Node, preceding_comments, preceding_comments ) - def process_nodes(self, parent, nodes): ''' Create the PSyIR of the supplied list of nodes in the @@ -5495,7 +5494,7 @@ def _subroutine_handler(self, node, parent): self.process_declarations(routine, decl_list, arg_list) # fparser puts comments after the declarations as part of - # the declarations, but in PSyclone they need to be a + # 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) From fcae91dd0c7f782b6d27bde1b7e12e8a150d75d3 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 16 Jul 2025 13:11:49 +0100 Subject: [PATCH 13/16] Changes for the review --- .../raise_psyir_2_gocean_kern_trans.py | 1 - src/psyclone/generator.py | 4 ++- src/psyclone/psyir/frontend/fparser2.py | 34 +++++++++++++++---- .../raise_psyir_2_alg_trans_test.py | 8 ++--- src/psyclone/tests/generator_test.py | 9 ++++- .../psyir/frontend/fparser2_comment_test.py | 20 +++++++++++ .../tests/psyir/nodes/codeblock_test.py | 2 ++ .../lfric/1_single_invoke_with_omp_dir.f90 | 4 ++- 8 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py b/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py index 7a3715bd5f..7c400ed56c 100644 --- a/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py +++ b/src/psyclone/domain/gocean/transformations/raise_psyir_2_gocean_kern_trans.py @@ -244,7 +244,6 @@ def apply(self, node, options=None): symbol_table=gotable.detach()) for child in routine.pop_all_children(): gokernsched.addchild(child) - routine.replace_with(gokernsched) diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index 082b890b6f..3ab487e95e 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -522,7 +522,9 @@ def main(arguments): ) parser.add_argument( "--keep-comments", default=False, action="store_true", - help="keeps comments from the original code (defaults to False)." + help="keeps comments from the original code (defaults to False). " + "Directives are not kept with this option (use " + "--keep-directives)." ) parser.add_argument( "--keep-directives", default=False, action="store_true", diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 5d2106e2a5..5231803f6a 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2852,7 +2852,11 @@ def _process_precision(type_spec, psyir_parent): def _add_comments_to_tree(self, parent: Node, preceding_comments, psy_child: Node) -> None: ''' - Add the comments in preceding_comments into the PsyIR tree. + Add the provided fparser2 comments to the PsyIR tree. + + If we are not ignoring directives, then these are placed into their + own CodeBlock nodes. All comments are attached to the appropriate + PSyIR node. :param parent: Parent node in the PSyIR we are constructing. :param preceding_comments: List of comment nodes to add to the tree. @@ -2863,6 +2867,9 @@ def _add_comments_to_tree(self, parent: Node, 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]) @@ -2874,6 +2881,7 @@ def _add_comments_to_tree(self, parent: Node, preceding_comments, 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 ) @@ -2949,6 +2957,8 @@ def process_nodes(self, parent, nodes): # them. if not self._ignore_directives and len(preceding_comments) != 0: for comment in preceding_comments[:]: + # TODO: fparser #469. This only captures some free-form + # directives. if comment.tostr().startswith("!$"): self.nodes_to_code_block(parent, [comment]) preceding_comments.remove(comment) @@ -5359,26 +5369,38 @@ def _process_args(self, node, call, canonicalise=None): def _get_lost_declaration_comments(self, decl_list)\ -> list[Fortran2003.Comment]: '''Finds comments from the variable declaration that the default - declaration handler doesn't keep. + declaration handler doesn't keep. Any comments that appear after + the final declaration but before the first PSyIR node created are + lost by the declaration handler so are returned from this function. + If the function finds a comment that should be an inline comment on + the last declaration, it attaches that comment to the declaration. :param decl_list: The declaration list being processed. + :type decl_list: List[:py:class:`Fortran2003.Specification_Part`] :returns: a list of comments 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): if len(comment.tostr()) == 0: continue if self._last_psyir_parsed_and_span is not None: last_symbol, last_span \ = self._last_psyir_parsed_and_span + # If the last node parsed is the last symbol declaration + # and the comment is attached to the declaration in the + # fparser tree we add the comment to the declaration. if (last_span is not None and last_span[1] == comment.item.span[0]): last_symbol.inline_comment\ = self._comment_to_string(comment) continue + # Otherwise the comment is not an inline comment, so we append + # it to the lost comments list. lost_comments.append(comment) return lost_comments @@ -5493,10 +5515,10 @@ def _subroutine_handler(self, node, parent): arg_list = [] self.process_declarations(routine, decl_list, arg_list) - # fparser puts comments after the 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. + # fparser puts comments that occur immediately after the + # 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) # Check whether the function-stmt has a prefix specifying the diff --git a/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py b/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py index 8303f51ecc..1338105bad 100644 --- a/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py +++ b/src/psyclone/tests/domain/common/transformations/raise_psyir_2_alg_trans_test.py @@ -32,7 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author R. W. Ford, STFC Daresbury Lab -# Modified: A. R. Porter and S. Siso, STFC Daresbury Lab +# Modified: A. R. Porter, S. Siso and A. B. G. Chalk, STFC Daresbury Lab '''Module containing tests for the translation of PSyIR to PSyclone Algorithm PSyIR. @@ -539,10 +539,8 @@ def test_multi_name(): def test_apply_keep_comments(): - '''Test that an invoke with an array reference argument is transformed - into PSyclone-specific AlgorithmInvokeCall and KernelFunctor - classes. - + '''Test that comment nodes are correctly kept on an invoke when the + RaisePSyIR2AlgTrans is applied. ''' code = ( "subroutine alg()\n" diff --git a/src/psyclone/tests/generator_test.py b/src/psyclone/tests/generator_test.py index 5e9eb1a0f5..60a6a12512 100644 --- a/src/psyclone/tests/generator_test.py +++ b/src/psyclone/tests/generator_test.py @@ -874,6 +874,10 @@ def test_keep_comments_and_keep_directives(capsys, caplog, tmpdir_factory, in caplog.record_tuples[0][2]) output, _ = capsys.readouterr() + +def test_keep_comments_lfric(capsys, monkeypatch): + '''Test that the LFRic API correctly keeps comments and directives + when applied the appropriate arguments.''' # Test this for LFRIC algorithm domain. monkeypatch.setattr(generator, "LFRIC_TESTING", True) filename = os.path.join(LFRIC_BASE_PATH, @@ -890,7 +894,10 @@ def test_keep_comments_and_keep_directives(capsys, caplog, tmpdir_factory, assert "! Here is a comment" in output assert "!$omp barrier" in output - # FIXME Test Gocean + +def test_keep_comments_gocean(capsys, monkeypatch): + '''Test that the GOcean API correctly keeps comments and directives + when applied the appropriate arguments.''' filename = os.path.join(GOCEAN_BASE_PATH, "single_invoke.f90") main([filename, "-api", "gocean"]) output, _ = capsys.readouterr() diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index 785294b81a..bd82cd51da 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -595,9 +595,29 @@ def test_lost_program_comments(): reader = FortranReader(ignore_comments=False) code = """program a integer :: i ! inline here + ! Comment here i = 1 end program""" psyir = reader.psyir_from_source(code) + assert (psyir.children[0].symbol_table.lookup("i").inline_comment == + "inline here") assignment = psyir.walk(Assignment)[0] assert assignment.preceding_comment == "Comment here" + + +def test_directive_at_end(): + """Test that the FortranReader stores a directive after all + other code in a subroutine.""" + + code = """subroutine x + integer :: i + i = i + 1 + !$omp barrier + end subroutine""" + reader = FortranReader(ignore_comments=False, ignore_directives=False) + psyir = reader.psyir_from_source(code) + routine = psyir.children[0] + # The directive is a codeblock + assert isinstance(routine.children[-1], CodeBlock) + assert routine.children[-1].debug_string() == "!$omp barrier\n" diff --git a/src/psyclone/tests/psyir/nodes/codeblock_test.py b/src/psyclone/tests/psyir/nodes/codeblock_test.py index 2035764b54..3c0a329179 100644 --- a/src/psyclone/tests/psyir/nodes/codeblock_test.py +++ b/src/psyclone/tests/psyir/nodes/codeblock_test.py @@ -157,6 +157,8 @@ def test_codeblock_get_symbol_names_comments_and_directives(): assert "omp" not in sym_names assert "dir" not in sym_names assert "private" not in sym_names + block = psyir.walk(CodeBlock)[1] + sym_names = block.get_symbol_names() assert "Here" not in sym_names assert "is" not in sym_names assert "a" not in sym_names diff --git a/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 b/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 index 51c8e9a9ac..ec036f104e 100644 --- a/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 +++ b/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 @@ -33,10 +33,12 @@ ! ----------------------------------------------------------------------------- ! Author R. W. Ford, STFC Daresbury Lab ! Modified I. Kavcic, Met Office +! Modified A. B. G. Chalk, STFC Daresbury Lab program single_invoke - ! Description: single function specified in an invoke call + ! Description: single function specified in an invoke call. Added directive + ! and comment to test keep-comments and keep-directives functionality. use constants_mod, only: r_def use field_mod, only: field_type use testkern_mod, only: testkern_type From 51a9648b5bef28463dcf0ceb9fb6c7fdc8840d27 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 16 Jul 2025 13:22:58 +0100 Subject: [PATCH 14/16] Add user guide changes --- doc/user_guide/psyclone_command.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/doc/user_guide/psyclone_command.rst b/doc/user_guide/psyclone_command.rst index 6eb36e8565..e92ae359a4 100644 --- a/doc/user_guide/psyclone_command.rst +++ b/doc/user_guide/psyclone_command.rst @@ -104,7 +104,8 @@ by the command: --log-level {OFF,DEBUG,INFO,WARNING,ERROR,CRITICAL} sets the level of the logging (defaults to OFF). --log-file LOG_FILE sets the output file to use for logging (defaults to stderr). - --keep-comments keeps comments from the original code (defaults to False). + --keep-comments keeps comments from the original code (defaults to False). + Directives are not kept with this option (use --keep-directives). --keep-directives keeps directives from the original code (defaults to False). Basic Use @@ -452,4 +453,9 @@ some limitations: kept. 2. Directives are kept as ``CodeBlock`` nodes in the PSyIR which means some transformations will be unavailable on regions containing these - nodes. + nodes. Also PSyclone will not know any details about these nodes + (including that they contain directives) but this functionality will + be improved over time. + +Note that using the ``keep-comments`` option alone means that any comments +that PSyclone interprets as directives will be lost from the input. From c5c6cd5cd0e0a249835b15708ffa88c38bef228d Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Thu, 17 Jul 2025 14:29:40 +0100 Subject: [PATCH 15/16] Refactoring of fparser2 to reduce code duplication --- src/psyclone/psyir/frontend/fparser2.py | 28 ++++++++----------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 4243e8d8b4..385d182826 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -5366,7 +5366,8 @@ def _process_args(self, node, call, canonicalise=None): return call - def _get_lost_declaration_comments(self, decl_list)\ + def _get_lost_declaration_comments(self, decl_list, + attach_trailing_symbol: bool = True)\ -> list[Fortran2003.Comment]: '''Finds comments from the variable declaration that the default declaration handler doesn't keep. Any comments that appear after @@ -5377,6 +5378,9 @@ def _get_lost_declaration_comments(self, decl_list)\ :param decl_list: The declaration list being processed. :type decl_list: List[:py:class:`Fortran2003.Specification_Part`] + :param attach_trailing_symbol: whether to attach the inline comment on + the last symbol to the tree or not. + Defaults to True :returns: a list of comments that have been missed. ''' @@ -5396,8 +5400,9 @@ def _get_lost_declaration_comments(self, decl_list)\ # fparser tree we add the comment to the declaration. if (last_span is not None and last_span[1] == comment.item.span[0]): - last_symbol.inline_comment\ - = self._comment_to_string(comment) + if attach_trailing_symbol: + last_symbol.inline_comment\ + = self._comment_to_string(comment) continue # Otherwise the comment is not an inline comment, so we append # it to the lost comments list. @@ -5674,22 +5679,7 @@ 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 = [] - if len(decl_list) != 0 and isinstance(decl_list[-1], - Fortran2003.Implicit_Part): - for comment in walk(decl_list[-1], Fortran2003.Comment): - if len(comment.tostr()) == 0: - continue - if self._last_psyir_parsed_and_span is not None: - last_symbol, last_span \ - = self._last_psyir_parsed_and_span - # If the comment goes on the last parsed psyir node - # or symbol, then we don't need to do anything with - # it as its handled by process_declarations. - if (last_span is not None - and last_span[1] == comment.item.span[0]): - continue - lost_comments.append(comment) + lost_comments = self._get_lost_declaration_comments(decl_list, False) try: prog_exec = _first_type_match(node.content, From 5159a404de6420416f1aa5ecf998ace47b860f0d Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Thu, 17 Jul 2025 16:23:35 +0100 Subject: [PATCH 16/16] Added keep-comments and keep-directives to some examples --- examples/gocean/eg6/Makefile | 2 +- examples/lfric/eg17/full_example/Makefile | 2 +- examples/nemo/eg5/Makefile | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/gocean/eg6/Makefile b/examples/gocean/eg6/Makefile index 59d92dce09..f552e7ee05 100644 --- a/examples/gocean/eg6/Makefile +++ b/examples/gocean/eg6/Makefile @@ -42,4 +42,4 @@ ENV = PSYCLONE_CONFIG=${PSYCLONE_DIR}/config/psyclone.cfg transform: ${PSYCLONE} -nodm -s ./backends_transform.py -api gocean alg.f90 \ - -oalg /dev/null -opsy /dev/null + -oalg /dev/null -opsy /dev/null --keep-comments diff --git a/examples/lfric/eg17/full_example/Makefile b/examples/lfric/eg17/full_example/Makefile index 8b2e236c03..5847e0d0d7 100644 --- a/examples/lfric/eg17/full_example/Makefile +++ b/examples/lfric/eg17/full_example/Makefile @@ -87,7 +87,7 @@ main_alg.f90: main_psy.f90 transform: main_psy.f90 %_psy.f90: %.x90 - ${PSYCLONE} -api lfric -opsy $*_psy.f90 -oalg $*_alg.f90 $< + ${PSYCLONE} -api lfric --keep-comments --keep-directives -opsy $*_psy.f90 -oalg $*_alg.f90 $< allclean: clean make -C $(LFRIC_PATH) allclean diff --git a/examples/nemo/eg5/Makefile b/examples/nemo/eg5/Makefile index c62c147a99..115f7c5309 100644 --- a/examples/nemo/eg5/Makefile +++ b/examples/nemo/eg5/Makefile @@ -92,7 +92,7 @@ transform: kernels # Need `-l all` to ensure line-lengths in generated code are less than the # standard-mandated 132 chars. kernels: extract_kernels.py - $(PSYCLONE) -l all -s ./extract_kernels.py -o psy.f90 ../code/tra_adv.F90 + $(PSYCLONE) --keep-comments --keep-directives -l all -s ./extract_kernels.py -o psy.f90 ../code/tra_adv.F90 $(EXTRACT_DIR)/$(LIB_NAME): ${MAKE} -C $(EXTRACT_DIR)