diff --git a/doc/user_guide/psyclone_command.rst b/doc/user_guide/psyclone_command.rst index 1d542ac0e9..e92ae359a4 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,9 @@ 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). + Directives are not kept with this option (use --keep-directives). + --keep-directives keeps directives from the original code (defaults to False). Basic Use --------- @@ -438,3 +442,20 @@ 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. 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. 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) 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..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 @@ -231,6 +231,10 @@ def apply(self, node, options=None): interface=interface) psy_call = Call.create(routine_symbol, arguments) + # Copy over the comments. + 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/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/lfric/transformations/raise_psyir_2_lfric_alg_trans.py b/src/psyclone/domain/lfric/transformations/raise_psyir_2_lfric_alg_trans.py index 056a02315a..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 @@ -104,6 +104,11 @@ def apply(self, call, index, options=None): invoke_call = LFRicAlgorithmInvokeCall.create( call.routine.symbol, calls, index, name=call_name) + + # Copy across any 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/generator.py b/src/psyclone/generator.py index 9a7b3b835d..3ab487e95e 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 @@ -175,7 +176,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 +212,9 @@ 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 + source. :raises GenerationError: if an invalid API is specified. :raises GenerationError: if an invalid kernel-renaming scheme is specified. @@ -274,17 +280,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 +520,16 @@ 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). " + "Directives are not kept with this option (use " + "--keep-directives)." + ) + 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 +606,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 +644,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 +756,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 +771,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 +799,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..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. @@ -104,11 +105,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. @@ -118,7 +121,8 @@ def parse_fp2(filename): # our configuration object. config = Config.get() 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 a8af08d570..385d182826 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2528,7 +2528,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) @@ -2847,6 +2849,43 @@ 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 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. + :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 + ) + def process_nodes(self, parent, nodes): ''' Create the PSyIR of the supplied list of nodes in the @@ -2889,9 +2928,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): - 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: @@ -2912,10 +2950,19 @@ 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) + # 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[:]: + # 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) + if self._last_comments_as_codeblocks and len(preceding_comments) != 0: self.nodes_to_code_block(parent, preceding_comments) @@ -5319,6 +5366,49 @@ 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]: + '''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 + 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`] + :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. + ''' + 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]): + 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. + 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. @@ -5430,24 +5520,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 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 # return type (other prefixes are handled in @@ -5599,6 +5676,11 @@ 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 = self._get_lost_declaration_comments(decl_list, False) + try: prog_exec = _first_type_match(node.content, Fortran2003.Execution_Part) @@ -5607,7 +5689,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/psyir/nodes/codeblock.py b/src/psyclone/psyir/nodes/codeblock.py index 05b4287205..4ac22c0e17 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 @@ -207,6 +208,23 @@ 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): + 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 def reference_accesses(self) -> VariablesAccessMap: 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..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. @@ -44,6 +45,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 +590,29 @@ 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 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" + " 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] + 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..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. @@ -536,3 +536,30 @@ 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 comment nodes are correctly kept on an invoke when the + RaisePSyIR2AlgTrans is applied. + ''' + 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..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 @@ -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" diff --git a/src/psyclone/tests/generator_test.py b/src/psyclone/tests/generator_test.py index 272e52e578..60a6a12512 100644 --- a/src/psyclone/tests/generator_test.py +++ b/src/psyclone/tests/generator_test.py @@ -807,6 +807,107 @@ 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() + + +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, + "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 + + +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() + 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. 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" 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..bd82cd51da 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -509,10 +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") EXPECTED_WITH_DIRECTIVES = """subroutine test_sub() @@ -529,10 +529,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) @@ -591,3 +587,37 @@ 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) + 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 7d680e8c35..3c0a329179 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,37 @@ 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, is + + !$omp dir private(i) + i = i + 1 + ! Here is a comment + end subroutine""" + + reader = FortranReader(ignore_comments=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() + assert "i" in sym_names + 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 + assert "comment" not in sym_names + + def test_codeblock_ref_accesses(parser): '''Test that the reference_accesses() method works as expected. 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..ec036f104e --- /dev/null +++ b/src/psyclone/tests/test_files/lfric/1_single_invoke_with_omp_dir.f90 @@ -0,0 +1,57 @@ +! ----------------------------------------------------------------------------- +! 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 +! Modified A. B. G. Chalk, STFC Daresbury Lab + +program single_invoke + + ! 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 + + 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