From 9d9f15c00b61ca4d4c36457cff1b26a1df487a96 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 2 Dec 2024 19:23:03 +0100 Subject: [PATCH 01/20] #1247 Upstreaming comments in PSyIR edits. --- src/psyclone/psyir/backend/fortran.py | 13 +- src/psyclone/psyir/frontend/fortran.py | 18 +- src/psyclone/psyir/frontend/fparser2.py | 207 ++++++++++++++---- src/psyclone/psyir/symbols/datasymbol.py | 4 +- src/psyclone/psyir/symbols/datatypes.py | 22 +- src/psyclone/psyir/symbols/symbol.py | 4 +- .../tests/psyir/symbols/datatype_test.py | 6 +- 7 files changed, 211 insertions(+), 63 deletions(-) diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index 743352628f..fedd1790f6 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -536,6 +536,11 @@ def gen_vardecl(self, symbol, include_visibility=False): f"and should not be provided to 'gen_vardecl'." ) + result = "" + if len(symbol.preceding_comment) > 0: + for line in symbol.preceding_comment.splitlines(): + result += f"{self._nindent}{self._COMMENT_PREFIX}{line}\n" + # Whether we're dealing with an array declaration and, if so, the # shape of that array. if isinstance(symbol.datatype, ArrayType): @@ -554,10 +559,12 @@ def gen_vardecl(self, symbol, include_visibility=False): # blocks appearing in SAVE statements. decln = add_accessibility_to_unsupported_declaration( symbol) - return f"{self._nindent}{decln}\n" + result += f"{self._nindent}{decln}\n" + return result decln = symbol.datatype.declaration - return f"{self._nindent}{decln}\n" + result += f"{self._nindent}{decln}\n" + return result # The Fortran backend only handles UnsupportedFortranType # declarations. raise VisitorError( @@ -566,7 +573,7 @@ def gen_vardecl(self, symbol, include_visibility=False): f"supported by the Fortran backend.") datatype = gen_datatype(symbol.datatype, symbol.name) - result = f"{self._nindent}{datatype}" + result += f"{self._nindent}{datatype}" if ArrayType.Extent.DEFERRED in array_shape: # A 'deferred' array extent means this is an allocatable array diff --git a/src/psyclone/psyir/frontend/fortran.py b/src/psyclone/psyir/frontend/fortran.py index 523712035a..6140f2ae6b 100644 --- a/src/psyclone/psyir/frontend/fortran.py +++ b/src/psyclone/psyir/frontend/fortran.py @@ -85,11 +85,14 @@ def validate_name(name: str): raise ValueError( f"Invalid Fortran name '{name}' found.") - def psyir_from_source(self, source_code: str, free_form: bool = True): + def psyir_from_source(self, source_code: str, free_form: bool = True, + ignore_comments: bool = True): ''' Generate the PSyIR tree representing the given Fortran source code. :param source_code: text representation of the code to be parsed. :param free_form: If parsing free-form code or not (default True). + :param ignore_comments: If comments should be ignored or not + (default True). :returns: PSyIR representing the provided Fortran source code. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -97,7 +100,8 @@ def psyir_from_source(self, source_code: str, free_form: bool = True): ''' SYMBOL_TABLES.clear() string_reader = FortranStringReader( - source_code, include_dirs=Config.get().include_paths) + source_code, include_dirs=Config.get().include_paths, + ignore_comments=ignore_comments) # Set reader to free format. string_reader.set_format(FortranFormat(free_form, False)) parse_tree = self._parser(string_reader) @@ -194,7 +198,8 @@ def psyir_from_statement(self, source_code: str, self._processor.process_nodes(fake_parent, exec_part.children) return fake_parent[0].detach() - def psyir_from_file(self, file_path, free_form=True): + def psyir_from_file(self, file_path, free_form=True, + ignore_comments: bool = True): ''' Generate the PSyIR tree representing the given Fortran file. :param file_path: path of the file to be read and parsed. @@ -203,6 +208,10 @@ def psyir_from_file(self, file_path, free_form=True): :param free_form: If parsing free-form code or not (default True). :type free_form: bool + :param ignore_comments: If comments should be ignored or not + (default True). + :type ignore_comments: bool + :returns: PSyIR representing the provided Fortran file. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -217,7 +226,8 @@ def psyir_from_file(self, file_path, free_form=True): # Using the FortranFileReader instead of manually open the file allows # fparser to keep the filename information in the tree reader = FortranFileReader(file_path, - include_dirs=Config.get().include_paths) + include_dirs=Config.get().include_paths, + ignore_comments=ignore_comments) reader.set_format(FortranFormat(free_form, False)) parse_tree = self._parser(reader) _, filename = os.path.split(file_path) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index c6855064cf..469953bc95 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -60,6 +60,7 @@ Reference, Return, Routine, Schedule, StructureReference, UnaryOperation, WhileLoop) from psyclone.psyir.nodes.array_mixin import ArrayMixin +from psyclone.psyir.nodes.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import ( ArgumentInterface, ArrayType, AutomaticInterface, CHARACTER_TYPE, CommonBlockInterface, ContainerSymbol, DataSymbol, DataTypeSymbol, @@ -712,14 +713,16 @@ def _process_routine_symbols(module_ast, container, visibility_map): # By default, Fortran routines are not elemental. is_elemental = False # Name of the routine. - name = str(routine.children[0].children[1]) + stmt = walk(routine, (Fortran2003.Subroutine_Stmt, + Fortran2003.Function_Stmt))[0] + name = str(stmt.children[1]) # Type to give the RoutineSymbol. sym_type = type_map[type(routine)]() # Visibility of the symbol. vis = visibility_map.get(name.lower(), container.symbol_table.default_visibility) # Check any prefixes on the routine declaration. - prefix = routine.children[0].children[0] + prefix = stmt.children[0] if prefix: for child in prefix.children: if isinstance(child, Fortran2003.Prefix_Spec): @@ -1623,7 +1626,7 @@ def _process_type_spec(self, parent, type_spec): return base_type, precision def _process_decln(self, scope, symbol_table, decl, visibility_map=None, - statics_list=()): + statics_list=(), preceding_comments_strs=None): ''' Process the supplied fparser2 parse tree for a declaration. For each entity that is declared, a symbol is added to the supplied symbol @@ -1643,6 +1646,9 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, appearing in a SAVE statement). If all symbols are static then this contains the single entry "*". :type statics_list: Iterable[str] + :param preceding_comments_strs: the comments that preceded this + declaration, as a list of strings. + :type preceding_comments_strs: Optional[List[str]] :raises NotImplementedError: if an unsupported attribute is found. :raises NotImplementedError: if an unsupported intent attribute is @@ -1664,6 +1670,9 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, ''' # pylint: disable=too-many-arguments + if preceding_comments_strs is None: + preceding_comments_strs = [] + (type_spec, attr_specs, entities) = decl.items # Parse the type_spec @@ -1897,6 +1906,10 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, f"'{sym_name}'.") from error symbol_table.add(sym, tag=tag) + # Add any preceding comments to the symbol + sym.preceding_comment = '\n'.join(preceding_comments_strs) + preceding_comments_strs = [] + if init_expr: # In Fortran, an initialisation expression on a declaration of # a symbol (whether in a routine or a module) implies that the @@ -2002,8 +2015,22 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): local_table = Container("tmp", parent=parent).symbol_table local_table.default_visibility = default_compt_visibility - for child in walk(decl, Fortran2003.Data_Component_Def_Stmt): - self._process_decln(parent, local_table, child) + preceding_comments_strs = [] + for child in decl.children: + if isinstance(child, Fortran2003.Derived_Type_Stmt): + continue + if isinstance(child, Fortran2003.Comment): + # fparser2 generates lots of empty Comment nodes (without even a '!'), drop them + if len(child.tostr()) == 0: + continue + comment_str = child.tostr()[1:].strip() + preceding_comments_strs.append(comment_str) + continue + if isinstance(child, Fortran2003.Component_Part): + for component in walk(child, Fortran2003.Data_Component_Def_Stmt): + self._process_decln(parent, local_table, component, + preceding_comments_strs=preceding_comments_strs) + preceding_comments_strs = [] # Convert from Symbols to StructureType components. for symbol in local_table.symbols: if symbol.is_unresolved: @@ -2016,7 +2043,7 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): datatype = symbol.datatype initial_value = symbol.initial_value dtype.add(symbol.name, datatype, symbol.visibility, - initial_value) + initial_value, symbol.preceding_comment) # Update its type with the definition we've found tsymbol.datatype = dtype @@ -2324,19 +2351,41 @@ def process_declarations(self, parent, nodes, arg_list, # The include_handler just raises an error so we use that to # reduce code duplication. self._include_handler(incl_nodes[0], parent) + # Now we've captured any derived-type definitions, proceed to look # at the variable declarations. + preceding_comments_strs = [] for node in nodes: - if isinstance(node, Fortran2003.Interface_Block): - + if isinstance(node, Fortran2003.Implicit_Part): + for comment in walk(node, Fortran2003.Comment): + comment_str = comment.tostr()[1:].strip() + preceding_comments_strs.append(comment_str) + # Anything other than a PARAMETER statement or an + # IMPLICIT NONE means we can't handle this code. + # Any PARAMETER statements are handled separately by the + # call to _process_parameter_stmts below. + # Any ENTRY statements are checked for in _subroutine_handler. + child_nodes = walk(node, Fortran2003.Format_Stmt) + if child_nodes: + raise NotImplementedError( + f"Error processing implicit-part: Format statements " + f"are not supported but found '{child_nodes[0]}'") + child_nodes = walk(node, Fortran2003.Implicit_Stmt) + if any(imp.children != ('NONE',) for imp in child_nodes): + raise NotImplementedError( + f"Error processing implicit-part: implicit variable " + f"declarations not supported but found '{node}'") + elif isinstance(node, Fortran2003.Interface_Block): self._process_interface_block(node, parent.symbol_table, visibility_map) elif isinstance(node, Fortran2003.Type_Declaration_Stmt): try: self._process_decln(parent, parent.symbol_table, node, - visibility_map, statics_list) + visibility_map, statics_list, + preceding_comments_strs) + preceding_comments_strs = [] except NotImplementedError: # Found an unsupported variable declaration. Create a # DataSymbol with UnsupportedType for each entity being @@ -2377,14 +2426,15 @@ def process_declarations(self, parent, nodes, arg_list, # possible that some may have already been processed # successfully and thus be in the symbol table. try: - parent.symbol_table.add( - DataSymbol( - symbol_name, UnsupportedFortranType( - str(node), - partial_datatype=datatype), - interface=UnknownInterface(), - visibility=vis, - initial_value=init)) + new_symbol = DataSymbol( + symbol_name, UnsupportedFortranType( + str(node), + partial_datatype=datatype), + interface=UnknownInterface(), + visibility=vis, + initial_value=init) + new_symbol.preceding_comment = '\n'.join(preceding_comments_strs) + parent.symbol_table.add(new_symbol) except KeyError as err: if len(orig_children) == 1: raise SymbolError( @@ -2405,23 +2455,6 @@ def process_declarations(self, parent, nodes, arg_list, # These node types are handled separately pass - elif isinstance(node, Fortran2003.Implicit_Part): - # Anything other than a PARAMETER statement or an - # IMPLICIT NONE means we can't handle this code. - # Any PARAMETER statements are handled separately by the - # call to _process_parameter_stmts below. - # Any ENTRY statements are checked for in _subroutine_handler. - child_nodes = walk(node, Fortran2003.Format_Stmt) - if child_nodes: - raise NotImplementedError( - f"Error processing implicit-part: Format statements " - f"are not supported but found '{child_nodes[0]}'") - child_nodes = walk(node, Fortran2003.Implicit_Stmt) - if any(imp.children != ('NONE',) for imp in child_nodes): - raise NotImplementedError( - f"Error processing implicit-part: implicit variable " - f"declarations not supported but found '{node}'") - elif isinstance(node, Fortran2003.Namelist_Stmt): # Place the declaration statement into the symbol table using # an internal symbol name. In case that we need more details @@ -2729,7 +2762,17 @@ def process_nodes(self, parent, nodes): ''' code_block_nodes = [] message = "PSyclone CodeBlock (unsupported code) reason:" + # Store any comments that precede the next node + preceding_comments_strs = [] for child in nodes: + # If the child is a comment, store it and continue to the next + if isinstance(child, Fortran2003.Comment): + if len(child.tostr()) == 0: + # Skip empty comments + continue + comment_str = child.tostr()[1:].strip() + preceding_comments_strs.append(comment_str) + continue try: psy_child = self._create_child(child, parent) except NotImplementedError as err: @@ -2748,6 +2791,11 @@ def process_nodes(self, parent, nodes): if psy_child: self.nodes_to_code_block(parent, code_block_nodes, message) message = "PSyclone CodeBlock (unsupported code) reason:" + # Add the comments to nodes that support it and reset the + # list of comments + if isinstance(psy_child, CommentableMixin): + psy_child.preceding_comment += '\n'.join(preceding_comments_strs) + preceding_comments_strs = [] 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. @@ -2778,6 +2826,7 @@ def _create_child(self, child, parent=None): # there), so we have to examine the first statement within it. We # must allow for the case where the block is empty though. if (child.content and child.content[0] and + (not isinstance(child.content[0], Fortran2003.Comment)) and child.content[0].item and child.content[0].item.label): raise NotImplementedError("Unsupported labelled statement") elif isinstance(child, StmtBase): @@ -3128,8 +3177,31 @@ def _do_construct_handler(self, node, parent): else: raise NotImplementedError("Unsupported Loop") - # Process loop body (ignore 'do' and 'end do' statements with [1:-1]) - self.process_nodes(parent=loop_body, nodes=node.content[1:-1]) + # Process loop body (ignore 'do' and 'end do' statements) + # Keep track of the comments before the 'do' statement + loop_body_nodes = [] + preceding_comments_strs = [] + found_do_stmt = False + for child in node.content: + if isinstance(child, Fortran2003.Comment) and not found_do_stmt: + if child.tostr() == "": + # Skip empty comments + continue + comment_str = child.tostr()[1:].strip() + preceding_comments_strs.append(comment_str) + continue + if isinstance(child, Fortran2003.Nonlabel_Do_Stmt): + found_do_stmt = True + continue + if isinstance(child, Fortran2003.End_Do_Stmt): + continue + + loop_body_nodes.append(child) + + # Add the comments to the loop node. + loop.preceding_comment = '\n'.join(preceding_comments_strs) + # Process the loop body. + self.process_nodes(parent=loop_body, nodes=loop_body_nodes) return loop @@ -3148,10 +3220,10 @@ def _if_construct_handler(self, node, parent): ''' # Check that the fparser2 parsetree has the expected structure - if not isinstance(node.content[0], Fortran2003.If_Then_Stmt): + if len(walk(node, Fortran2003.If_Then_Stmt)) == 0: raise InternalError( f"Failed to find opening if then statement in: {node}") - if not isinstance(node.content[-1], Fortran2003.End_If_Stmt): + if len(walk(node, Fortran2003.End_If_Stmt)) == 0: raise InternalError( f"Failed to find closing end if statement in: {node}") @@ -3164,6 +3236,18 @@ def _if_construct_handler(self, node, parent): Fortran2003.End_If_Stmt)): clause_indices.append(idx) + # Get the comments before the 'if' statement + preceding_comments_strs = [] + for child in node.content[:clause_indices[0]]: + if isinstance(child, Fortran2003.Comment): + if len(child.tostr()) == 0: + # Skip empty comments + continue + comment_str = child.tostr()[1:].strip() + preceding_comments_strs.append(comment_str) + # NOTE: The comments are added to the IfBlock node. + # NOTE: Comments before the 'else[if]' statements are not handled. + # Deal with each clause: "if", "else if" or "else". ifblock = None currentparent = parent @@ -3194,6 +3278,9 @@ def _if_construct_handler(self, node, parent): elsebody.ast = node.content[start_idx] newifblock.ast = node.content[start_idx] + # Add the comments to the if block. + newifblock.preceding_comment = '\n'.join(preceding_comments_strs) + # Create condition as first child self.process_nodes(parent=newifblock, nodes=[clause.items[0]]) @@ -3735,10 +3822,10 @@ def _case_construct_handler(self, node, parent): ''' # Check that the fparser2 parsetree has the expected structure - if not isinstance(node.content[0], Fortran2003.Select_Case_Stmt): + if len(walk(node, Fortran2003.Select_Case_Stmt)) == 0: raise InternalError( f"Failed to find opening case statement in: {node}") - if not isinstance(node.content[-1], Fortran2003.End_Select_Stmt): + if len(walk(node, Fortran2003.End_Select_Stmt)) == 0: raise InternalError( f"Failed to find closing case statement in: {node}") @@ -5125,7 +5212,20 @@ def _subroutine_handler(self, node, parent): if isinstance(parent, FileContainer): _process_routine_symbols(node, parent, {}) - name = node.children[0].children[1].string + # Get the subroutine or function statement and store the comments + # that precede it. + preceding_comments_strs = [] + for child in node.children: + if isinstance(child, Fortran2003.Comment): + if len(child.tostr()) == 0: + continue + preceding_comments_strs.append(child.tostr()[1:].strip()) + continue + if isinstance(child, (Fortran2003.Subroutine_Stmt, + Fortran2003.Function_Stmt)): + stmt = child + break + name = stmt.children[1].string routine = None # The Routine may have been forward declared in # _process_routine_symbol, and so may already exist in the @@ -5144,6 +5244,8 @@ def _subroutine_handler(self, node, parent): # empty Routine is detached from the tree. parent.addchild(routine) + routine.preceding_comment = '\n'.join(preceding_comments_strs) + try: routine._ast = node @@ -5162,19 +5264,29 @@ def _subroutine_handler(self, node, parent): # Dummy_Arg_List, even if there's only one of them. if (isinstance(node, (Fortran2003.Subroutine_Subprogram, Fortran2003.Function_Subprogram)) and - isinstance(node.children[0].children[2], + isinstance(stmt.children[2], Fortran2003.Dummy_Arg_List)): - arg_list = node.children[0].children[2].children + arg_list = stmt.children[2].children else: # Routine has no arguments arg_list = [] self.process_declarations(routine, decl_list, arg_list) + # TODO: fparser issue + # fparser puts comments at the end of the declarations + # whereas as preceding comments they belong in the execution part + 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 + lost_comments.append(comment) + # Check whether the function-stmt has a prefix specifying the # return type (other prefixes are handled in # _process_routine_symbols()). base_type = None - prefix = node.children[0].children[0] + prefix = stmt.children[0] if prefix: for child in prefix.children: if isinstance(child, Fortran2003.Prefix_Spec): @@ -5188,7 +5300,7 @@ def _subroutine_handler(self, node, parent): if isinstance(node, Fortran2003.Function_Subprogram): # Check whether this function-stmt has a suffix containing # 'RETURNS' - suffix = node.children[0].children[3] + suffix = stmt.children[3] if suffix: # Although the suffix can, in principle, contain a proc- # language-binding-spec (e.g. BIND(C, "some_name")), this @@ -5258,7 +5370,10 @@ def _subroutine_handler(self, node, parent): # valid. pass else: - self.process_nodes(routine, sub_exec.content) + # TODO: fparser issue + # Put the comments from the end of the declarations part + # at the start of the execution part manually + self.process_nodes(routine, lost_comments + sub_exec.content) except NotImplementedError as err: sym = routine.symbol routine.detach() diff --git a/src/psyclone/psyir/symbols/datasymbol.py b/src/psyclone/psyir/symbols/datasymbol.py index c1ddb9e20d..b9089d2786 100644 --- a/src/psyclone/psyir/symbols/datasymbol.py +++ b/src/psyclone/psyir/symbols/datasymbol.py @@ -319,11 +319,13 @@ def copy(self): new_datatype = self.datatype.copy() else: new_datatype = self.datatype - return DataSymbol(self.name, new_datatype, + copy = DataSymbol(self.name, new_datatype, visibility=self.visibility, interface=self.interface.copy(), is_constant=self.is_constant, initial_value=new_init_value) + copy.preceding_comment = self.preceding_comment + return copy def copy_properties(self, symbol_in): '''Replace all properties in this object with the properties from diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index ed9a635413..10c6247980 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -918,12 +918,15 @@ class ComponentType: :param visibility: whether this member is public or private. :param initial_value: the initial value of this member (if any). :type initial_value: Optional[:py:class:`psyclone.psyir.nodes.Node`] + :param preceding_comment: a comment that precedes this component. + :type preceding_comment: Optional[str] ''' name: str # Use Union for compatibility with Python < 3.10 datatype: Union[DataType, DataTypeSymbol] visibility: Symbol.Visibility initial_value: Any + preceding_comment: str = "" def __init__(self): self._components = OrderedDict() @@ -952,10 +955,11 @@ def create(components): ''' stype = StructureType() for component in components: - if len(component) != 4: + if len(component) not in (4, 5): raise TypeError( - f"Each component must be specified using a 4-tuple of " - f"(name, type, visibility, initial_value) but found a " + f"Each component must be specified using a 4 or 5-tuple " + f"of (name, type, visibility, initial_value, " + f"preceding_comment) but found a " f"tuple with {len(component)} members: {component}") stype.add(*component) return stype @@ -968,7 +972,8 @@ def components(self): ''' return self._components - def add(self, name, datatype, visibility, initial_value): + def add(self, name, datatype, visibility, initial_value, + preceding_comment = ""): ''' Create a component with the supplied attributes and add it to this StructureType. @@ -982,6 +987,8 @@ def add(self, name, datatype, visibility, initial_value): :param initial_value: the initial value of the new component. :type initial_value: Optional[ :py:class:`psyclone.psyir.nodes.DataNode`] + :param preceding_comment: a comment that precedes this component. + :type preceding_comment: Optional[str] :raises TypeError: if any of the supplied values are of the wrong type. @@ -1016,9 +1023,14 @@ def add(self, name, datatype, visibility, initial_value): f"The initial value of a component of a StructureType must " f"be None or an instance of 'DataNode', but got " f"'{type(initial_value).__name__}'.") + if not isinstance(preceding_comment, str): + raise TypeError( + f"The preceding_comment of a component of a StructureType " + f"must be a 'str' but got " + f"'{type(preceding_comment).__name__}'") self._components[name] = self.ComponentType( - name, datatype, visibility, initial_value) + name, datatype, visibility, initial_value, preceding_comment) def lookup(self, name): ''' diff --git a/src/psyclone/psyir/symbols/symbol.py b/src/psyclone/psyir/symbols/symbol.py index a69529919d..1cb134f94b 100644 --- a/src/psyclone/psyir/symbols/symbol.py +++ b/src/psyclone/psyir/symbols/symbol.py @@ -45,6 +45,8 @@ AutomaticInterface, SymbolInterface, ArgumentInterface, UnresolvedInterface, ImportInterface, UnknownInterface, CommonBlockInterface, DefaultModuleInterface, StaticInterface) +from psyclone.psyir.nodes.commentable_mixin import CommentableMixin + class SymbolError(PSycloneError): @@ -59,7 +61,7 @@ def __init__(self, value): self.value = "PSyclone SymbolTable error: "+str(value) -class Symbol(): +class Symbol(CommentableMixin): '''Generic Symbol item for the Symbol Table and PSyIR References. It has an immutable name label because it must always match with the key in the SymbolTable. If the symbol is private then it is only visible diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 32b9ebe76b..5a7660ec03 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -954,9 +954,9 @@ def test_create_structuretype(): StructureType.create([ ("fred", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None), ("george", Symbol.Visibility.PRIVATE)]) - assert ("Each component must be specified using a 4-tuple of (name, " - "type, visibility, initial_value) but found a tuple with 2 " - "members: ('george', " in str(err.value)) + assert ("Each component must be specified using a 4 or 5-tuple of (name, " + "type, visibility, initial_value, preceding_comment) but found a " + "tuple with 2 members: ('george', " in str(err.value)) def test_structuretype_eq(): From d5b0a7e468775e8019b15bda854d8de6cc0f3056 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 2 Dec 2024 20:51:53 +0100 Subject: [PATCH 02/20] #1247 Fix comments on nested ifs. Some tests. --- src/psyclone/psyir/frontend/fparser2.py | 2 + .../psyir/frontend/fparser2_comment_test.py | 248 ++++++++++++++++++ 2 files changed, 250 insertions(+) create mode 100644 src/psyclone/tests/psyir/frontend/fparser2_comment_test.py diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 469953bc95..7902a8b8f0 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2339,6 +2339,7 @@ def process_declarations(self, parent, nodes, arg_list, # Handle any derived-type declarations/definitions before we look # at general variable declarations in case any of the latter use # the former. + # TODO: add support for comments on derived types. for decl in walk(nodes, Fortran2003.Derived_Type_Def): self._process_derived_type_decln(parent, decl, visibility_map) @@ -3280,6 +3281,7 @@ def _if_construct_handler(self, node, parent): # Add the comments to the if block. newifblock.preceding_comment = '\n'.join(preceding_comments_strs) + preceding_comments_strs = [] # Create condition as first child self.process_nodes(parent=newifblock, diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py new file mode 100644 index 0000000000..3e88718370 --- /dev/null +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -0,0 +1,248 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2021-2024, 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 Julien Remy, Université Grenoble Alpes & Inria + +'''Performs pytest tests on the support for comments in the fparser2 +PSyIR front-end''' + +import pytest + +from psyclone.psyir.frontend.fortran import FortranReader +from psyclone.psyir.nodes import Container, Routine, Assignment, Loop, IfBlock, Call +from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.symbols import DataTypeSymbol, StructureType + +from psyclone.psyir.backend.fortran import FortranWriter + +# Test code +CODE = ''' +! Comment on module 'test_mod' +! and second line +module test_mod + implicit none + ! Comment on derived type 'my_type' SHOULD BE LOST + type :: my_type + ! Comment on component 'i' + ! and second line + integer :: i + ! Comment on component 'j' + integer :: j + end type my_type +contains + ! Comment on a subroutine + subroutine test_sub() + ! Comment on variable 'a' + ! and second line + integer :: a + ! Comment on variable 'i' + integer :: i + ! Comment on variable 'j' + integer :: j + ! Comment on assignment 'a = 1' + ! and second line + a = 1 + ! Comment on call 'call test_sub()' + call test_sub() + ! Comment on if block 'if (a == 1) then' + if (a == 1) then + ! Comment on assignment 'a = 2' + a = 2 + ! Comment on elseif block 'elseif (a == 2) then' SHOULD BE LOST + elseif (a == 2) then + ! Comment on assignment 'a = 3' + a = 3 + ! Comment on else block 'else' SHOULD BE LOST + else + ! Comment on assignment 'a = 4' + a = 4 + ! Comment on 'end if' SHOULD BE LOST + end if + ! Comment on loop 'do i = 1, 10' + do i = 1, 10 + ! Comment on assignment 'a = 5' + a = 5 + ! Comment on loop 'do j = 1, 10' + do j = 1, 10 + ! Comment on assignment 'a = 6' + a = 6 + end do + end do + end subroutine test_sub +end module test_mod +''' + +def test_no_comments(): + '''Test that the FortranReader is without comments by default''' + reader = FortranReader() + psyir = reader.psyir_from_source(CODE) + + module = psyir.children[0] + assert isinstance(module, Container) + assert module.name == "test_mod" + assert module.preceding_comment == "" + + my_type_sym = module.symbol_table.lookup("my_type") + assert isinstance(my_type_sym, DataTypeSymbol) + assert my_type_sym.preceding_comment == "" + + assert isinstance(my_type_sym.datatype, StructureType) + for component in my_type_sym.datatype.components.values(): + assert component.preceding_comment == "" + + routine = module.walk(Routine)[0] + assert routine.name == "test_sub" + assert routine.preceding_comment == "" + for symbol in routine.symbol_table.symbols: + assert symbol.preceding_comment == "" + commentable_nodes = routine.walk(CommentableMixin) + assert len(commentable_nodes) != 0 + for node in commentable_nodes: + assert node.preceding_comment == "" + + +def test_comments(): + '''Test that the FortranReader is able to read comments''' + reader = FortranReader() + psyir = reader.psyir_from_source(CODE, ignore_comments=False) + + module = psyir.children[0] + assert module.preceding_comment == "Comment on module 'test_mod'\nand second line" + + # TODO: add support for comments on derived types. + my_type_sym = module.symbol_table.lookup("my_type") + assert my_type_sym.preceding_comment == "" + + assert isinstance(my_type_sym.datatype, StructureType) + for i, component in enumerate(my_type_sym.datatype.components.values()): + if i == 0: + assert component.preceding_comment == "Comment on component 'i'\nand second line" + else: + assert component.preceding_comment == "Comment on component 'j'" + + routine = module.walk(Routine)[0] + assert routine.preceding_comment == "Comment on a subroutine" + + for i, symbol in enumerate(routine.symbol_table.symbols): + if i == 0: + assert symbol.preceding_comment == "Comment on variable 'a'\nand second line" + else: + assert symbol.preceding_comment == f"Comment on variable '{symbol.name}'" + + for i, assignment in enumerate(routine.walk(Assignment)): + if i == 0: + assert assignment.preceding_comment == "Comment on assignment 'a = 1'\nand second line" + else: + assert assignment.preceding_comment == f"Comment on assignment 'a = {i+1}'" + + call = routine.walk(Call)[0] + assert call.preceding_comment == "Comment on call 'call test_sub()'" + + ifblock = routine.walk(IfBlock)[0] + assert ifblock.preceding_comment == "Comment on if block 'if (a == 1) then'" + + loops = routine.walk(Loop) + loop_i = loops[0] + # OMP directives should be ignored + assert loop_i.preceding_comment == "Comment on loop 'do i = 1, 10'" + + loop_j = loops[1] + assert loop_j.preceding_comment == "Comment on loop 'do j = 1, 10'" + +EXPECTED_WITH_COMMENTS = '''! Comment on module 'test_mod' +! and second line +module test_mod + implicit none + type, public :: my_type + ! Comment on component 'i' + ! and second line + integer, public :: i + ! Comment on component 'j' + integer, public :: j + end type my_type + public + + contains + ! Comment on a subroutine + subroutine test_sub() + ! Comment on variable 'a' + ! and second line + integer :: a + ! Comment on variable 'i' + integer :: i + ! Comment on variable 'j' + integer :: j + + ! Comment on assignment 'a = 1' + ! and second line + a = 1 + ! Comment on call 'call test_sub()' + call test_sub() + ! Comment on if block 'if (a == 1) then' + if (a == 1) then + ! Comment on assignment 'a = 2' + a = 2 + else + if (a == 2) then + ! Comment on assignment 'a = 3' + a = 3 + else + ! Comment on assignment 'a = 4' + a = 4 + end if + end if + ! Comment on loop 'do i = 1, 10' + do i = 1, 10, 1 + ! Comment on assignment 'a = 5' + a = 5 + ! Comment on loop 'do j = 1, 10' + do j = 1, 10, 1 + ! Comment on assignment 'a = 6' + a = 6 + enddo + enddo + + end subroutine test_sub + +end module test_mod +''' + +def test_write_comments(): + '''Test that the comments are written back to the code''' + reader = FortranReader() + writer = FortranWriter() + psyir = reader.psyir_from_source(CODE, ignore_comments=False) + generated_code = writer(psyir) + assert generated_code == EXPECTED_WITH_COMMENTS + + From 48fe55cdf259c2005ddf4977e8d089dfe2b2ae43 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 2 Dec 2024 21:23:25 +0100 Subject: [PATCH 03/20] #1247 Some tests for comments --- src/psyclone/psyir/frontend/fortran.py | 36 ++++++++- src/psyclone/psyir/frontend/fparser2.py | 28 ++++--- src/psyclone/psyir/symbols/datatypes.py | 2 +- src/psyclone/psyir/symbols/symbol.py | 1 - .../psyir/frontend/fparser2_comment_test.py | 77 ++++++++++++++++--- 5 files changed, 117 insertions(+), 27 deletions(-) diff --git a/src/psyclone/psyir/frontend/fortran.py b/src/psyclone/psyir/frontend/fortran.py index 6140f2ae6b..df0af1c2ea 100644 --- a/src/psyclone/psyir/frontend/fortran.py +++ b/src/psyclone/psyir/frontend/fortran.py @@ -44,7 +44,7 @@ from fparser.two import Fortran2003, pattern_tools from fparser.two.parser import ParserFactory from fparser.two.symbol_table import SYMBOL_TABLES -from fparser.two.utils import NoMatchError +from fparser.two.utils import NoMatchError, walk from psyclone.configuration import Config from psyclone.psyir.frontend.fparser2 import Fparser2Reader from psyclone.psyir.nodes import Schedule, Assignment, Routine @@ -86,13 +86,17 @@ def validate_name(name: str): f"Invalid Fortran name '{name}' found.") def psyir_from_source(self, source_code: str, free_form: bool = True, - ignore_comments: bool = True): + ignore_comments: bool = True, + ignore_directives: bool = True): ''' Generate the PSyIR tree representing the given Fortran source code. :param source_code: text representation of the code to be parsed. :param free_form: If parsing free-form code or not (default True). - :param ignore_comments: If comments should be ignored or not + :param ignore_comments: If comments should be ignored or not (default True). + :param ignore_directives: If directives should be ignored or not + (default True). Only has an effect + if ignore_comments is False. :returns: PSyIR representing the provided Fortran source code. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -105,6 +109,10 @@ def psyir_from_source(self, source_code: str, free_form: bool = True, # Set reader to free format. string_reader.set_format(FortranFormat(free_form, False)) parse_tree = self._parser(string_reader) + + if (not ignore_comments) and ignore_directives: + self._strip_directives(parse_tree) + psyir = self._processor.generate_psyir(parse_tree) return psyir @@ -199,7 +207,8 @@ def psyir_from_statement(self, source_code: str, return fake_parent[0].detach() def psyir_from_file(self, file_path, free_form=True, - ignore_comments: bool = True): + ignore_comments: bool = True, + ignore_directives: bool = True): ''' Generate the PSyIR tree representing the given Fortran file. :param file_path: path of the file to be read and parsed. @@ -212,6 +221,11 @@ def psyir_from_file(self, file_path, free_form=True, (default True). :type ignore_comments: bool + :param ignore_directives: If directives should be ignored or not + (default True). Only has an effect + if ignore_comments is False. + :type ignore_directives: bool + :returns: PSyIR representing the provided Fortran file. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -231,9 +245,23 @@ def psyir_from_file(self, file_path, free_form=True, reader.set_format(FortranFormat(free_form, False)) parse_tree = self._parser(reader) _, filename = os.path.split(file_path) + + if (not ignore_comments) and ignore_directives: + self._strip_directives(parse_tree) + psyir = self._processor.generate_psyir(parse_tree, filename) return psyir + def _strip_directives(self, node): + '''Remove all '!$' directives from the comments in the fparser tree. + + :param node: the fparser node to process. + ''' + for comment in walk(node, Fortran2003.Comment): + if comment.tostr().startswith("!$"): + comment.items[0] = "" + return node + # For Sphinx AutoAPI documentation generation __all__ = ['FortranReader'] diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 7902a8b8f0..ff5243a055 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -714,7 +714,7 @@ def _process_routine_symbols(module_ast, container, visibility_map): is_elemental = False # Name of the routine. stmt = walk(routine, (Fortran2003.Subroutine_Stmt, - Fortran2003.Function_Stmt))[0] + Fortran2003.Function_Stmt))[0] name = str(stmt.children[1]) # Type to give the RoutineSymbol. sym_type = type_map[type(routine)]() @@ -2020,15 +2020,17 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): if isinstance(child, Fortran2003.Derived_Type_Stmt): continue if isinstance(child, Fortran2003.Comment): - # fparser2 generates lots of empty Comment nodes (without even a '!'), drop them + # fparser2 generates lots of empty Comment nodes + # (without even a '!'), drop them if len(child.tostr()) == 0: continue comment_str = child.tostr()[1:].strip() preceding_comments_strs.append(comment_str) continue if isinstance(child, Fortran2003.Component_Part): - for component in walk(child, Fortran2003.Data_Component_Def_Stmt): - self._process_decln(parent, local_table, component, + for component in walk(child, + Fortran2003.Data_Component_Def_Stmt): + self._process_decln(parent, local_table, component, preceding_comments_strs=preceding_comments_strs) preceding_comments_strs = [] # Convert from Symbols to StructureType components. @@ -2433,8 +2435,9 @@ def process_declarations(self, parent, nodes, arg_list, partial_datatype=datatype), interface=UnknownInterface(), visibility=vis, - initial_value=init) - new_symbol.preceding_comment = '\n'.join(preceding_comments_strs) + initial_value=init) + new_symbol.preceding_comment \ + = '\n'.join(preceding_comments_strs) parent.symbol_table.add(new_symbol) except KeyError as err: if len(orig_children) == 1: @@ -2766,7 +2769,7 @@ def process_nodes(self, parent, nodes): # Store any comments that precede the next node preceding_comments_strs = [] for child in nodes: - # If the child is a comment, store it and continue to the next + # If the child is a comment, store it and continue to the next if isinstance(child, Fortran2003.Comment): if len(child.tostr()) == 0: # Skip empty comments @@ -2795,7 +2798,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 += '\n'.join(preceding_comments_strs) + psy_child.preceding_comment\ + += '\n'.join(preceding_comments_strs) preceding_comments_strs = [] parent.addchild(psy_child) # If psy_child is not initialised but it didn't produce a @@ -3280,7 +3284,8 @@ def _if_construct_handler(self, node, parent): newifblock.ast = node.content[start_idx] # Add the comments to the if block. - newifblock.preceding_comment = '\n'.join(preceding_comments_strs) + newifblock.preceding_comment \ + = '\n'.join(preceding_comments_strs) preceding_comments_strs = [] # Create condition as first child @@ -5275,10 +5280,11 @@ def _subroutine_handler(self, node, parent): self.process_declarations(routine, decl_list, arg_list) # TODO: fparser issue - # fparser puts comments at the end of the declarations + # fparser puts comments at the end of the declarations # whereas as preceding comments they belong in the execution part lost_comments = [] - if len(decl_list) != 0 and isinstance(decl_list[-1], Fortran2003.Implicit_Part): + 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 diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index 10c6247980..6cd8b9c3dc 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -973,7 +973,7 @@ def components(self): return self._components def add(self, name, datatype, visibility, initial_value, - preceding_comment = ""): + preceding_comment=""): ''' Create a component with the supplied attributes and add it to this StructureType. diff --git a/src/psyclone/psyir/symbols/symbol.py b/src/psyclone/psyir/symbols/symbol.py index 1cb134f94b..0c35d4455a 100644 --- a/src/psyclone/psyir/symbols/symbol.py +++ b/src/psyclone/psyir/symbols/symbol.py @@ -48,7 +48,6 @@ from psyclone.psyir.nodes.commentable_mixin import CommentableMixin - class SymbolError(PSycloneError): ''' PSyclone-specific exception for use with errors relating to the Symbol and diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index 3e88718370..10a6ca0b27 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -33,20 +33,21 @@ # ----------------------------------------------------------------------------- # Author Julien Remy, Université Grenoble Alpes & Inria -'''Performs pytest tests on the support for comments in the fparser2 -PSyIR front-end''' +"""Performs pytest tests on the support for comments in the fparser2 +PSyIR front-end""" import pytest from psyclone.psyir.frontend.fortran import FortranReader -from psyclone.psyir.nodes import Container, Routine, Assignment, Loop, IfBlock, Call +from psyclone.psyir.nodes import (Container, Routine, Assignment, + Loop, IfBlock, Call) from psyclone.psyir.nodes.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import DataTypeSymbol, StructureType from psyclone.psyir.backend.fortran import FortranWriter # Test code -CODE = ''' +CODE = """ ! Comment on module 'test_mod' ! and second line module test_mod @@ -100,10 +101,11 @@ end do end subroutine test_sub end module test_mod -''' +""" + def test_no_comments(): - '''Test that the FortranReader is without comments by default''' + """Test that the FortranReader is without comments by default""" reader = FortranReader() psyir = reader.psyir_from_source(CODE) @@ -132,7 +134,7 @@ def test_no_comments(): def test_comments(): - '''Test that the FortranReader is able to read comments''' + """Test that the FortranReader is able to read comments""" reader = FortranReader() psyir = reader.psyir_from_source(CODE, ignore_comments=False) @@ -179,7 +181,8 @@ def test_comments(): loop_j = loops[1] assert loop_j.preceding_comment == "Comment on loop 'do j = 1, 10'" -EXPECTED_WITH_COMMENTS = '''! Comment on module 'test_mod' + +EXPECTED_WITH_COMMENTS = """! Comment on module 'test_mod' ! and second line module test_mod implicit none @@ -235,10 +238,11 @@ def test_comments(): end subroutine test_sub end module test_mod -''' +""" + def test_write_comments(): - '''Test that the comments are written back to the code''' + """Test that the comments are written back to the code""" reader = FortranReader() writer = FortranWriter() psyir = reader.psyir_from_source(CODE, ignore_comments=False) @@ -246,3 +250,56 @@ def test_write_comments(): assert generated_code == EXPECTED_WITH_COMMENTS +CODE_WITH_DIRECTIVE = """ +subroutine test_sub() + integer :: a + integer :: i + ! Comment on loop 'do i = 1, 10' + !$omp parallel do + do i = 1, 10 + a = 1 + end do +end subroutine test_sub +""" + + +def test_no_directives(): + """Test that the FortranReader is without directives by default""" + reader = FortranReader() + psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE, ignore_comments=False) + + loop = psyir.walk(Loop)[0] + assert loop.preceding_comment == "Comment on loop 'do i = 1, 10'" + + +def test_directives(): + """Test that the FortranReader is able to read directives""" + reader = FortranReader() + psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False) + + loop = psyir.walk(Loop)[0] + assert loop.preceding_comment == "Comment on loop 'do i = 1, 10'\n$omp parallel do" + + +EXPECTED_WITH_DIRECTIVES = """subroutine test_sub() + integer :: a + integer :: i + + ! Comment on loop 'do i = 1, 10' + !$omp parallel do + do i = 1, 10, 1 + a = 1 + enddo + +end subroutine test_sub +""" + +@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() + writer = FortranWriter() + psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False) + generated_code = writer(psyir) + assert generated_code == EXPECTED_WITH_DIRECTIVES From bcb387fe989aab8a01dc43adf5d26effaa06a6fc Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Tue, 3 Dec 2024 16:43:43 +0100 Subject: [PATCH 04/20] #1247 Inline comments are attached to wrong item. --- .../psyir/frontend/fparser2_comment_test.py | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index 10a6ca0b27..371e3f2aba 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -295,7 +295,7 @@ def test_directives(): """ @pytest.mark.xfail(reason="Directive is written back as '! $omp parallel do'" - "instead of '!$omp parallel do'") + "instead of '!$omp parallel do'") def test_write_directives(): """Test that the directives are written back to the code""" reader = FortranReader() @@ -303,3 +303,32 @@ def test_write_directives(): psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False) generated_code = writer(psyir) assert generated_code == EXPECTED_WITH_DIRECTIVES + + +CODE_WITH_INLINE_COMMENT = """ +subroutine test_sub() + integer :: a ! THIS GETS ATTACHED TO THE SYMBOL 'i' + integer :: i + a = 1 ! THIS GETS ATTACHED TO THE ASSIGNMENT 'i = 1' + i = 1 +end subroutine test_sub +""" + +def test_inline_comment(): + """Test that the FortranReader is able to read inline comments""" + reader = FortranReader() + psyir = reader.psyir_from_source(CODE_WITH_INLINE_COMMENT, ignore_comments=False) + + routine = psyir.walk(Routine)[0] + sym_a = routine.symbol_table.lookup("a") + assert sym_a.preceding_comment == "" + sym_i = routine.symbol_table.lookup("i") + assert sym_i.preceding_comment == "THIS GETS ATTACHED TO THE SYMBOL 'i'" + + assignment = routine.walk(Assignment)[0] + assert "a = 1" in assignment.debug_string() + assert assignment.preceding_comment == "" + + assignment = routine.walk(Assignment)[1] + assert "i = 1" in assignment.debug_string() + assert assignment.preceding_comment == "THIS GETS ATTACHED TO THE ASSIGNMENT 'i = 1'" From 4b90713b808e8daeea02a511085862cd83acfcff Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Wed, 4 Dec 2024 15:56:52 +0100 Subject: [PATCH 05/20] #1247 Add support for inline comments in the frontend using fparser2 `node.item.span` line info. --- src/psyclone/psyir/backend/fortran.py | 13 +- src/psyclone/psyir/frontend/fparser2.py | 150 +++++++++++++++++- .../psyir/symbols/data_type_symbol.py | 5 +- src/psyclone/psyir/symbols/datasymbol.py | 3 + src/psyclone/psyir/symbols/datatypes.py | 30 +++- src/psyclone/psyir/symbols/symbol.py | 9 +- src/psyclone/psyir/symbols/typed_symbol.py | 5 +- .../psyir/frontend/fparser2_comment_test.py | 21 ++- .../tests/psyir/symbols/datatype_test.py | 7 +- 9 files changed, 216 insertions(+), 27 deletions(-) diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index fedd1790f6..921572a131 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -559,11 +559,17 @@ def gen_vardecl(self, symbol, include_visibility=False): # blocks appearing in SAVE statements. decln = add_accessibility_to_unsupported_declaration( symbol) - result += f"{self._nindent}{decln}\n" + result += f"{self._nindent}{decln}" + if symbol.inline_comment != "": + result += f" {self._COMMENT_PREFIX}{symbol.inline_comment}" + result += "\n" return result decln = symbol.datatype.declaration - result += f"{self._nindent}{decln}\n" + result += f"{self._nindent}{decln}" + if symbol.inline_comment != "": + result += f" {self._COMMENT_PREFIX}{symbol.inline_comment}" + result += "\n" return result # The Fortran backend only handles UnsupportedFortranType # declarations. @@ -623,6 +629,9 @@ def gen_vardecl(self, symbol, include_visibility=False): f"However it has an interface of '{symbol.interface}'.") result += " = " + self._visit(symbol.initial_value) + if symbol.inline_comment != "": + result += f" {self._COMMENT_PREFIX}{symbol.inline_comment}" + return result + "\n" def gen_interfacedecl(self, symbol): diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index ff5243a055..fb00d7feba 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -979,6 +979,87 @@ def __init__(self): Fortran2003.Main_Program: self._main_program_handler, Fortran2003.Program: self._program_handler, } + # Used to attach inline comments to the PSyIR symbols and nodes + self._last_symbol_parsed_and_span = None + self._last_node_parsed_and_span = None + + @property + def last_symbol_parsed_and_span(self): + ''' + :returns: the last symbol parsed and the span of source code lines it + was found in. + :rtype: Union[None, Tuple[:py:class:`psyclone.psyir.symbols.Symbol`, + Tuple[int, int]]]''' + return self._last_symbol_parsed_and_span + + @last_symbol_parsed_and_span.setter + def last_symbol_parsed_and_span(self, value): + '''Setter for the last_symbol_parsed_and_span property. + + :param value: the last symbol parsed and the span of source code lines + it was found in. + :type value: Tuple[:py:class:`psyclone.psyir.symbols.Symbol`, + Tuple[int, int]] + + :raises TypeError: if the value is not of the right type. + ''' + if not isinstance(value, tuple) or len(value) != 2: + raise TypeError( + "The value of the last_symbol_parsed_and_span property must " + "be a 2-tuple.") + if not isinstance(value[0], Symbol): + raise TypeError( + "The first element of the last_symbol_parsed_and_span tuple " + "must be a Symbol.") + if not isinstance(value[1], tuple) or len(value[1]) != 2: + raise TypeError( + "The second element of the last_symbol_parsed_and_span tuple " + "must be a 2-tuple.") + if not all(isinstance(item, int) for item in value[1]): + raise TypeError( + "The second element of the last_symbol_parsed_and_span tuple " + "must contain two integers.") + + self._last_symbol_parsed_and_span = value + + @property + def last_node_parsed_and_span(self): + ''' + :returns: the last node parsed and the span of source code lines it + was found in. + :rtype: Union[None, Tuple[:py:class:`psyclone.psyir.nodes.Node`, + Tuple[int, int]]]''' + return self._last_node_parsed_and_span + + @last_node_parsed_and_span.setter + def last_node_parsed_and_span(self, value): + '''Setter for the last_node_parsed_and_span property. + + :param value: the last node parsed and the span of source code lines + it was found in. + :type value: Tuple[:py:class:`psyclone.psyclone.psyir.nodes.Node`, + Tuple[int, int]] + + :raises TypeError: if the value is not of the right type. + ''' + if not isinstance(value, tuple) or len(value) != 2: + raise TypeError( + "The value of the last_node_parsed_and_span property must " + "be a 2-tuple.") + if not isinstance(value[0], Node): + raise TypeError( + "The first element of the last_node_parsed_and_span tuple " + "must be a Node.") + if not isinstance(value[1], tuple) or len(value[1]) != 2: + raise TypeError( + "The second element of the last_node_parsed_and_span tuple " + "must be a 2-tuple.") + if not all(isinstance(item, int) for item in value[1]): + raise TypeError( + "The second element of the last_node_parsed_and_span tuple " + "must contain two integers.") + + self._last_node_parsed_and_span = value @staticmethod def nodes_to_code_block(parent, fp2_nodes, message=None): @@ -1910,6 +1991,8 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, sym.preceding_comment = '\n'.join(preceding_comments_strs) preceding_comments_strs = [] + self.last_symbol_parsed_and_span = (sym, decl.item.span) + if init_expr: # In Fortran, an initialisation expression on a declaration of # a symbol (whether in a routine or a module) implies that the @@ -2025,6 +2108,13 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): if len(child.tostr()) == 0: continue comment_str = child.tostr()[1:].strip() + if self.last_symbol_parsed_and_span is not None: + last_sym, last_span \ + = self.last_symbol_parsed_and_span + if (last_span[1] is not None + and last_span[1] == child.item.span[0]): + last_sym.inline_comment = comment_str + continue preceding_comments_strs.append(comment_str) continue if isinstance(child, Fortran2003.Component_Part): @@ -2045,7 +2135,8 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): datatype = symbol.datatype initial_value = symbol.initial_value dtype.add(symbol.name, datatype, symbol.visibility, - initial_value, symbol.preceding_comment) + initial_value, symbol.preceding_comment, + symbol.inline_comment) # Update its type with the definition we've found tsymbol.datatype = dtype @@ -2363,6 +2454,12 @@ def process_declarations(self, parent, nodes, arg_list, if isinstance(node, Fortran2003.Implicit_Part): for comment in walk(node, Fortran2003.Comment): comment_str = comment.tostr()[1:].strip() + if self.last_symbol_parsed_and_span is not None: + last_sym, last_span = self.last_symbol_parsed_and_span + if (last_span[1] is not None + and last_span[1] == comment.item.span[0]): + last_sym.inline_comment = comment_str + continue preceding_comments_strs.append(comment_str) # Anything other than a PARAMETER statement or an # IMPLICIT NONE means we can't handle this code. @@ -2438,6 +2535,8 @@ def process_declarations(self, parent, nodes, arg_list, initial_value=init) new_symbol.preceding_comment \ = '\n'.join(preceding_comments_strs) + self.last_symbol_parsed_and_span = (new_symbol, + node.item.span) parent.symbol_table.add(new_symbol) except KeyError as err: if len(orig_children) == 1: @@ -2769,12 +2868,20 @@ def process_nodes(self, parent, nodes): # Store any comments that precede the next node preceding_comments_strs = [] for child in nodes: - # If the child is a comment, store it and continue to the next + # If the child is a comment, attach it to the preceding node if + # it is an inline comment or store it for the next node. if isinstance(child, Fortran2003.Comment): if len(child.tostr()) == 0: # Skip empty comments continue comment_str = child.tostr()[1:].strip() + if self.last_node_parsed_and_span is not None: + last_node, last_span \ + = self.last_node_parsed_and_span + if last_span[1] is not None and \ + last_span[1] == child.item.span[0]: + last_node.inline_comment = comment_str + continue preceding_comments_strs.append(comment_str) continue try: @@ -2801,6 +2908,10 @@ def process_nodes(self, parent, nodes): psy_child.preceding_comment\ += '\n'.join(preceding_comments_strs) preceding_comments_strs = [] + if (isinstance(psy_child, CommentableMixin) + and child.item is not None): + self.last_node_parsed_and_span = (psy_child, + child.item.span) 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. @@ -3193,6 +3304,12 @@ def _do_construct_handler(self, node, parent): # Skip empty comments continue comment_str = child.tostr()[1:].strip() + if self.last_node_parsed_and_span is not None: + last_node, last_span = self.last_node_parsed_and_span + if last_span[1] is not None and \ + last_span[1] == child.item.span[0]: + last_node.inline_comment = comment_str + continue preceding_comments_strs.append(comment_str) continue if isinstance(child, Fortran2003.Nonlabel_Do_Stmt): @@ -3249,6 +3366,13 @@ def _if_construct_handler(self, node, parent): # Skip empty comments continue comment_str = child.tostr()[1:].strip() + if self.last_node_parsed_and_span is not None: + last_node, last_span = self.last_node_parsed_and_span + if last_span[1] is not None and \ + last_span[1] == child.item.span[0]: + last_node.inline_comment = comment_str + continue + preceding_comments_strs.append(comment_str) # NOTE: The comments are added to the IfBlock node. # NOTE: Comments before the 'else[if]' statements are not handled. @@ -5220,13 +5344,23 @@ def _subroutine_handler(self, node, parent): _process_routine_symbols(node, parent, {}) # Get the subroutine or function statement and store the comments - # that precede it. + # that precede it, or attach it to the last parsed node if it is + # on the same line. preceding_comments_strs = [] for child in node.children: if isinstance(child, Fortran2003.Comment): if len(child.tostr()) == 0: continue - preceding_comments_strs.append(child.tostr()[1:].strip()) + comment = child.tostr()[1:].strip() + if self.last_node_parsed_and_span is not None: + last_node, last_span \ + = self.last_node_parsed_and_span + if (isinstance(last_node, CommentableMixin) + and last_span is not None + and last_span[1] == child.item.span[0]): + last_node.inline_comment = comment + continue + preceding_comments_strs.append(comment) continue if isinstance(child, (Fortran2003.Subroutine_Stmt, Fortran2003.Function_Stmt)): @@ -5282,12 +5416,20 @@ def _subroutine_handler(self, node, parent): # TODO: fparser issue # 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_symbol_parsed_and_span is not None: + last_symbol, last_span \ + = self.last_symbol_parsed_and_span + if (last_span is not None + and last_span[1] == comment.item.span[0]): + last_symbol.inline_comment = comment.tostr()[1:].strip() + continue lost_comments.append(comment) # Check whether the function-stmt has a prefix specifying the diff --git a/src/psyclone/psyir/symbols/data_type_symbol.py b/src/psyclone/psyir/symbols/data_type_symbol.py index 92df96eb9c..3d6363c575 100644 --- a/src/psyclone/psyir/symbols/data_type_symbol.py +++ b/src/psyclone/psyir/symbols/data_type_symbol.py @@ -73,8 +73,11 @@ def copy(self): :rtype: :py:class:`psyclone.psyir.symbols.TypeSymbol` ''' - return type(self)(self.name, self.datatype, visibility=self.visibility, + copy = type(self)(self.name, self.datatype, visibility=self.visibility, interface=self.interface.copy()) + copy.preceding_comment = self.preceding_comment + copy.inline_comment = self.inline_comment + return copy def __str__(self): return f"{self.name}: {type(self).__name__}" diff --git a/src/psyclone/psyir/symbols/datasymbol.py b/src/psyclone/psyir/symbols/datasymbol.py index b9089d2786..1c84105414 100644 --- a/src/psyclone/psyir/symbols/datasymbol.py +++ b/src/psyclone/psyir/symbols/datasymbol.py @@ -325,6 +325,7 @@ def copy(self): is_constant=self.is_constant, initial_value=new_init_value) copy.preceding_comment = self.preceding_comment + copy.inline_comment = self.inline_comment return copy def copy_properties(self, symbol_in): @@ -343,6 +344,8 @@ def copy_properties(self, symbol_in): super().copy_properties(symbol_in) self._is_constant = symbol_in.is_constant self._initial_value = symbol_in.initial_value + self.preceding_comment = symbol_in.preceding_comment + self.inline_comment = symbol_in.inline_comment def replace_symbols_using(self, table): ''' diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index 6cd8b9c3dc..e0af1f57d0 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -920,6 +920,9 @@ class ComponentType: :type initial_value: Optional[:py:class:`psyclone.psyir.nodes.Node`] :param preceding_comment: a comment that precedes this component. :type preceding_comment: Optional[str] + :param inline_comment: a comment that follows this component on the + same line. + :type inline_comment: Optional[str] ''' name: str # Use Union for compatibility with Python < 3.10 @@ -927,6 +930,7 @@ class ComponentType: visibility: Symbol.Visibility initial_value: Any preceding_comment: str = "" + inline_comment: str = "" def __init__(self): self._components = OrderedDict() @@ -940,13 +944,16 @@ def create(components): Creates a StructureType from the supplied list of properties. :param components: the name, type, visibility (whether public or - private) and initial value (if any) of each component. + private), initial value (if any), preceding comment (if any) + and inline comment (if any) of each component. :type components: List[tuple[ str, :py:class:`psyclone.psyir.symbols.DataType` | :py:class:`psyclone.psyir.symbols.DataTypeSymbol`, :py:class:`psyclone.psyir.symbols.Symbol.Visibility`, - Optional[:py:class:`psyclone.psyir.symbols.DataNode`] + Optional[:py:class:`psyclone.psyir.symbols.DataNode`], + Optional[str], + Optional[str] ]] :returns: the new type object. @@ -955,11 +962,11 @@ def create(components): ''' stype = StructureType() for component in components: - if len(component) not in (4, 5): + if len(component) not in (4, 5, 6): raise TypeError( - f"Each component must be specified using a 4 or 5-tuple " + f"Each component must be specified using a 4 to 6-tuple " f"of (name, type, visibility, initial_value, " - f"preceding_comment) but found a " + f"preceding_comment, inline_comment) but found a " f"tuple with {len(component)} members: {component}") stype.add(*component) return stype @@ -973,7 +980,7 @@ def components(self): return self._components def add(self, name, datatype, visibility, initial_value, - preceding_comment=""): + preceding_comment="", inline_comment=""): ''' Create a component with the supplied attributes and add it to this StructureType. @@ -989,6 +996,9 @@ def add(self, name, datatype, visibility, initial_value, :py:class:`psyclone.psyir.nodes.DataNode`] :param preceding_comment: a comment that precedes this component. :type preceding_comment: Optional[str] + :param inline_comment: a comment that follows this component on the + same line. + :type inline_comment: Optional[str] :raises TypeError: if any of the supplied values are of the wrong type. @@ -1028,9 +1038,15 @@ def add(self, name, datatype, visibility, initial_value, f"The preceding_comment of a component of a StructureType " f"must be a 'str' but got " f"'{type(preceding_comment).__name__}'") + if not isinstance(inline_comment, str): + raise TypeError( + f"The inline_comment of a component of a StructureType must " + f"be a 'str' but got " + f"'{type(inline_comment).__name__}'") self._components[name] = self.ComponentType( - name, datatype, visibility, initial_value, preceding_comment) + name, datatype, visibility, initial_value, preceding_comment, + inline_comment) def lookup(self, name): ''' diff --git a/src/psyclone/psyir/symbols/symbol.py b/src/psyclone/psyir/symbols/symbol.py index 0c35d4455a..8f3c17d544 100644 --- a/src/psyclone/psyir/symbols/symbol.py +++ b/src/psyclone/psyir/symbols/symbol.py @@ -147,8 +147,11 @@ def copy(self): ''' # The constructors for all Symbol-based classes have 'name' as the # first positional argument. - return type(self)(self.name, visibility=self.visibility, - interface=self.interface.copy()) + copy = type(self)(self.name, visibility=self.visibility, + interface=self.interface.copy()) + copy.preceding_comment = self.preceding_comment + copy.inline_comment = self.inline_comment + return copy def copy_properties(self, symbol_in): '''Replace all properties in this object with the properties from @@ -164,6 +167,8 @@ def copy_properties(self, symbol_in): raise TypeError(f"Argument should be of type 'Symbol' but " f"found '{type(symbol_in).__name__}'.") self._interface = symbol_in.interface + self.preceding_comment = symbol_in.preceding_comment + self.inline_comment = symbol_in.inline_comment def specialise(self, subclass, **kwargs): '''Specialise this symbol so that it becomes an instance of the class diff --git a/src/psyclone/psyir/symbols/typed_symbol.py b/src/psyclone/psyir/symbols/typed_symbol.py index 89c0c9dd1d..45ed342af1 100644 --- a/src/psyclone/psyir/symbols/typed_symbol.py +++ b/src/psyclone/psyir/symbols/typed_symbol.py @@ -131,9 +131,12 @@ def copy(self): ''' # The constructors for all Symbol-based classes have 'name' as the # first positional argument. - return type(self)(self.name, self.datatype.copy(), + copy = type(self)(self.name, self.datatype.copy(), visibility=self.visibility, interface=self.interface.copy()) + copy.preceding_comment = self.preceding_comment + copy.inline_comment = self.inline_comment + return copy def copy_properties(self, symbol_in): '''Replace all properties in this object with the properties from diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index 371e3f2aba..c496f41f34 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -307,10 +307,13 @@ def test_write_directives(): CODE_WITH_INLINE_COMMENT = """ subroutine test_sub() - integer :: a ! THIS GETS ATTACHED TO THE SYMBOL 'i' - integer :: i - a = 1 ! THIS GETS ATTACHED TO THE ASSIGNMENT 'i = 1' - i = 1 + integer :: a ! Inline comment on 'integer :: a' + ! Preceding comment on 'i = 1' + integer :: i ! Inline comment on 'integer :: i' + ! Preceding comment on 'a = 1' + a = 1 ! Inline comment on 'a = 1' + ! Preceding comment on 'i = 1' + i = 1 ! Inline comment on 'i = 1' end subroutine test_sub """ @@ -322,13 +325,17 @@ def test_inline_comment(): routine = psyir.walk(Routine)[0] sym_a = routine.symbol_table.lookup("a") assert sym_a.preceding_comment == "" + assert sym_a.inline_comment == "Inline comment on 'integer :: a'" sym_i = routine.symbol_table.lookup("i") - assert sym_i.preceding_comment == "THIS GETS ATTACHED TO THE SYMBOL 'i'" + assert sym_i.preceding_comment == "Preceding comment on 'i = 1'" + assert sym_i.inline_comment == "Inline comment on 'integer :: i'" assignment = routine.walk(Assignment)[0] assert "a = 1" in assignment.debug_string() - assert assignment.preceding_comment == "" + assert assignment.preceding_comment == "Preceding comment on 'a = 1'" + assert assignment.inline_comment == "Inline comment on 'a = 1'" assignment = routine.walk(Assignment)[1] assert "i = 1" in assignment.debug_string() - assert assignment.preceding_comment == "THIS GETS ATTACHED TO THE ASSIGNMENT 'i = 1'" + assert assignment.preceding_comment == "Preceding comment on 'i = 1'" + assert assignment.inline_comment == "Inline comment on 'i = 1'" diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 5a7660ec03..ab678137bb 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -954,9 +954,10 @@ def test_create_structuretype(): StructureType.create([ ("fred", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None), ("george", Symbol.Visibility.PRIVATE)]) - assert ("Each component must be specified using a 4 or 5-tuple of (name, " - "type, visibility, initial_value, preceding_comment) but found a " - "tuple with 2 members: ('george', " in str(err.value)) + assert ("Each component must be specified using a 4 to 6-tuple of (name, " + "type, visibility, initial_value, preceding_comment, " + "inline_comment) but found a tuple with 2 members: ('george', " + in str(err.value)) def test_structuretype_eq(): From f2e95ff36955f337a0e0146fe36e7e722bbb8811 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Wed, 4 Dec 2024 16:59:48 +0100 Subject: [PATCH 06/20] #1247 Add support for preceding comments on derived types. --- src/psyclone/psyir/backend/fortran.py | 15 ++++- src/psyclone/psyir/frontend/fparser2.py | 59 ++++++++++++++++--- .../psyir/frontend/fparser2_comment_test.py | 41 +++++++++---- .../frontend/fparser2_derived_type_test.py | 10 +++- 4 files changed, 102 insertions(+), 23 deletions(-) diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index 921572a131..8ec4efc618 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -712,7 +712,12 @@ def gen_typedecl(self, symbol, include_visibility=True): f"Fortran backend cannot generate code for symbol " f"'{symbol.name}' of type '{type(symbol.datatype).__name__}'") - result = f"{self._nindent}type" + result = "" + if symbol.preceding_comment != "": + for line in symbol.preceding_comment.splitlines(): + result += f"{self._nindent}{self._COMMENT_PREFIX}{line}\n" + + result += f"{self._nindent}type" if include_visibility: if symbol.visibility == Symbol.Visibility.PRIVATE: @@ -742,7 +747,13 @@ def gen_typedecl(self, symbol, include_visibility=True): include_visibility=include_visibility) self._depth -= 1 - result += f"{self._nindent}end type {symbol.name}\n" + result += f"{self._nindent}end type {symbol.name}" + + if symbol.inline_comment != "": + result += f" {self._COMMENT_PREFIX}{symbol.inline_comment}" + + result += "\n" + return result def gen_default_access_stmt(self, symbol_table): diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index fb00d7feba..a88550395a 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2002,7 +2002,8 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, else: sym.interface = this_interface - def _process_derived_type_decln(self, parent, decl, visibility_map): + def _process_derived_type_decln(self, parent, decl, visibility_map, + preceding_comments_strs=None): ''' Process the supplied fparser2 parse tree for a derived-type declaration. A DataTypeSymbol representing the derived-type is added @@ -2016,12 +2017,21 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): those symbols listed in an accessibility statement). :type visibility_map: dict[str, :py:class:`psyclone.psyir.symbols.Symbol.Visibility`] + :param preceding_comments_strs: the comments that preceded this + declaration, as a list of strings. + :type preceding_comments_strs: Optional[List[str]] :raises SymbolError: if a Symbol already exists with the same name as the derived type being defined and it is not a DataTypeSymbol or is not of UnresolvedType. + + :return: the DataTypeSymbol representing the derived type. + :rtype: :py:class:`psyclone.psyir.symbols.DataTypeSymbol` ''' + if preceding_comments_strs is None: + preceding_comments_strs = [] + name = str(walk(decl.children[0], Fortran2003.Type_Name)[0]).lower() # Create a new StructureType for this derived type dtype = StructureType() @@ -2072,6 +2082,8 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): tsymbol = DataTypeSymbol(name, dtype, visibility=dtype_symbol_vis) parent.symbol_table.add(tsymbol) + tsymbol.preceding_comment = '\n'.join(preceding_comments_strs) + # Populate this StructureType by processing the components of # the derived type try: @@ -2147,6 +2159,8 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): tsymbol.datatype = UnsupportedFortranType(str(decl)) tsymbol.interface = UnknownInterface() + return tsymbol + def _get_partial_datatype(self, node, scope, visibility_map): '''Try to obtain partial datatype information from node by removing any unsupported properties in the declaration. @@ -2433,8 +2447,29 @@ def process_declarations(self, parent, nodes, arg_list, # at general variable declarations in case any of the latter use # the former. # TODO: add support for comments on derived types. - for decl in walk(nodes, Fortran2003.Derived_Type_Def): - self._process_derived_type_decln(parent, decl, visibility_map) + preceding_comments_strs = [] + for node in nodes: + if isinstance(node, Fortran2003.Implicit_Part): + for comment in walk(node, Fortran2003.Comment): + if len(comment.tostr()) == 0: + continue + comment_str = comment.tostr()[1:].strip() + if self.last_symbol_parsed_and_span is not None: + last_sym, last_span \ + = self.last_symbol_parsed_and_span + if (last_span[1] is not None + and last_span[1] == comment.item.span[0]): + last_sym.inline_comment = comment_str + continue + preceding_comments_strs.append(comment_str) + elif isinstance(node, Fortran2003.Derived_Type_Def): + sym = self._process_derived_type_decln(parent, node, + visibility_map, + preceding_comments_strs) + preceding_comments_strs = [] + derived_type_span = (node.children[0].item.span[0], + node.children[-1].item.span[1]) + self.last_symbol_parsed_and_span = (sym, derived_type_span) # INCLUDE statements are *not* part of the Fortran language and # can appear anywhere. Therefore we have to do a walk to make sure we @@ -2908,10 +2943,20 @@ def process_nodes(self, parent, nodes): psy_child.preceding_comment\ += '\n'.join(preceding_comments_strs) preceding_comments_strs = [] - if (isinstance(psy_child, CommentableMixin) - and child.item is not None): - self.last_node_parsed_and_span = (psy_child, - child.item.span) + if isinstance(psy_child, CommentableMixin): + if child.item is not None: + self.last_node_parsed_and_span = (psy_child, + child.item.span) + # If the fparser2 node has no span, try to build one + # from the spans of the first and last children. + elif (len(child.children) != 0 + and (child.children[0] is not None + and child.children[0].item is not None) + and (child.children[-1] is not None + and child.children[-1].item is not None)): + span = (child.children[0].item.span[0], + child.children[-1].item.span[1]) + self.last_node_parsed_and_span = (psy_child, span) 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. diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index c496f41f34..c08ca446ec 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -52,14 +52,17 @@ ! and second line module test_mod implicit none - ! Comment on derived type 'my_type' SHOULD BE LOST + ! Comment on derived type 'my_type' type :: my_type ! Comment on component 'i' ! and second line - integer :: i + integer :: i ! Inline comment on 'integer :: i' ! Comment on component 'j' integer :: j - end type my_type + end type my_type ! Inline comment on 'end type my_type' + ! Comment on derived type 'my_type2' + type :: my_type2 + end type my_type2 ! Inline comment on 'end type my_type2' contains ! Comment on a subroutine subroutine test_sub() @@ -97,9 +100,9 @@ do j = 1, 10 ! Comment on assignment 'a = 6' a = 6 - end do - end do - end subroutine test_sub + end do ! Inline comment on 'end do j = 1, 10' + end do ! Inline comment on 'end do i = 1, 10' + end subroutine test_sub ! Inline comment on 'end subroutine test_sub' end module test_mod """ @@ -143,17 +146,25 @@ def test_comments(): # TODO: add support for comments on derived types. my_type_sym = module.symbol_table.lookup("my_type") - assert my_type_sym.preceding_comment == "" + assert my_type_sym.preceding_comment == "Comment on derived type 'my_type'" + assert my_type_sym.inline_comment == "Inline comment on 'end type my_type'" assert isinstance(my_type_sym.datatype, StructureType) for i, component in enumerate(my_type_sym.datatype.components.values()): if i == 0: assert component.preceding_comment == "Comment on component 'i'\nand second line" + assert component.inline_comment == "Inline comment on 'integer :: i'" else: assert component.preceding_comment == "Comment on component 'j'" + assert component.inline_comment == "" + + my_type2_sym = module.symbol_table.lookup("my_type2") + assert my_type2_sym.preceding_comment == "Comment on derived type 'my_type2'" + assert my_type2_sym.inline_comment == "Inline comment on 'end type my_type2'" routine = module.walk(Routine)[0] assert routine.preceding_comment == "Comment on a subroutine" + assert routine.inline_comment == "Inline comment on 'end subroutine test_sub'" for i, symbol in enumerate(routine.symbol_table.symbols): if i == 0: @@ -177,22 +188,28 @@ def test_comments(): loop_i = loops[0] # OMP directives should be ignored assert loop_i.preceding_comment == "Comment on loop 'do i = 1, 10'" + assert loop_i.inline_comment == "Inline comment on 'end do i = 1, 10'" loop_j = loops[1] assert loop_j.preceding_comment == "Comment on loop 'do j = 1, 10'" + assert loop_j.inline_comment == "Inline comment on 'end do j = 1, 10'" EXPECTED_WITH_COMMENTS = """! Comment on module 'test_mod' ! and second line module test_mod implicit none + ! Comment on derived type 'my_type' type, public :: my_type ! Comment on component 'i' ! and second line - integer, public :: i + integer, public :: i ! Inline comment on 'integer :: i' ! Comment on component 'j' integer, public :: j - end type my_type + end type my_type ! Inline comment on 'end type my_type' + ! Comment on derived type 'my_type2' + type, public :: my_type2 + end type my_type2 ! Inline comment on 'end type my_type2' public contains @@ -232,10 +249,10 @@ def test_comments(): do j = 1, 10, 1 ! Comment on assignment 'a = 6' a = 6 - enddo - enddo + enddo ! Inline comment on 'end do j = 1, 10' + enddo ! Inline comment on 'end do i = 1, 10' - end subroutine test_sub + end subroutine test_sub ! Inline comment on 'end subroutine test_sub' end module test_mod """ diff --git a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py index 2ad3382fb5..7262ad2aab 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py @@ -185,7 +185,10 @@ def test_name_clash_derived_type_def(f2008_parser): # This should raise an error because the Container symbol table will # already contain a RoutineSymbol named 'my_type' with pytest.raises(SymbolError) as err: - processor.process_declarations(fake_parent, fparser2spec.content, []) + processor.process_declarations(fake_parent, + walk(fparser2spec.content, + Fortran2003.Derived_Type_Def), + []) assert ("Error processing definition of derived type 'my_type'. The " "symbol table already contains an entry with this name but it is a" " 'RoutineSymbol' when it should be a 'DataTypeSymbol' (for the " @@ -200,7 +203,10 @@ def test_name_clash_derived_type_def(f2008_parser): " end type my_type2\n" "end subroutine my_sub2\n")) with pytest.raises(SymbolError) as err: - processor.process_declarations(fake_parent, fparser2spec.content, []) + processor.process_declarations(fake_parent, + walk(fparser2spec.content, + Fortran2003.Derived_Type_Def), + []) assert ("Error processing definition of derived type 'my_type2'. The " "symbol table already contains a DataTypeSymbol with this name but" " it is of type 'UnsupportedFortranType' when it should be of " From d9b9ba605404327e2721cfb0068eb8997e57ae79 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Wed, 4 Dec 2024 18:13:52 +0100 Subject: [PATCH 07/20] #1247 Add CodeBlocks for last comments of a block. Some cleanup. --- src/psyclone/psyir/frontend/fparser2.py | 234 +++++++++--------- .../psyir/frontend/fparser2_comment_test.py | 53 +++- 2 files changed, 163 insertions(+), 124 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index a88550395a..9933248bc5 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -1058,7 +1058,7 @@ def last_node_parsed_and_span(self, value): raise TypeError( "The second element of the last_node_parsed_and_span tuple " "must contain two integers.") - + self._last_node_parsed_and_span = value @staticmethod @@ -1707,7 +1707,7 @@ def _process_type_spec(self, parent, type_spec): return base_type, precision def _process_decln(self, scope, symbol_table, decl, visibility_map=None, - statics_list=(), preceding_comments_strs=None): + statics_list=(), preceding_comments=None): ''' Process the supplied fparser2 parse tree for a declaration. For each entity that is declared, a symbol is added to the supplied symbol @@ -1727,9 +1727,12 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, appearing in a SAVE statement). If all symbols are static then this contains the single entry "*". :type statics_list: Iterable[str] - :param preceding_comments_strs: the comments that preceded this - declaration, as a list of strings. - :type preceding_comments_strs: Optional[List[str]] + :param preceding_comments: the comments that preceded this + declaration, as a list of fparser2 Comments. + :type preceding_comments: Optional[ + List[ + :py:class:`fparser.two.Fortran2003.Comment` + ]] :raises NotImplementedError: if an unsupported attribute is found. :raises NotImplementedError: if an unsupported intent attribute is @@ -1751,8 +1754,8 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, ''' # pylint: disable=too-many-arguments - if preceding_comments_strs is None: - preceding_comments_strs = [] + if preceding_comments is None: + preceding_comments = [] (type_spec, attr_specs, entities) = decl.items @@ -1988,8 +1991,8 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, symbol_table.add(sym, tag=tag) # Add any preceding comments to the symbol - sym.preceding_comment = '\n'.join(preceding_comments_strs) - preceding_comments_strs = [] + sym.preceding_comment = self._comments_list_to_string(preceding_comments) + preceding_comments = [] self.last_symbol_parsed_and_span = (sym, decl.item.span) @@ -2003,7 +2006,7 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, sym.interface = this_interface def _process_derived_type_decln(self, parent, decl, visibility_map, - preceding_comments_strs=None): + preceding_comments=None): ''' Process the supplied fparser2 parse tree for a derived-type declaration. A DataTypeSymbol representing the derived-type is added @@ -2017,9 +2020,12 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, those symbols listed in an accessibility statement). :type visibility_map: dict[str, :py:class:`psyclone.psyir.symbols.Symbol.Visibility`] - :param preceding_comments_strs: the comments that preceded this - declaration, as a list of strings. - :type preceding_comments_strs: Optional[List[str]] + :param preceding_comments: the comments that preceded this + declaration, as a list of fparser2 Comments. + :type preceding_comments: Optional[ + List[ + :py:class:`fparser.two.Fortran2003.Comment` + ]] :raises SymbolError: if a Symbol already exists with the same name as the derived type being defined and it is not a DataTypeSymbol @@ -2029,8 +2035,8 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, :rtype: :py:class:`psyclone.psyir.symbols.DataTypeSymbol` ''' - if preceding_comments_strs is None: - preceding_comments_strs = [] + if preceding_comments is None: + preceding_comments = [] name = str(walk(decl.children[0], Fortran2003.Type_Name)[0]).lower() # Create a new StructureType for this derived type @@ -2082,7 +2088,7 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, tsymbol = DataTypeSymbol(name, dtype, visibility=dtype_symbol_vis) parent.symbol_table.add(tsymbol) - tsymbol.preceding_comment = '\n'.join(preceding_comments_strs) + tsymbol.preceding_comment = self._comments_list_to_string(preceding_comments) # Populate this StructureType by processing the components of # the derived type @@ -2110,31 +2116,20 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, local_table = Container("tmp", parent=parent).symbol_table local_table.default_visibility = default_compt_visibility - preceding_comments_strs = [] + preceding_comments = [] for child in decl.children: if isinstance(child, Fortran2003.Derived_Type_Stmt): continue if isinstance(child, Fortran2003.Comment): - # fparser2 generates lots of empty Comment nodes - # (without even a '!'), drop them - if len(child.tostr()) == 0: - continue - comment_str = child.tostr()[1:].strip() - if self.last_symbol_parsed_and_span is not None: - last_sym, last_span \ - = self.last_symbol_parsed_and_span - if (last_span[1] is not None - and last_span[1] == child.item.span[0]): - last_sym.inline_comment = comment_str - continue - preceding_comments_strs.append(comment_str) + self.process_comment(child, preceding_comments, + self.last_symbol_parsed_and_span) continue if isinstance(child, Fortran2003.Component_Part): for component in walk(child, Fortran2003.Data_Component_Def_Stmt): self._process_decln(parent, local_table, component, - preceding_comments_strs=preceding_comments_strs) - preceding_comments_strs = [] + preceding_comments=preceding_comments) + preceding_comments = [] # Convert from Symbols to StructureType components. for symbol in local_table.symbols: if symbol.is_unresolved: @@ -2446,27 +2441,17 @@ def process_declarations(self, parent, nodes, arg_list, # Handle any derived-type declarations/definitions before we look # at general variable declarations in case any of the latter use # the former. - # TODO: add support for comments on derived types. - preceding_comments_strs = [] + preceding_comments = [] for node in nodes: if isinstance(node, Fortran2003.Implicit_Part): for comment in walk(node, Fortran2003.Comment): - if len(comment.tostr()) == 0: - continue - comment_str = comment.tostr()[1:].strip() - if self.last_symbol_parsed_and_span is not None: - last_sym, last_span \ - = self.last_symbol_parsed_and_span - if (last_span[1] is not None - and last_span[1] == comment.item.span[0]): - last_sym.inline_comment = comment_str - continue - preceding_comments_strs.append(comment_str) + self.process_comment(comment, preceding_comments, + self.last_symbol_parsed_and_span) elif isinstance(node, Fortran2003.Derived_Type_Def): sym = self._process_derived_type_decln(parent, node, visibility_map, - preceding_comments_strs) - preceding_comments_strs = [] + preceding_comments) + preceding_comments = [] derived_type_span = (node.children[0].item.span[0], node.children[-1].item.span[1]) self.last_symbol_parsed_and_span = (sym, derived_type_span) @@ -2483,19 +2468,14 @@ def process_declarations(self, parent, nodes, arg_list, # Now we've captured any derived-type definitions, proceed to look # at the variable declarations. - preceding_comments_strs = [] + preceding_comments = [] for node in nodes: if isinstance(node, Fortran2003.Implicit_Part): for comment in walk(node, Fortran2003.Comment): - comment_str = comment.tostr()[1:].strip() - if self.last_symbol_parsed_and_span is not None: - last_sym, last_span = self.last_symbol_parsed_and_span - if (last_span[1] is not None - and last_span[1] == comment.item.span[0]): - last_sym.inline_comment = comment_str - continue - preceding_comments_strs.append(comment_str) + self.process_comment(comment, preceding_comments, + self.last_symbol_parsed_and_span) + continue # Anything other than a PARAMETER statement or an # IMPLICIT NONE means we can't handle this code. # Any PARAMETER statements are handled separately by the @@ -2519,8 +2499,8 @@ def process_declarations(self, parent, nodes, arg_list, try: self._process_decln(parent, parent.symbol_table, node, visibility_map, statics_list, - preceding_comments_strs) - preceding_comments_strs = [] + preceding_comments) + preceding_comments = [] except NotImplementedError: # Found an unsupported variable declaration. Create a # DataSymbol with UnsupportedType for each entity being @@ -2569,7 +2549,7 @@ def process_declarations(self, parent, nodes, arg_list, visibility=vis, initial_value=init) new_symbol.preceding_comment \ - = '\n'.join(preceding_comments_strs) + = '\n'.join(preceding_comments) self.last_symbol_parsed_and_span = (new_symbol, node.item.span) parent.symbol_table.add(new_symbol) @@ -2901,23 +2881,13 @@ def process_nodes(self, parent, nodes): code_block_nodes = [] message = "PSyclone CodeBlock (unsupported code) reason:" # Store any comments that precede the next node - preceding_comments_strs = [] + preceding_comments = [] for child in nodes: # If the child is a comment, attach it to the preceding node if # it is an inline comment or store it for the next node. if isinstance(child, Fortran2003.Comment): - if len(child.tostr()) == 0: - # Skip empty comments - continue - comment_str = child.tostr()[1:].strip() - if self.last_node_parsed_and_span is not None: - last_node, last_span \ - = self.last_node_parsed_and_span - if last_span[1] is not None and \ - last_span[1] == child.item.span[0]: - last_node.inline_comment = comment_str - continue - preceding_comments_strs.append(comment_str) + self.process_comment(child, preceding_comments, + self.last_node_parsed_and_span) continue try: psy_child = self._create_child(child, parent) @@ -2941,8 +2911,8 @@ def process_nodes(self, parent, nodes): # list of comments if isinstance(psy_child, CommentableMixin): psy_child.preceding_comment\ - += '\n'.join(preceding_comments_strs) - preceding_comments_strs = [] + += self._comments_list_to_string(preceding_comments) + preceding_comments = [] if isinstance(psy_child, CommentableMixin): if child.item is not None: self.last_node_parsed_and_span = (psy_child, @@ -2963,6 +2933,9 @@ def process_nodes(self, parent, nodes): # Complete any unfinished code-block self.nodes_to_code_block(parent, code_block_nodes, message) + if len(preceding_comments) != 0: + self.nodes_to_code_block(parent, preceding_comments) + def _create_child(self, child, parent=None): ''' Create a PSyIR node representing the supplied fparser 2 node. @@ -3341,21 +3314,12 @@ def _do_construct_handler(self, node, parent): # Process loop body (ignore 'do' and 'end do' statements) # Keep track of the comments before the 'do' statement loop_body_nodes = [] - preceding_comments_strs = [] + preceding_comments = [] found_do_stmt = False for child in node.content: if isinstance(child, Fortran2003.Comment) and not found_do_stmt: - if child.tostr() == "": - # Skip empty comments - continue - comment_str = child.tostr()[1:].strip() - if self.last_node_parsed_and_span is not None: - last_node, last_span = self.last_node_parsed_and_span - if last_span[1] is not None and \ - last_span[1] == child.item.span[0]: - last_node.inline_comment = comment_str - continue - preceding_comments_strs.append(comment_str) + self.process_comment(child, preceding_comments, + self.last_node_parsed_and_span) continue if isinstance(child, Fortran2003.Nonlabel_Do_Stmt): found_do_stmt = True @@ -3366,7 +3330,7 @@ def _do_construct_handler(self, node, parent): loop_body_nodes.append(child) # Add the comments to the loop node. - loop.preceding_comment = '\n'.join(preceding_comments_strs) + loop.preceding_comment = self._comments_list_to_string(preceding_comments) # Process the loop body. self.process_nodes(parent=loop_body, nodes=loop_body_nodes) @@ -3404,21 +3368,11 @@ def _if_construct_handler(self, node, parent): clause_indices.append(idx) # Get the comments before the 'if' statement - preceding_comments_strs = [] + preceding_comments = [] for child in node.content[:clause_indices[0]]: if isinstance(child, Fortran2003.Comment): - if len(child.tostr()) == 0: - # Skip empty comments - continue - comment_str = child.tostr()[1:].strip() - if self.last_node_parsed_and_span is not None: - last_node, last_span = self.last_node_parsed_and_span - if last_span[1] is not None and \ - last_span[1] == child.item.span[0]: - last_node.inline_comment = comment_str - continue - - preceding_comments_strs.append(comment_str) + self.process_comment(child, preceding_comments, + self.last_node_parsed_and_span) # NOTE: The comments are added to the IfBlock node. # NOTE: Comments before the 'else[if]' statements are not handled. @@ -3453,9 +3407,8 @@ def _if_construct_handler(self, node, parent): newifblock.ast = node.content[start_idx] # Add the comments to the if block. - newifblock.preceding_comment \ - = '\n'.join(preceding_comments_strs) - preceding_comments_strs = [] + newifblock.preceding_comment = self._comments_list_to_string(preceding_comments) + preceding_comments = [] # Create condition as first child self.process_nodes(parent=newifblock, @@ -5391,21 +5344,11 @@ def _subroutine_handler(self, node, parent): # Get the subroutine or function statement and store the comments # that precede it, or attach it to the last parsed node if it is # on the same line. - preceding_comments_strs = [] + preceding_comments = [] for child in node.children: if isinstance(child, Fortran2003.Comment): - if len(child.tostr()) == 0: - continue - comment = child.tostr()[1:].strip() - if self.last_node_parsed_and_span is not None: - last_node, last_span \ - = self.last_node_parsed_and_span - if (isinstance(last_node, CommentableMixin) - and last_span is not None - and last_span[1] == child.item.span[0]): - last_node.inline_comment = comment - continue - preceding_comments_strs.append(comment) + self.process_comment(child, preceding_comments, + self.last_node_parsed_and_span) continue if isinstance(child, (Fortran2003.Subroutine_Stmt, Fortran2003.Function_Stmt)): @@ -5430,7 +5373,7 @@ def _subroutine_handler(self, node, parent): # empty Routine is detached from the tree. parent.addchild(routine) - routine.preceding_comment = '\n'.join(preceding_comments_strs) + routine.preceding_comment = self._comments_list_to_string(preceding_comments) try: routine._ast = node @@ -5718,6 +5661,63 @@ def _program_handler(self, node, parent): self.process_nodes(file_container, node.children) return file_container + def _comment_to_string(self, comment): + '''Convert a comment to a string, by stripping the '!' and any + leading/trailing whitespace. + + :param comment: Comment to convert to a string. + :type comment: :py:class:`fparser.two.utils.Comment` + + :returns: The comment as a string. + :rtype: str + + ''' + return comment.tostr()[1:].strip() + + def _comments_list_to_string(self, comments): + ''' + Convert a list of comments to a single string with line breaks. + + :param comments: List of comments. + :type comments: list[:py:class:`fparser.two.utils.Comment`] + + :returns: A single string containing all the comments. + :rtype: str + + ''' + return '\n'.join([self._comment_to_string(comment) + for comment in comments]) + + def process_comment(self, comment, preceding_comments, last_psyir_and_span): + ''' + Process a comment and attach it to the last PSyIR object (Symbol or + Node) if it is an inline comment. Otherwise append it to the + preceding_comments list. Ignore empty comments. + + :param comment: Comment to process. + :type comment: :py:class:`fparser.two.utils.Comment` + :param preceding_comments: List of comments that precede the next node. + :type preceding_comments: List[:py:class:`fparser.two.utils.Comment`] + :param last_psyir_and_span: Tuple containing the last PSyIR object and + its span. + :type last_psyir_and_span: Tuple[ + Union[ + :py:class:`psyclone.psyir.nodes.Node`, + :py:class:`psyclone.psyir.symbols.Symbol` + ], + Tuple[int, int]] + + ''' + if len(comment.tostr()) == 0: + return + if last_psyir_and_span is not None: + last_sym, last_span = last_psyir_and_span + if last_span[1] is not None and last_span[1] == comment.item.span[0]: + last_sym.inline_comment = self._comment_to_string(comment) + return + + preceding_comments.append(comment) + # For Sphinx AutoAPI documentation generation __all__ = ["Fparser2Reader"] diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index c08ca446ec..d0ee391f7f 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -38,9 +38,11 @@ import pytest +from fparser.two import Fortran2003 + from psyclone.psyir.frontend.fortran import FortranReader from psyclone.psyir.nodes import (Container, Routine, Assignment, - Loop, IfBlock, Call) + Loop, IfBlock, Call, CodeBlock) from psyclone.psyir.nodes.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import DataTypeSymbol, StructureType @@ -82,16 +84,16 @@ if (a == 1) then ! Comment on assignment 'a = 2' a = 2 - ! Comment on elseif block 'elseif (a == 2) then' SHOULD BE LOST + ! Comment on elseif block 'elseif (a == 2) then' => CodeBlock elseif (a == 2) then ! Comment on assignment 'a = 3' a = 3 - ! Comment on else block 'else' SHOULD BE LOST + ! Comment on else block 'else' => CodeBlock else ! Comment on assignment 'a = 4' a = 4 - ! Comment on 'end if' SHOULD BE LOST - end if + ! Comment on 'end if' => CodeBlock + end if ! Inline comment on 'end if' ! Comment on loop 'do i = 1, 10' do i = 1, 10 ! Comment on assignment 'a = 5' @@ -100,8 +102,11 @@ do j = 1, 10 ! Comment on assignment 'a = 6' a = 6 + ! Comment at end of loop on j => CodeBlock end do ! Inline comment on 'end do j = 1, 10' + ! Comment at end of loop on i => CodeBlock end do ! Inline comment on 'end do i = 1, 10' + ! Comment at end of subroutine => CodeBlock end subroutine test_sub ! Inline comment on 'end subroutine test_sub' end module test_mod """ @@ -135,6 +140,8 @@ def test_no_comments(): for node in commentable_nodes: assert node.preceding_comment == "" + assert len(routine.walk(CodeBlock)) == 0 + def test_comments(): """Test that the FortranReader is able to read comments""" @@ -165,6 +172,10 @@ def test_comments(): routine = module.walk(Routine)[0] assert routine.preceding_comment == "Comment on a subroutine" assert routine.inline_comment == "Inline comment on 'end subroutine test_sub'" + last_child = routine.children[-1] + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert last_child.ast.tostr() == "! Comment at end of subroutine => CodeBlock" for i, symbol in enumerate(routine.symbol_table.symbols): if i == 0: @@ -183,16 +194,38 @@ def test_comments(): ifblock = routine.walk(IfBlock)[0] assert ifblock.preceding_comment == "Comment on if block 'if (a == 1) then'" + last_child = ifblock.if_body.children[-1] + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert last_child.ast.tostr() == "! Comment on elseif block 'elseif (a == 2) then' => CodeBlock" + ifblock2 = ifblock.else_body.children[0] + last_child = ifblock2.if_body.children[-1] + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert last_child.ast.tostr() == "! Comment on else block 'else' => CodeBlock" + last_child = ifblock2.else_body.children[-1] + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert last_child.ast.tostr() == "! Comment on 'end if' => CodeBlock" loops = routine.walk(Loop) loop_i = loops[0] - # OMP directives should be ignored + assert loop_i.variable.name == "i" assert loop_i.preceding_comment == "Comment on loop 'do i = 1, 10'" assert loop_i.inline_comment == "Inline comment on 'end do i = 1, 10'" + last_child = loop_i.loop_body.children[-1] + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert last_child.ast.tostr() == "! Comment at end of loop on i => CodeBlock" loop_j = loops[1] + assert loop_j.variable.name == "j" assert loop_j.preceding_comment == "Comment on loop 'do j = 1, 10'" assert loop_j.inline_comment == "Inline comment on 'end do j = 1, 10'" + last_child = loop_j.loop_body.children[-1] + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert last_child.ast.tostr() == "! Comment at end of loop on j => CodeBlock" EXPECTED_WITH_COMMENTS = """! Comment on module 'test_mod' @@ -232,15 +265,18 @@ def test_comments(): if (a == 1) then ! Comment on assignment 'a = 2' a = 2 + ! Comment on elseif block 'elseif (a == 2) then' => CodeBlock else if (a == 2) then ! Comment on assignment 'a = 3' a = 3 + ! Comment on else block 'else' => CodeBlock else ! Comment on assignment 'a = 4' a = 4 + ! Comment on 'end if' => CodeBlock end if - end if + end if ! Inline comment on 'end if' ! Comment on loop 'do i = 1, 10' do i = 1, 10, 1 ! Comment on assignment 'a = 5' @@ -249,8 +285,11 @@ def test_comments(): do j = 1, 10, 1 ! Comment on assignment 'a = 6' a = 6 + ! Comment at end of loop on j => CodeBlock enddo ! Inline comment on 'end do j = 1, 10' + ! Comment at end of loop on i => CodeBlock enddo ! Inline comment on 'end do i = 1, 10' + ! Comment at end of subroutine => CodeBlock end subroutine test_sub ! Inline comment on 'end subroutine test_sub' From 96f3f050405bd670bbf9acc9e1428c4f0ec45243 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 9 Dec 2024 15:59:19 +0100 Subject: [PATCH 08/20] #1247 Formatting --- src/psyclone/psyir/backend/fortran.py | 6 +- src/psyclone/psyir/frontend/fparser2.py | 52 ++++---- src/psyclone/psyir/symbols/datatypes.py | 2 +- src/psyclone/psyir/symbols/symbol.py | 2 +- .../psyir/frontend/fparser2_comment_test.py | 111 ++++++++++++++---- 5 files changed, 123 insertions(+), 50 deletions(-) diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index 8ec4efc618..b1d05135e9 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -561,14 +561,16 @@ def gen_vardecl(self, symbol, include_visibility=False): symbol) result += f"{self._nindent}{decln}" if symbol.inline_comment != "": - result += f" {self._COMMENT_PREFIX}{symbol.inline_comment}" + result += (f" {self._COMMENT_PREFIX}" + f"{symbol.inline_comment}") result += "\n" return result decln = symbol.datatype.declaration result += f"{self._nindent}{decln}" if symbol.inline_comment != "": - result += f" {self._COMMENT_PREFIX}{symbol.inline_comment}" + result += (f" {self._COMMENT_PREFIX}" + f"{symbol.inline_comment}") result += "\n" return result # The Fortran backend only handles UnsupportedFortranType diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 9933248bc5..4e9c536ef9 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -995,12 +995,12 @@ def last_symbol_parsed_and_span(self): @last_symbol_parsed_and_span.setter def last_symbol_parsed_and_span(self, value): '''Setter for the last_symbol_parsed_and_span property. - + :param value: the last symbol parsed and the span of source code lines it was found in. :type value: Tuple[:py:class:`psyclone.psyir.symbols.Symbol`, Tuple[int, int]] - + :raises TypeError: if the value is not of the right type. ''' if not isinstance(value, tuple) or len(value) != 2: @@ -1034,7 +1034,7 @@ def last_node_parsed_and_span(self): @last_node_parsed_and_span.setter def last_node_parsed_and_span(self, value): '''Setter for the last_node_parsed_and_span property. - + :param value: the last node parsed and the span of source code lines it was found in. :type value: Tuple[:py:class:`psyclone.psyclone.psyir.nodes.Node`, @@ -1991,7 +1991,8 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, symbol_table.add(sym, tag=tag) # Add any preceding comments to the symbol - sym.preceding_comment = self._comments_list_to_string(preceding_comments) + sym.preceding_comment\ + = self._comments_list_to_string(preceding_comments) preceding_comments = [] self.last_symbol_parsed_and_span = (sym, decl.item.span) @@ -2030,7 +2031,7 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, :raises SymbolError: if a Symbol already exists with the same name as the derived type being defined and it is not a DataTypeSymbol or is not of UnresolvedType. - + :return: the DataTypeSymbol representing the derived type. :rtype: :py:class:`psyclone.psyir.symbols.DataTypeSymbol` @@ -2088,7 +2089,8 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, tsymbol = DataTypeSymbol(name, dtype, visibility=dtype_symbol_vis) parent.symbol_table.add(tsymbol) - tsymbol.preceding_comment = self._comments_list_to_string(preceding_comments) + tsymbol.preceding_comment\ + = self._comments_list_to_string(preceding_comments) # Populate this StructureType by processing the components of # the derived type @@ -2127,8 +2129,9 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, if isinstance(child, Fortran2003.Component_Part): for component in walk(child, Fortran2003.Data_Component_Def_Stmt): - self._process_decln(parent, local_table, component, - preceding_comments=preceding_comments) + self._process_decln( + parent, local_table, component, + preceding_comments=preceding_comments) preceding_comments = [] # Convert from Symbols to StructureType components. for symbol in local_table.symbols: @@ -2453,7 +2456,7 @@ def process_declarations(self, parent, nodes, arg_list, preceding_comments) preceding_comments = [] derived_type_span = (node.children[0].item.span[0], - node.children[-1].item.span[1]) + node.children[-1].item.span[1]) self.last_symbol_parsed_and_span = (sym, derived_type_span) # INCLUDE statements are *not* part of the Fortran language and @@ -2911,7 +2914,8 @@ def process_nodes(self, parent, nodes): # list of comments if isinstance(psy_child, CommentableMixin): psy_child.preceding_comment\ - += self._comments_list_to_string(preceding_comments) + += self._comments_list_to_string( + preceding_comments) preceding_comments = [] if isinstance(psy_child, CommentableMixin): if child.item is not None: @@ -3330,7 +3334,8 @@ def _do_construct_handler(self, node, parent): loop_body_nodes.append(child) # Add the comments to the loop node. - loop.preceding_comment = self._comments_list_to_string(preceding_comments) + loop.preceding_comment\ + = self._comments_list_to_string(preceding_comments) # Process the loop body. self.process_nodes(parent=loop_body, nodes=loop_body_nodes) @@ -3407,7 +3412,8 @@ def _if_construct_handler(self, node, parent): newifblock.ast = node.content[start_idx] # Add the comments to the if block. - newifblock.preceding_comment = self._comments_list_to_string(preceding_comments) + newifblock.preceding_comment\ + = self._comments_list_to_string(preceding_comments) preceding_comments = [] # Create condition as first child @@ -5373,7 +5379,8 @@ def _subroutine_handler(self, node, parent): # empty Routine is detached from the tree. parent.addchild(routine) - routine.preceding_comment = self._comments_list_to_string(preceding_comments) + routine.preceding_comment\ + = self._comments_list_to_string(preceding_comments) try: routine._ast = node @@ -5415,8 +5422,9 @@ def _subroutine_handler(self, node, parent): last_symbol, last_span \ = self.last_symbol_parsed_and_span if (last_span is not None - and last_span[1] == comment.item.span[0]): - last_symbol.inline_comment = comment.tostr()[1:].strip() + and last_span[1] == comment.item.span[0]): + last_symbol.inline_comment\ + = self._comment_to_string(comment) continue lost_comments.append(comment) @@ -5664,13 +5672,13 @@ def _program_handler(self, node, parent): def _comment_to_string(self, comment): '''Convert a comment to a string, by stripping the '!' and any leading/trailing whitespace. - + :param comment: Comment to convert to a string. :type comment: :py:class:`fparser.two.utils.Comment` - + :returns: The comment as a string. :rtype: str - + ''' return comment.tostr()[1:].strip() @@ -5688,9 +5696,10 @@ def _comments_list_to_string(self, comments): return '\n'.join([self._comment_to_string(comment) for comment in comments]) - def process_comment(self, comment, preceding_comments, last_psyir_and_span): + def process_comment(self, comment, preceding_comments, + last_psyir_and_span): ''' - Process a comment and attach it to the last PSyIR object (Symbol or + Process a comment and attach it to the last PSyIR object (Symbol or Node) if it is an inline comment. Otherwise append it to the preceding_comments list. Ignore empty comments. @@ -5712,7 +5721,8 @@ def process_comment(self, comment, preceding_comments, last_psyir_and_span): return if last_psyir_and_span is not None: last_sym, last_span = last_psyir_and_span - if last_span[1] is not None and last_span[1] == comment.item.span[0]: + if (last_span[1] is not None + and last_span[1] == comment.item.span[0]): last_sym.inline_comment = self._comment_to_string(comment) return diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index e0af1f57d0..a025906f31 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -920,7 +920,7 @@ class ComponentType: :type initial_value: Optional[:py:class:`psyclone.psyir.nodes.Node`] :param preceding_comment: a comment that precedes this component. :type preceding_comment: Optional[str] - :param inline_comment: a comment that follows this component on the + :param inline_comment: a comment that follows this component on the same line. :type inline_comment: Optional[str] ''' diff --git a/src/psyclone/psyir/symbols/symbol.py b/src/psyclone/psyir/symbols/symbol.py index 8f3c17d544..efa566abc1 100644 --- a/src/psyclone/psyir/symbols/symbol.py +++ b/src/psyclone/psyir/symbols/symbol.py @@ -148,7 +148,7 @@ def copy(self): # The constructors for all Symbol-based classes have 'name' as the # first positional argument. copy = type(self)(self.name, visibility=self.visibility, - interface=self.interface.copy()) + interface=self.interface.copy()) copy.preceding_comment = self.preceding_comment copy.inline_comment = self.inline_comment return copy diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index d0ee391f7f..859503ca48 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -41,8 +41,15 @@ from fparser.two import Fortran2003 from psyclone.psyir.frontend.fortran import FortranReader -from psyclone.psyir.nodes import (Container, Routine, Assignment, - Loop, IfBlock, Call, CodeBlock) +from psyclone.psyir.nodes import ( + Container, + Routine, + Assignment, + Loop, + IfBlock, + Call, + CodeBlock, +) from psyclone.psyir.nodes.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import DataTypeSymbol, StructureType @@ -149,7 +156,10 @@ def test_comments(): psyir = reader.psyir_from_source(CODE, ignore_comments=False) module = psyir.children[0] - assert module.preceding_comment == "Comment on module 'test_mod'\nand second line" + assert ( + module.preceding_comment + == "Comment on module 'test_mod'\nand second line" + ) # TODO: add support for comments on derived types. my_type_sym = module.symbol_table.lookup("my_type") @@ -159,50 +169,82 @@ def test_comments(): assert isinstance(my_type_sym.datatype, StructureType) for i, component in enumerate(my_type_sym.datatype.components.values()): if i == 0: - assert component.preceding_comment == "Comment on component 'i'\nand second line" - assert component.inline_comment == "Inline comment on 'integer :: i'" + assert ( + component.preceding_comment + == "Comment on component 'i'\nand second line" + ) + assert ( + component.inline_comment == "Inline comment on 'integer :: i'" + ) else: assert component.preceding_comment == "Comment on component 'j'" assert component.inline_comment == "" my_type2_sym = module.symbol_table.lookup("my_type2") - assert my_type2_sym.preceding_comment == "Comment on derived type 'my_type2'" - assert my_type2_sym.inline_comment == "Inline comment on 'end type my_type2'" + assert ( + my_type2_sym.preceding_comment == "Comment on derived type 'my_type2'" + ) + assert ( + my_type2_sym.inline_comment == "Inline comment on 'end type my_type2'" + ) routine = module.walk(Routine)[0] assert routine.preceding_comment == "Comment on a subroutine" - assert routine.inline_comment == "Inline comment on 'end subroutine test_sub'" + assert ( + routine.inline_comment == "Inline comment on 'end subroutine test_sub'" + ) last_child = routine.children[-1] assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) - assert last_child.ast.tostr() == "! Comment at end of subroutine => CodeBlock" + assert ( + last_child.ast.tostr() == "! Comment at end of subroutine => CodeBlock" + ) for i, symbol in enumerate(routine.symbol_table.symbols): if i == 0: - assert symbol.preceding_comment == "Comment on variable 'a'\nand second line" + assert ( + symbol.preceding_comment + == "Comment on variable 'a'\nand second line" + ) else: - assert symbol.preceding_comment == f"Comment on variable '{symbol.name}'" + assert ( + symbol.preceding_comment + == f"Comment on variable '{symbol.name}'" + ) for i, assignment in enumerate(routine.walk(Assignment)): if i == 0: - assert assignment.preceding_comment == "Comment on assignment 'a = 1'\nand second line" + assert ( + assignment.preceding_comment + == "Comment on assignment 'a = 1'\nand second line" + ) else: - assert assignment.preceding_comment == f"Comment on assignment 'a = {i+1}'" + assert ( + assignment.preceding_comment + == f"Comment on assignment 'a = {i+1}'" + ) call = routine.walk(Call)[0] assert call.preceding_comment == "Comment on call 'call test_sub()'" ifblock = routine.walk(IfBlock)[0] - assert ifblock.preceding_comment == "Comment on if block 'if (a == 1) then'" + assert ( + ifblock.preceding_comment == "Comment on if block 'if (a == 1) then'" + ) last_child = ifblock.if_body.children[-1] assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) - assert last_child.ast.tostr() == "! Comment on elseif block 'elseif (a == 2) then' => CodeBlock" + assert ( + last_child.ast.tostr() + == "! Comment on elseif block 'elseif (a == 2) then' => CodeBlock" + ) ifblock2 = ifblock.else_body.children[0] last_child = ifblock2.if_body.children[-1] assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) - assert last_child.ast.tostr() == "! Comment on else block 'else' => CodeBlock" + assert ( + last_child.ast.tostr() == "! Comment on else block 'else' => CodeBlock" + ) last_child = ifblock2.else_body.children[-1] assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) @@ -216,7 +258,9 @@ def test_comments(): last_child = loop_i.loop_body.children[-1] assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) - assert last_child.ast.tostr() == "! Comment at end of loop on i => CodeBlock" + assert ( + last_child.ast.tostr() == "! Comment at end of loop on i => CodeBlock" + ) loop_j = loops[1] assert loop_j.variable.name == "j" @@ -225,7 +269,9 @@ def test_comments(): last_child = loop_j.loop_body.children[-1] assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) - assert last_child.ast.tostr() == "! Comment at end of loop on j => CodeBlock" + assert ( + last_child.ast.tostr() == "! Comment at end of loop on j => CodeBlock" + ) EXPECTED_WITH_COMMENTS = """! Comment on module 'test_mod' @@ -322,7 +368,9 @@ def test_write_comments(): def test_no_directives(): """Test that the FortranReader is without directives by default""" reader = FortranReader() - psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE, ignore_comments=False) + psyir = reader.psyir_from_source( + CODE_WITH_DIRECTIVE, ignore_comments=False + ) loop = psyir.walk(Loop)[0] assert loop.preceding_comment == "Comment on loop 'do i = 1, 10'" @@ -331,10 +379,15 @@ def test_no_directives(): def test_directives(): """Test that the FortranReader is able to read directives""" reader = FortranReader() - psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False) + psyir = reader.psyir_from_source( + CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False + ) loop = psyir.walk(Loop)[0] - assert loop.preceding_comment == "Comment on loop 'do i = 1, 10'\n$omp parallel do" + assert ( + loop.preceding_comment + == "Comment on loop 'do i = 1, 10'\n$omp parallel do" + ) EXPECTED_WITH_DIRECTIVES = """subroutine test_sub() @@ -350,13 +403,18 @@ def test_directives(): end subroutine test_sub """ -@pytest.mark.xfail(reason="Directive is written back as '! $omp parallel do'" - "instead of '!$omp parallel do'") + +@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() writer = FortranWriter() - psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False) + psyir = reader.psyir_from_source( + CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False + ) generated_code = writer(psyir) assert generated_code == EXPECTED_WITH_DIRECTIVES @@ -373,10 +431,13 @@ def test_write_directives(): end subroutine test_sub """ + def test_inline_comment(): """Test that the FortranReader is able to read inline comments""" reader = FortranReader() - psyir = reader.psyir_from_source(CODE_WITH_INLINE_COMMENT, ignore_comments=False) + psyir = reader.psyir_from_source( + CODE_WITH_INLINE_COMMENT, ignore_comments=False + ) routine = psyir.walk(Routine)[0] sym_a = routine.symbol_table.lookup("a") From 3f910a925659c718f4fb9c540f218d774579b082 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 9 Dec 2024 16:49:19 +0100 Subject: [PATCH 09/20] #1247 Make last comments as codeblocks optional. --- src/psyclone/psyir/frontend/fortran.py | 27 ++- src/psyclone/psyir/frontend/fparser2.py | 30 ++- .../psyir/frontend/fparser2_comment_test.py | 186 ++++++++++++++---- 3 files changed, 201 insertions(+), 42 deletions(-) diff --git a/src/psyclone/psyir/frontend/fortran.py b/src/psyclone/psyir/frontend/fortran.py index df0af1c2ea..ca4b9c9004 100644 --- a/src/psyclone/psyir/frontend/fortran.py +++ b/src/psyclone/psyir/frontend/fortran.py @@ -87,7 +87,8 @@ def validate_name(name: str): def psyir_from_source(self, source_code: str, free_form: bool = True, ignore_comments: bool = True, - ignore_directives: bool = True): + ignore_directives: bool = True, + last_comments_as_codeblocks: bool = False): ''' Generate the PSyIR tree representing the given Fortran source code. :param source_code: text representation of the code to be parsed. @@ -97,7 +98,13 @@ def psyir_from_source(self, source_code: str, free_form: bool = True, :param ignore_directives: If directives should be ignored or not (default True). Only has an effect if ignore_comments is False. - + :param last_comments_as_codeblocks: If the last comments in the + a given block (e.g. subroutine, + do, if-then body, etc.) should + be kept as code blocks or lost + (default False). + Only has an effect if + ignore_comments is False. :returns: PSyIR representing the provided Fortran source code. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -112,6 +119,8 @@ def psyir_from_source(self, source_code: str, free_form: bool = True, if (not ignore_comments) and ignore_directives: self._strip_directives(parse_tree) + self._processor.last_comments_as_codeblocks\ + = last_comments_as_codeblocks psyir = self._processor.generate_psyir(parse_tree) return psyir @@ -208,7 +217,8 @@ def psyir_from_statement(self, source_code: str, def psyir_from_file(self, file_path, free_form=True, ignore_comments: bool = True, - ignore_directives: bool = True): + ignore_directives: bool = True, + last_comments_as_codeblocks: bool = False): ''' Generate the PSyIR tree representing the given Fortran file. :param file_path: path of the file to be read and parsed. @@ -220,11 +230,18 @@ def psyir_from_file(self, file_path, free_form=True, :param ignore_comments: If comments should be ignored or not (default True). :type ignore_comments: bool - :param ignore_directives: If directives should be ignored or not (default True). Only has an effect if ignore_comments is False. :type ignore_directives: bool + :param last_comments_as_codeblocks: If the last comments in the + a given block (e.g. subroutine, + do, if-then body, etc.) should + be kept as code blocks or lost + (default False). + Only has an effect if + ignore_comments is False. + :type last_comments_as_codeblocks: bool :returns: PSyIR representing the provided Fortran file. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -248,6 +265,8 @@ def psyir_from_file(self, file_path, free_form=True, if (not ignore_comments) and ignore_directives: self._strip_directives(parse_tree) + self._processor.last_comments_as_codeblocks\ + = last_comments_as_codeblocks psyir = self._processor.generate_psyir(parse_tree, filename) return psyir diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 4e9c536ef9..d22030ee76 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -982,6 +982,7 @@ def __init__(self): # Used to attach inline comments to the PSyIR symbols and nodes self._last_symbol_parsed_and_span = None self._last_node_parsed_and_span = None + self._last_comments_as_codeblocks = False @property def last_symbol_parsed_and_span(self): @@ -1061,6 +1062,33 @@ def last_node_parsed_and_span(self, value): self._last_node_parsed_and_span = value + @property + def last_comments_as_codeblocks(self): + ''' + :returns: whether the last comments in a given block (e.g. subroutine, + do, if-then body, etc.) should be kept as code blocks or lost + (default False). + Only has an effect if the fparser2 parse tree has comments. + ''' + return self._last_comments_as_codeblocks + + @last_comments_as_codeblocks.setter + def last_comments_as_codeblocks(self, value): + '''Setter for the last_comments_as_codeblocks property. + + :param value: whether the last comments in a given block (e.g. + subroutine, do, if-then body, etc.) should be kept as + code blocks or lost. + Only has an effect if the fparser2 parse tree has + comments. + :type value: bool + ''' + if not isinstance(value, bool): + raise TypeError( + "The value of the last_comments_as_codeblocks property must " + "be a boolean.") + self._last_comments_as_codeblocks = value + @staticmethod def nodes_to_code_block(parent, fp2_nodes, message=None): '''Create a CodeBlock for the supplied list of fparser2 nodes and then @@ -2937,7 +2965,7 @@ def process_nodes(self, parent, nodes): # Complete any unfinished code-block self.nodes_to_code_block(parent, code_block_nodes, message) - if len(preceding_comments) != 0: + if self.last_comments_as_codeblocks and len(preceding_comments) != 0: self.nodes_to_code_block(parent, preceding_comments) def _create_child(self, child, parent=None): diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index 859503ca48..a578dd6917 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -113,8 +113,15 @@ end do ! Inline comment on 'end do j = 1, 10' ! Comment at end of loop on i => CodeBlock end do ! Inline comment on 'end do i = 1, 10' + ! Comment on 'do while (a < 10)' + do while (a < 10) + ! Comment on assignment 'a = 7' + a = 7 ! Inline comment on 'a = 7' + ! Comment at end of while loop => CodeBlock + end do ! Inline comment on 'end do while (a < 10)' ! Comment at end of subroutine => CodeBlock end subroutine test_sub ! Inline comment on 'end subroutine test_sub' +! Comment at end of module => CodeBlock end module test_mod """ @@ -149,19 +156,27 @@ def test_no_comments(): assert len(routine.walk(CodeBlock)) == 0 - -def test_comments(): +@pytest.mark.parametrize("last_comments_as_codeblocks", [True, False]) +def test_comments_and_codeblocks(last_comments_as_codeblocks): """Test that the FortranReader is able to read comments""" reader = FortranReader() - psyir = reader.psyir_from_source(CODE, ignore_comments=False) + psyir = reader.psyir_from_source( + CODE, ignore_comments=False, + last_comments_as_codeblocks=last_comments_as_codeblocks) module = psyir.children[0] assert ( module.preceding_comment == "Comment on module 'test_mod'\nand second line" ) + if last_comments_as_codeblocks: + assert isinstance(module.children[-1], CodeBlock) + assert isinstance(module.children[-1].ast, Fortran2003.Comment) + assert (module.children[-1].ast.tostr() + == "! Comment at end of module => CodeBlock") + else: + assert not isinstance(module.children[-1], CodeBlock) - # TODO: add support for comments on derived types. my_type_sym = module.symbol_table.lookup("my_type") assert my_type_sym.preceding_comment == "Comment on derived type 'my_type'" assert my_type_sym.inline_comment == "Inline comment on 'end type my_type'" @@ -194,11 +209,14 @@ def test_comments(): routine.inline_comment == "Inline comment on 'end subroutine test_sub'" ) last_child = routine.children[-1] - assert isinstance(last_child, CodeBlock) - assert isinstance(last_child.ast, Fortran2003.Comment) - assert ( - last_child.ast.tostr() == "! Comment at end of subroutine => CodeBlock" - ) + if last_comments_as_codeblocks: + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert ( + last_child.ast.tostr() == "! Comment at end of subroutine => CodeBlock" + ) + else: + assert not isinstance(last_child, CodeBlock) for i, symbol in enumerate(routine.symbol_table.symbols): if i == 0: @@ -232,23 +250,32 @@ def test_comments(): ifblock.preceding_comment == "Comment on if block 'if (a == 1) then'" ) last_child = ifblock.if_body.children[-1] - assert isinstance(last_child, CodeBlock) - assert isinstance(last_child.ast, Fortran2003.Comment) - assert ( - last_child.ast.tostr() - == "! Comment on elseif block 'elseif (a == 2) then' => CodeBlock" - ) + if last_comments_as_codeblocks: + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert ( + last_child.ast.tostr() + == "! Comment on elseif block 'elseif (a == 2) then' => CodeBlock" + ) + else: + assert not isinstance(last_child, CodeBlock) ifblock2 = ifblock.else_body.children[0] last_child = ifblock2.if_body.children[-1] - assert isinstance(last_child, CodeBlock) - assert isinstance(last_child.ast, Fortran2003.Comment) - assert ( - last_child.ast.tostr() == "! Comment on else block 'else' => CodeBlock" - ) + if last_comments_as_codeblocks: + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert ( + last_child.ast.tostr() == "! Comment on else block 'else' => CodeBlock" + ) + else: + assert not isinstance(last_child, CodeBlock) last_child = ifblock2.else_body.children[-1] - assert isinstance(last_child, CodeBlock) - assert isinstance(last_child.ast, Fortran2003.Comment) - assert last_child.ast.tostr() == "! Comment on 'end if' => CodeBlock" + if last_comments_as_codeblocks: + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert last_child.ast.tostr() == "! Comment on 'end if' => CodeBlock" + else: + assert not isinstance(last_child, CodeBlock) loops = routine.walk(Loop) loop_i = loops[0] @@ -256,25 +283,31 @@ def test_comments(): assert loop_i.preceding_comment == "Comment on loop 'do i = 1, 10'" assert loop_i.inline_comment == "Inline comment on 'end do i = 1, 10'" last_child = loop_i.loop_body.children[-1] - assert isinstance(last_child, CodeBlock) - assert isinstance(last_child.ast, Fortran2003.Comment) - assert ( - last_child.ast.tostr() == "! Comment at end of loop on i => CodeBlock" - ) + if last_comments_as_codeblocks: + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert ( + last_child.ast.tostr() == "! Comment at end of loop on i => CodeBlock" + ) + else: + assert not isinstance(last_child, CodeBlock) loop_j = loops[1] assert loop_j.variable.name == "j" assert loop_j.preceding_comment == "Comment on loop 'do j = 1, 10'" assert loop_j.inline_comment == "Inline comment on 'end do j = 1, 10'" last_child = loop_j.loop_body.children[-1] - assert isinstance(last_child, CodeBlock) - assert isinstance(last_child.ast, Fortran2003.Comment) - assert ( - last_child.ast.tostr() == "! Comment at end of loop on j => CodeBlock" - ) + if last_comments_as_codeblocks: + assert isinstance(last_child, CodeBlock) + assert isinstance(last_child.ast, Fortran2003.Comment) + assert ( + last_child.ast.tostr() == "! Comment at end of loop on j => CodeBlock" + ) + else: + assert not isinstance(last_child, CodeBlock) -EXPECTED_WITH_COMMENTS = """! Comment on module 'test_mod' +EXPECTED_WITH_COMMENTS_AND_CODEBLOCKS = """! Comment on module 'test_mod' ! and second line module test_mod implicit none @@ -335,21 +368,100 @@ def test_comments(): enddo ! Inline comment on 'end do j = 1, 10' ! Comment at end of loop on i => CodeBlock enddo ! Inline comment on 'end do i = 1, 10' + ! Comment on 'do while (a < 10)' + do while (a < 10) + ! Comment on assignment 'a = 7' + a = 7 ! Inline comment on 'a = 7' + ! Comment at end of while loop => CodeBlock + end do ! Inline comment on 'end do while (a < 10)' ! Comment at end of subroutine => CodeBlock end subroutine test_sub ! Inline comment on 'end subroutine test_sub' + ! Comment at end of module => CodeBlock end module test_mod """ +EXPECTED_WITH_COMMENTS_AND_NO_CODEBLOCKS = """! Comment on module 'test_mod' +! and second line +module test_mod + implicit none + ! Comment on derived type 'my_type' + type, public :: my_type + ! Comment on component 'i' + ! and second line + integer, public :: i ! Inline comment on 'integer :: i' + ! Comment on component 'j' + integer, public :: j + end type my_type ! Inline comment on 'end type my_type' + ! Comment on derived type 'my_type2' + type, public :: my_type2 + end type my_type2 ! Inline comment on 'end type my_type2' + public -def test_write_comments(): + contains + ! Comment on a subroutine + subroutine test_sub() + ! Comment on variable 'a' + ! and second line + integer :: a + ! Comment on variable 'i' + integer :: i + ! Comment on variable 'j' + integer :: j + + ! Comment on assignment 'a = 1' + ! and second line + a = 1 + ! Comment on call 'call test_sub()' + call test_sub() + ! Comment on if block 'if (a == 1) then' + if (a == 1) then + ! Comment on assignment 'a = 2' + a = 2 + else + if (a == 2) then + ! Comment on assignment 'a = 3' + a = 3 + else + ! Comment on assignment 'a = 4' + a = 4 + end if + end if ! Inline comment on 'end if' + ! Comment on loop 'do i = 1, 10' + do i = 1, 10, 1 + ! Comment on assignment 'a = 5' + a = 5 + ! Comment on loop 'do j = 1, 10' + do j = 1, 10, 1 + ! Comment on assignment 'a = 6' + a = 6 + enddo ! Inline comment on 'end do j = 1, 10' + enddo ! Inline comment on 'end do i = 1, 10' + ! Comment on 'do while (a < 10)' + do while (a < 10) + ! Comment on assignment 'a = 7' + a = 7 ! Inline comment on 'a = 7' + end do ! Inline comment on 'end do while (a < 10)' + + end subroutine test_sub ! Inline comment on 'end subroutine test_sub' + +end module test_mod +""" + +@pytest.mark.parametrize("last_comments_as_codeblocks", [True, False]) +def test_write_comments(last_comments_as_codeblocks): """Test that the comments are written back to the code""" reader = FortranReader() writer = FortranWriter() - psyir = reader.psyir_from_source(CODE, ignore_comments=False) + psyir = reader.psyir_from_source( + CODE, ignore_comments=False, + last_comments_as_codeblocks=last_comments_as_codeblocks) generated_code = writer(psyir) - assert generated_code == EXPECTED_WITH_COMMENTS + if last_comments_as_codeblocks: + assert generated_code == EXPECTED_WITH_COMMENTS_AND_CODEBLOCKS + else: + assert generated_code == EXPECTED_WITH_COMMENTS_AND_NO_CODEBLOCKS CODE_WITH_DIRECTIVE = """ From ba7d5fb7d123457bcb452ac4b380a974ac983a9d Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 9 Dec 2024 16:50:59 +0100 Subject: [PATCH 10/20] #1247 Formatting --- .../tests/psyir/frontend/fparser2_comment_test.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index a578dd6917..b385f8a335 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -156,6 +156,7 @@ def test_no_comments(): assert len(routine.walk(CodeBlock)) == 0 + @pytest.mark.parametrize("last_comments_as_codeblocks", [True, False]) def test_comments_and_codeblocks(last_comments_as_codeblocks): """Test that the FortranReader is able to read comments""" @@ -213,7 +214,8 @@ def test_comments_and_codeblocks(last_comments_as_codeblocks): assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) assert ( - last_child.ast.tostr() == "! Comment at end of subroutine => CodeBlock" + last_child.ast.tostr() + == "! Comment at end of subroutine => CodeBlock" ) else: assert not isinstance(last_child, CodeBlock) @@ -265,7 +267,8 @@ def test_comments_and_codeblocks(last_comments_as_codeblocks): assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) assert ( - last_child.ast.tostr() == "! Comment on else block 'else' => CodeBlock" + last_child.ast.tostr() + == "! Comment on else block 'else' => CodeBlock" ) else: assert not isinstance(last_child, CodeBlock) @@ -287,7 +290,8 @@ def test_comments_and_codeblocks(last_comments_as_codeblocks): assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) assert ( - last_child.ast.tostr() == "! Comment at end of loop on i => CodeBlock" + last_child.ast.tostr() + == "! Comment at end of loop on i => CodeBlock" ) else: assert not isinstance(last_child, CodeBlock) @@ -301,7 +305,8 @@ def test_comments_and_codeblocks(last_comments_as_codeblocks): assert isinstance(last_child, CodeBlock) assert isinstance(last_child.ast, Fortran2003.Comment) assert ( - last_child.ast.tostr() == "! Comment at end of loop on j => CodeBlock" + last_child.ast.tostr() + == "! Comment at end of loop on j => CodeBlock" ) else: assert not isinstance(last_child, CodeBlock) @@ -449,6 +454,7 @@ def test_comments_and_codeblocks(last_comments_as_codeblocks): end module test_mod """ + @pytest.mark.parametrize("last_comments_as_codeblocks", [True, False]) def test_write_comments(last_comments_as_codeblocks): """Test that the comments are written back to the code""" From 851a7f8456a684d513a4873ed45d94756b10c2d8 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 9 Dec 2024 18:14:03 +0100 Subject: [PATCH 11/20] #1247 Move commentable_mixin.py up one directory as it's used in both nodes and symbols. --- doc/developer_guide/psyir.rst | 2 +- src/psyclone/psyir/backend/visitor.py | 2 +- src/psyclone/psyir/{nodes => }/commentable_mixin.py | 0 src/psyclone/psyir/frontend/fparser2.py | 2 +- src/psyclone/psyir/nodes/container.py | 2 +- src/psyclone/psyir/nodes/routine.py | 2 +- src/psyclone/psyir/nodes/statement.py | 2 +- src/psyclone/psyir/symbols/symbol.py | 2 +- src/psyclone/tests/psyir/frontend/fparser2_comment_test.py | 2 +- 9 files changed, 8 insertions(+), 8 deletions(-) rename src/psyclone/psyir/{nodes => }/commentable_mixin.py (100%) diff --git a/doc/developer_guide/psyir.rst b/doc/developer_guide/psyir.rst index b5bc1ea2ba..8e2e629560 100644 --- a/doc/developer_guide/psyir.rst +++ b/doc/developer_guide/psyir.rst @@ -898,7 +898,7 @@ class, for example: .. code-block:: python - from psyclone.psyir.nodes.commentable_mixin import CommentableMixin + from psyclone.psyir.commentable_mixin import CommentableMixin class MyNode(Node, CommentableMixin): ''' Example node ''' diff --git a/src/psyclone/psyir/backend/visitor.py b/src/psyclone/psyir/backend/visitor.py index cb92956c36..b8db98037a 100644 --- a/src/psyclone/psyir/backend/visitor.py +++ b/src/psyclone/psyir/backend/visitor.py @@ -45,7 +45,7 @@ from psyclone.errors import PSycloneError from psyclone.psyir.nodes import Node, Schedule, Container -from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.commentable_mixin import CommentableMixin class VisitorError(PSycloneError): diff --git a/src/psyclone/psyir/nodes/commentable_mixin.py b/src/psyclone/psyir/commentable_mixin.py similarity index 100% rename from src/psyclone/psyir/nodes/commentable_mixin.py rename to src/psyclone/psyir/commentable_mixin.py diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index d22030ee76..df3e115e4c 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -60,7 +60,7 @@ Reference, Return, Routine, Schedule, StructureReference, UnaryOperation, WhileLoop) from psyclone.psyir.nodes.array_mixin import ArrayMixin -from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import ( ArgumentInterface, ArrayType, AutomaticInterface, CHARACTER_TYPE, CommonBlockInterface, ContainerSymbol, DataSymbol, DataTypeSymbol, diff --git a/src/psyclone/psyir/nodes/container.py b/src/psyclone/psyir/nodes/container.py index e35b706063..a2b909fc9b 100644 --- a/src/psyclone/psyir/nodes/container.py +++ b/src/psyclone/psyir/nodes/container.py @@ -44,7 +44,7 @@ from psyclone.psyir.symbols import (GenericInterfaceSymbol, RoutineSymbol, Symbol, SymbolTable) from psyclone.errors import GenerationError -from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.commentable_mixin import CommentableMixin class Container(ScopingNode, CommentableMixin): diff --git a/src/psyclone/psyir/nodes/routine.py b/src/psyclone/psyir/nodes/routine.py index d42256ef76..2f96da91aa 100644 --- a/src/psyclone/psyir/nodes/routine.py +++ b/src/psyclone/psyir/nodes/routine.py @@ -45,7 +45,7 @@ from psyclone.errors import GenerationError from psyclone.psyir.nodes.codeblock import CodeBlock -from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.nodes.node import Node from psyclone.psyir.nodes.schedule import Schedule from psyclone.psyir.symbols import DataSymbol, RoutineSymbol diff --git a/src/psyclone/psyir/nodes/statement.py b/src/psyclone/psyir/nodes/statement.py index 5057e157c5..d25f4ff13d 100644 --- a/src/psyclone/psyir/nodes/statement.py +++ b/src/psyclone/psyir/nodes/statement.py @@ -37,7 +37,7 @@ ''' This module contains the Statement abstract node implementation.''' from psyclone.psyir.nodes.node import Node -from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.commentable_mixin import CommentableMixin class Statement(Node, CommentableMixin): diff --git a/src/psyclone/psyir/symbols/symbol.py b/src/psyclone/psyir/symbols/symbol.py index efa566abc1..ce6850f6de 100644 --- a/src/psyclone/psyir/symbols/symbol.py +++ b/src/psyclone/psyir/symbols/symbol.py @@ -45,7 +45,7 @@ AutomaticInterface, SymbolInterface, ArgumentInterface, UnresolvedInterface, ImportInterface, UnknownInterface, CommonBlockInterface, DefaultModuleInterface, StaticInterface) -from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.commentable_mixin import CommentableMixin class SymbolError(PSycloneError): diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index b385f8a335..e7d2a1c8ed 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -50,7 +50,7 @@ Call, CodeBlock, ) -from psyclone.psyir.nodes.commentable_mixin import CommentableMixin +from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import DataTypeSymbol, StructureType from psyclone.psyir.backend.fortran import FortranWriter From d3aaba53275afcd4b293f7e3cf3d6dd2142f06cf Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 9 Dec 2024 18:31:58 +0100 Subject: [PATCH 12/20] #1247 Fix --- src/psyclone/psyir/frontend/fparser2.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index df3e115e4c..2a70100130 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -49,7 +49,7 @@ from fparser.common.readfortran import FortranStringReader from fparser.two import C99Preprocessor, Fortran2003, utils from fparser.two.parser import ParserFactory -from fparser.two.utils import walk, BlockBase, StmtBase +from fparser.two.utils import walk, BlockBase, StmtBase, Base from psyclone.configuration import Config from psyclone.errors import InternalError, GenerationError @@ -2952,9 +2952,9 @@ def process_nodes(self, parent, nodes): # If the fparser2 node has no span, try to build one # from the spans of the first and last children. elif (len(child.children) != 0 - and (child.children[0] is not None + and (isinstance(child.children[0], Base) and child.children[0].item is not None) - and (child.children[-1] is not None + and (isinstance(child.children[-1], Base) and child.children[-1].item is not None)): span = (child.children[0].item.span[0], child.children[-1].item.span[1]) From a0a27d6a166d89b991ddd0cbe09bca58d982a180 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Tue, 10 Dec 2024 14:52:28 +0100 Subject: [PATCH 13/20] #1247 More test coverage, move test file --- src/psyclone/psyir/frontend/fparser2.py | 115 +++--------------- .../psyir/backend/fortran_gen_decls_test.py | 14 +++ .../fortran_unsupported_declns_test.py | 10 ++ .../{nodes => }/commentable_mixin_test.py | 0 .../tests/psyir/frontend/fortran_test.py | 53 +++++++- .../frontend/fparser2_derived_type_test.py | 38 ++++++ .../tests/psyir/frontend/fparser2_test.py | 14 +++ .../tests/psyir/symbols/datatype_test.py | 11 ++ 8 files changed, 158 insertions(+), 97 deletions(-) rename src/psyclone/tests/psyir/{nodes => }/commentable_mixin_test.py (100%) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 2a70100130..5328db3c97 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -984,84 +984,6 @@ def __init__(self): self._last_node_parsed_and_span = None self._last_comments_as_codeblocks = False - @property - def last_symbol_parsed_and_span(self): - ''' - :returns: the last symbol parsed and the span of source code lines it - was found in. - :rtype: Union[None, Tuple[:py:class:`psyclone.psyir.symbols.Symbol`, - Tuple[int, int]]]''' - return self._last_symbol_parsed_and_span - - @last_symbol_parsed_and_span.setter - def last_symbol_parsed_and_span(self, value): - '''Setter for the last_symbol_parsed_and_span property. - - :param value: the last symbol parsed and the span of source code lines - it was found in. - :type value: Tuple[:py:class:`psyclone.psyir.symbols.Symbol`, - Tuple[int, int]] - - :raises TypeError: if the value is not of the right type. - ''' - if not isinstance(value, tuple) or len(value) != 2: - raise TypeError( - "The value of the last_symbol_parsed_and_span property must " - "be a 2-tuple.") - if not isinstance(value[0], Symbol): - raise TypeError( - "The first element of the last_symbol_parsed_and_span tuple " - "must be a Symbol.") - if not isinstance(value[1], tuple) or len(value[1]) != 2: - raise TypeError( - "The second element of the last_symbol_parsed_and_span tuple " - "must be a 2-tuple.") - if not all(isinstance(item, int) for item in value[1]): - raise TypeError( - "The second element of the last_symbol_parsed_and_span tuple " - "must contain two integers.") - - self._last_symbol_parsed_and_span = value - - @property - def last_node_parsed_and_span(self): - ''' - :returns: the last node parsed and the span of source code lines it - was found in. - :rtype: Union[None, Tuple[:py:class:`psyclone.psyir.nodes.Node`, - Tuple[int, int]]]''' - return self._last_node_parsed_and_span - - @last_node_parsed_and_span.setter - def last_node_parsed_and_span(self, value): - '''Setter for the last_node_parsed_and_span property. - - :param value: the last node parsed and the span of source code lines - it was found in. - :type value: Tuple[:py:class:`psyclone.psyclone.psyir.nodes.Node`, - Tuple[int, int]] - - :raises TypeError: if the value is not of the right type. - ''' - if not isinstance(value, tuple) or len(value) != 2: - raise TypeError( - "The value of the last_node_parsed_and_span property must " - "be a 2-tuple.") - if not isinstance(value[0], Node): - raise TypeError( - "The first element of the last_node_parsed_and_span tuple " - "must be a Node.") - if not isinstance(value[1], tuple) or len(value[1]) != 2: - raise TypeError( - "The second element of the last_node_parsed_and_span tuple " - "must be a 2-tuple.") - if not all(isinstance(item, int) for item in value[1]): - raise TypeError( - "The second element of the last_node_parsed_and_span tuple " - "must contain two integers.") - - self._last_node_parsed_and_span = value - @property def last_comments_as_codeblocks(self): ''' @@ -1085,8 +1007,8 @@ def last_comments_as_codeblocks(self, value): ''' if not isinstance(value, bool): raise TypeError( - "The value of the last_comments_as_codeblocks property must " - "be a boolean.") + f"The value of the last_comments_as_codeblocks property must " + f"be a boolean but found '{type(value).__name__}'.") self._last_comments_as_codeblocks = value @staticmethod @@ -2023,7 +1945,7 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, = self._comments_list_to_string(preceding_comments) preceding_comments = [] - self.last_symbol_parsed_and_span = (sym, decl.item.span) + self._last_symbol_parsed_and_span = (sym, decl.item.span) if init_expr: # In Fortran, an initialisation expression on a declaration of @@ -2152,7 +2074,7 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, continue if isinstance(child, Fortran2003.Comment): self.process_comment(child, preceding_comments, - self.last_symbol_parsed_and_span) + self._last_symbol_parsed_and_span) continue if isinstance(child, Fortran2003.Component_Part): for component in walk(child, @@ -2477,7 +2399,7 @@ def process_declarations(self, parent, nodes, arg_list, if isinstance(node, Fortran2003.Implicit_Part): for comment in walk(node, Fortran2003.Comment): self.process_comment(comment, preceding_comments, - self.last_symbol_parsed_and_span) + self._last_symbol_parsed_and_span) elif isinstance(node, Fortran2003.Derived_Type_Def): sym = self._process_derived_type_decln(parent, node, visibility_map, @@ -2485,7 +2407,7 @@ def process_declarations(self, parent, nodes, arg_list, preceding_comments = [] derived_type_span = (node.children[0].item.span[0], node.children[-1].item.span[1]) - self.last_symbol_parsed_and_span = (sym, derived_type_span) + self._last_symbol_parsed_and_span = (sym, derived_type_span) # INCLUDE statements are *not* part of the Fortran language and # can appear anywhere. Therefore we have to do a walk to make sure we @@ -2505,7 +2427,7 @@ def process_declarations(self, parent, nodes, arg_list, if isinstance(node, Fortran2003.Implicit_Part): for comment in walk(node, Fortran2003.Comment): self.process_comment(comment, preceding_comments, - self.last_symbol_parsed_and_span) + self._last_symbol_parsed_and_span) continue # Anything other than a PARAMETER statement or an # IMPLICIT NONE means we can't handle this code. @@ -2581,8 +2503,9 @@ def process_declarations(self, parent, nodes, arg_list, initial_value=init) new_symbol.preceding_comment \ = '\n'.join(preceding_comments) - self.last_symbol_parsed_and_span = (new_symbol, - node.item.span) + self._last_symbol_parsed_and_span\ + = (new_symbol, + node.item.span) parent.symbol_table.add(new_symbol) except KeyError as err: if len(orig_children) == 1: @@ -2918,7 +2841,7 @@ def process_nodes(self, parent, nodes): # it is an inline comment or store it for the next node. if isinstance(child, Fortran2003.Comment): self.process_comment(child, preceding_comments, - self.last_node_parsed_and_span) + self._last_node_parsed_and_span) continue try: psy_child = self._create_child(child, parent) @@ -2947,8 +2870,8 @@ def process_nodes(self, parent, nodes): preceding_comments = [] if isinstance(psy_child, CommentableMixin): if child.item is not None: - self.last_node_parsed_and_span = (psy_child, - child.item.span) + self._last_node_parsed_and_span = (psy_child, + child.item.span) # If the fparser2 node has no span, try to build one # from the spans of the first and last children. elif (len(child.children) != 0 @@ -2958,7 +2881,7 @@ def process_nodes(self, parent, nodes): and child.children[-1].item is not None)): span = (child.children[0].item.span[0], child.children[-1].item.span[1]) - self.last_node_parsed_and_span = (psy_child, span) + self._last_node_parsed_and_span = (psy_child, span) 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. @@ -3351,7 +3274,7 @@ def _do_construct_handler(self, node, parent): for child in node.content: if isinstance(child, Fortran2003.Comment) and not found_do_stmt: self.process_comment(child, preceding_comments, - self.last_node_parsed_and_span) + self._last_node_parsed_and_span) continue if isinstance(child, Fortran2003.Nonlabel_Do_Stmt): found_do_stmt = True @@ -3405,7 +3328,7 @@ def _if_construct_handler(self, node, parent): for child in node.content[:clause_indices[0]]: if isinstance(child, Fortran2003.Comment): self.process_comment(child, preceding_comments, - self.last_node_parsed_and_span) + self._last_node_parsed_and_span) # NOTE: The comments are added to the IfBlock node. # NOTE: Comments before the 'else[if]' statements are not handled. @@ -5382,7 +5305,7 @@ def _subroutine_handler(self, node, parent): for child in node.children: if isinstance(child, Fortran2003.Comment): self.process_comment(child, preceding_comments, - self.last_node_parsed_and_span) + self._last_node_parsed_and_span) continue if isinstance(child, (Fortran2003.Subroutine_Stmt, Fortran2003.Function_Stmt)): @@ -5446,9 +5369,9 @@ def _subroutine_handler(self, node, parent): for comment in walk(decl_list[-1], Fortran2003.Comment): if len(comment.tostr()) == 0: continue - if self.last_symbol_parsed_and_span is not None: + if self._last_symbol_parsed_and_span is not None: last_symbol, last_span \ - = self.last_symbol_parsed_and_span + = self._last_symbol_parsed_and_span if (last_span is not None and last_span[1] == comment.item.span[0]): last_symbol.inline_comment\ diff --git a/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py b/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py index 3961dd42ba..a8c7938f04 100644 --- a/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py @@ -298,6 +298,20 @@ def test_gen_decls_static_variables(fortran_writer): assert "parameter :: v1 = 1" in fortran_writer.gen_vardecl(sym) +def test_gen_decls_comments(fortran_writer): + '''Test that the gen_vardecl method adds comments to the Fortran code + when the symbol has a description. + + ''' + sym = DataSymbol("v1", datatype=INTEGER_TYPE) + sym.preceding_comment = "Preceding comment" + sym.inline_comment = "Inline comment" + result = fortran_writer.gen_vardecl(sym) + expected = ("! Preceding comment\n" + "integer :: v1 ! Inline comment") + assert expected in result + + @pytest.mark.parametrize("visibility", ["public", "private"]) def test_visibility_abstract_interface(fortran_reader, fortran_writer, visibility): diff --git a/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py b/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py index 2f88e8266a..4d56510f0c 100644 --- a/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py @@ -224,6 +224,16 @@ def test_fw_add_accessibility(): "end type var") +def test_fw_preceding_and_inline_comment(fortran_writer): + '''Test that comments are correctly added to the generated code''' + symbol = DataSymbol("var", UnsupportedFortranType("integer :: var")) + symbol.preceding_comment = "This is a preceding comment" + symbol.inline_comment = "This is an inline comment" + expected = ("! This is a preceding comment\n" + "integer :: var ! This is an inline comment") + assert expected in fortran_writer.gen_vardecl(symbol) + + def test_generating_unsupportedtype_routine_imports( fortran_reader, tmpdir, monkeypatch, fortran_writer): ''' Tests that generating UnsupportedType imported RoutineSymbols (if their diff --git a/src/psyclone/tests/psyir/nodes/commentable_mixin_test.py b/src/psyclone/tests/psyir/commentable_mixin_test.py similarity index 100% rename from src/psyclone/tests/psyir/nodes/commentable_mixin_test.py rename to src/psyclone/tests/psyir/commentable_mixin_test.py diff --git a/src/psyclone/tests/psyir/frontend/fortran_test.py b/src/psyclone/tests/psyir/frontend/fortran_test.py index 2ffe3b983d..5f9c56f165 100644 --- a/src/psyclone/tests/psyir/frontend/fortran_test.py +++ b/src/psyclone/tests/psyir/frontend/fortran_test.py @@ -43,7 +43,8 @@ from psyclone.psyir.frontend.fparser2 import Fparser2Reader from psyclone.psyir.nodes import ( Routine, FileContainer, UnaryOperation, BinaryOperation, Literal, - Assignment, CodeBlock, IntrinsicCall) + Assignment, CodeBlock, IntrinsicCall, Loop) +from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import ( SymbolTable, DataSymbol, ScalarType, UnresolvedType) @@ -69,6 +70,20 @@ end ''' +CODE_WITH_COMMENTS_AND_DIRECTIVES = ''' +subroutine my_sub + integer :: a, b + + ! Comment on do loop + !$omp parallel do + do a = 1, 10 + ! Comment on assignment + b = a + end do + !$omp end parallel do +end subroutine my_sub +''' + def test_fortran_reader_constructor(): ''' Test that the constructor initialises the _parser and _processor @@ -224,3 +239,39 @@ def test_fortran_psyir_from_file(fortran_reader, tmpdir_factory): with pytest.raises(IOError) as err: fortran_reader.psyir_from_file(filename) assert "No such file or directory: '" + str(filename) in str(err.value) + + # Check that directives and comments are ignored by default + filename = str(tmpdir_factory.mktemp('frontend_test').join("comments.f90")) + with open(filename, "w", encoding='utf-8') as wfile: + wfile.write(CODE_WITH_COMMENTS_AND_DIRECTIVES) + file_container = fortran_reader.psyir_from_file(filename) + assert isinstance(file_container, FileContainer) + for node in file_container.walk(CommentableMixin): + assert node.preceding_comment == "" + assert node.inline_comment == "" + + # Check that comments can be preserved, and that directives are still + # ignored by default + file_container = fortran_reader.psyir_from_file( + filename, ignore_comments=False) + assert isinstance(file_container, FileContainer) + for node in file_container.walk(CommentableMixin): + if isinstance(node, Loop): + assert node.preceding_comment == "Comment on do loop" + elif isinstance(node, Assignment): + assert node.preceding_comment == "Comment on assignment" + else: + assert node.preceding_comment == "" + + # Check that directives can be preserved + file_container = fortran_reader.psyir_from_file( + filename, ignore_comments=False, ignore_directives=False) + assert isinstance(file_container, FileContainer) + 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: + assert node.preceding_comment == "" diff --git a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py index 7262ad2aab..673ac283ba 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py @@ -239,6 +239,44 @@ def test_existing_symbol_derived_type_def(f2008_parser): assert isinstance(typ.interface, ImportInterface) +def test_preceding_comments(f2008_parser): + ''' Check that the frontend correctly handles comments that precede + a derived type definition. ''' + fake_parent = KernelSchedule.create("dummy_schedule") + processor = Fparser2Reader() + comment = Fortran2003.Comment( + FortranStringReader("! This is a comment\n", + ignore_comments=False)) + other = Fortran2003.Comment( + FortranStringReader("! This is another comment\n", + ignore_comments=False)) + fparser2spec = f2008_parser(FortranStringReader("subroutine my_sub\n" + "type :: my_type\n" + " integer :: flag\n" + "end type my_type\n" + "end subroutine my_sub\n")) + type_decl = walk(fparser2spec, types=Fortran2003.Derived_Type_Def) + typ = processor._process_derived_type_decln(fake_parent, type_decl[0], + dict(), + preceding_comments=[comment, + other]) + assert typ.preceding_comment == ("This is a comment\n" + "This is another comment") + + fake_parent = KernelSchedule.create("dummy_schedule") + processor = Fparser2Reader() + fparser2spec = f2008_parser(FortranStringReader("subroutine my_sub\n" + "type :: my_type\n" + " integer :: flag\n" + "end type my_type\n" + "end subroutine my_sub\n")) + type_decl = walk(fparser2spec, types=Fortran2003.Derived_Type_Def) + typ = processor._process_derived_type_decln(fake_parent, type_decl[0], + dict(), + preceding_comments=None) + assert typ.preceding_comment == "" + + @pytest.mark.usefixtures("f2008_parser") @pytest.mark.parametrize("use_stmt", ["use grid_mod, only: grid_type", "use grid_mod, only: GRID_TYPE", diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py index f102dd7083..1e38775983 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py @@ -151,6 +151,20 @@ def test_get_arg_names(parser): # Class Fparser2Reader +def test_last_comments_as_codeblocks(): + '''Test that the last_comments_as_codeblocks property and setter works as + expected.''' + processor = Fparser2Reader() + # Default value + assert not processor.last_comments_as_codeblocks + # Setter expects a boolean + with pytest.raises(TypeError) as error: + processor.last_comments_as_codeblocks = "false" + assert ("The value of the last_comments_as_codeblocks property must " + "be a boolean but found 'str'." in str(error.value)) + processor.last_comments_as_codeblocks = True + assert processor.last_comments_as_codeblocks + def test_get_routine_schedules_wrong_module(parser): '''Test that get_routine_schedules() raises the expected errors if there diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index ab678137bb..32fc56c411 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -920,6 +920,17 @@ def test_structure_type(): assert ("The initial value of a component of a StructureType must be " "None or an instance of 'DataNode', but got 'str'." in str(err.value)) + with pytest.raises(TypeError) as err: + stype.add("hello", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None, + preceding_comment=None) + assert ("The preceding_comment of a component of a StructureType " + "must be a 'str' but got 'NoneType'" in str(err.value)) + with pytest.raises(TypeError) as err: + stype.add("hello", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None, + inline_comment=None) + assert ("The inline_comment of a component of a StructureType " + "must be a 'str' but got 'NoneType'" in str(err.value)) + with pytest.raises(KeyError): stype.lookup("missing") # Cannot have a recursive type definition From 6fb625931f231ca8a5e78cab329eeb0834f689f4 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Tue, 10 Dec 2024 15:17:23 +0100 Subject: [PATCH 14/20] #1247 More codecov? Fix docstring formatting. --- src/psyclone/psyir/frontend/fparser2.py | 10 +++++----- .../psyir/backend/fortran_unsupported_declns_test.py | 8 ++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 5328db3c97..361af8c2d9 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -5660,11 +5660,11 @@ def process_comment(self, comment, preceding_comments, :type preceding_comments: List[:py:class:`fparser.two.utils.Comment`] :param last_psyir_and_span: Tuple containing the last PSyIR object and its span. - :type last_psyir_and_span: Tuple[ - Union[ - :py:class:`psyclone.psyir.nodes.Node`, - :py:class:`psyclone.psyir.symbols.Symbol` - ], + :type last_psyir_and_span: Tuple[\ + Union[\ + :py:class:`psyclone.psyir.nodes.Node`,\ + :py:class:`psyclone.psyir.symbols.Symbol`\ + ],\ Tuple[int, int]] ''' diff --git a/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py b/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py index 4d56510f0c..2fa054320a 100644 --- a/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py @@ -231,8 +231,16 @@ def test_fw_preceding_and_inline_comment(fortran_writer): symbol.inline_comment = "This is an inline comment" expected = ("! This is a preceding comment\n" "integer :: var ! This is an inline comment") + assert (not isinstance(symbol, RoutineSymbol) + and not symbol.name.startswith("_PSYCLONE_INTERNAL")) assert expected in fortran_writer.gen_vardecl(symbol) + # include_visibility=True case + expected = ("! This is a preceding comment\n" + "integer, public :: var ! This is an inline comment") + assert expected in fortran_writer.gen_vardecl(symbol, + include_visibility=True) + def test_generating_unsupportedtype_routine_imports( fortran_reader, tmpdir, monkeypatch, fortran_writer): From fe4986c324a5fd620cc265ede9634d986ceec2b5 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 16 Dec 2024 19:35:11 +0100 Subject: [PATCH 15/20] #1247 Some edits wrt Sergi's review. Not finished. --- src/psyclone/psyir/backend/fortran.py | 10 +- src/psyclone/psyir/frontend/fparser2.py | 110 ++++++------------ src/psyclone/psyir/symbols/symbol.py | 2 - .../psyir/backend/fortran_gen_decls_test.py | 17 ++- .../fortran_unsupported_declns_test.py | 2 - .../frontend/fparser2_derived_type_test.py | 39 +++---- 6 files changed, 73 insertions(+), 107 deletions(-) diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index b1d05135e9..e2bd7008cb 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -559,14 +559,8 @@ def gen_vardecl(self, symbol, include_visibility=False): # blocks appearing in SAVE statements. decln = add_accessibility_to_unsupported_declaration( symbol) - result += f"{self._nindent}{decln}" - if symbol.inline_comment != "": - result += (f" {self._COMMENT_PREFIX}" - f"{symbol.inline_comment}") - result += "\n" - return result - - decln = symbol.datatype.declaration + else: + decln = symbol.datatype.declaration result += f"{self._nindent}{decln}" if symbol.inline_comment != "": result += (f" {self._COMMENT_PREFIX}" diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 361af8c2d9..f3e400e013 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -980,8 +980,7 @@ def __init__(self): Fortran2003.Program: self._program_handler, } # Used to attach inline comments to the PSyIR symbols and nodes - self._last_symbol_parsed_and_span = None - self._last_node_parsed_and_span = None + self._last_psyir_parsed_and_span = None self._last_comments_as_codeblocks = False @property @@ -1657,7 +1656,7 @@ def _process_type_spec(self, parent, type_spec): return base_type, precision def _process_decln(self, scope, symbol_table, decl, visibility_map=None, - statics_list=(), preceding_comments=None): + statics_list=()): ''' Process the supplied fparser2 parse tree for a declaration. For each entity that is declared, a symbol is added to the supplied symbol @@ -1677,12 +1676,6 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, appearing in a SAVE statement). If all symbols are static then this contains the single entry "*". :type statics_list: Iterable[str] - :param preceding_comments: the comments that preceded this - declaration, as a list of fparser2 Comments. - :type preceding_comments: Optional[ - List[ - :py:class:`fparser.two.Fortran2003.Comment` - ]] :raises NotImplementedError: if an unsupported attribute is found. :raises NotImplementedError: if an unsupported intent attribute is @@ -1702,10 +1695,11 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, :raises GenerationError: if a set of incompatible Fortran attributes are found in a symbol declaration. + :returns: the newly created symbol. + :rtype: :py:class:`psyclone.psyir.symbols.DataSymbol` + ''' # pylint: disable=too-many-arguments - if preceding_comments is None: - preceding_comments = [] (type_spec, attr_specs, entities) = decl.items @@ -1940,12 +1934,7 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, f"'{sym_name}'.") from error symbol_table.add(sym, tag=tag) - # Add any preceding comments to the symbol - sym.preceding_comment\ - = self._comments_list_to_string(preceding_comments) - preceding_comments = [] - - self._last_symbol_parsed_and_span = (sym, decl.item.span) + self._last_psyir_parsed_and_span = (sym, decl.item.span) if init_expr: # In Fortran, an initialisation expression on a declaration of @@ -1956,8 +1945,9 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, else: sym.interface = this_interface - def _process_derived_type_decln(self, parent, decl, visibility_map, - preceding_comments=None): + return sym + + def _process_derived_type_decln(self, parent, decl, visibility_map): ''' Process the supplied fparser2 parse tree for a derived-type declaration. A DataTypeSymbol representing the derived-type is added @@ -1971,12 +1961,6 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, those symbols listed in an accessibility statement). :type visibility_map: dict[str, :py:class:`psyclone.psyir.symbols.Symbol.Visibility`] - :param preceding_comments: the comments that preceded this - declaration, as a list of fparser2 Comments. - :type preceding_comments: Optional[ - List[ - :py:class:`fparser.two.Fortran2003.Comment` - ]] :raises SymbolError: if a Symbol already exists with the same name as the derived type being defined and it is not a DataTypeSymbol @@ -1986,9 +1970,6 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, :rtype: :py:class:`psyclone.psyir.symbols.DataTypeSymbol` ''' - if preceding_comments is None: - preceding_comments = [] - name = str(walk(decl.children[0], Fortran2003.Type_Name)[0]).lower() # Create a new StructureType for this derived type dtype = StructureType() @@ -2039,9 +2020,6 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, tsymbol = DataTypeSymbol(name, dtype, visibility=dtype_symbol_vis) parent.symbol_table.add(tsymbol) - tsymbol.preceding_comment\ - = self._comments_list_to_string(preceding_comments) - # Populate this StructureType by processing the components of # the derived type try: @@ -2073,15 +2051,15 @@ def _process_derived_type_decln(self, parent, decl, visibility_map, if isinstance(child, Fortran2003.Derived_Type_Stmt): continue if isinstance(child, Fortran2003.Comment): - self.process_comment(child, preceding_comments, - self._last_symbol_parsed_and_span) + self.process_comment(child, preceding_comments) continue if isinstance(child, Fortran2003.Component_Part): for component in walk(child, Fortran2003.Data_Component_Def_Stmt): - self._process_decln( - parent, local_table, component, - preceding_comments=preceding_comments) + csym = self._process_decln(parent, local_table, + component) + csym.preceding_comment = self._comments_list_to_string( + preceding_comments) preceding_comments = [] # Convert from Symbols to StructureType components. for symbol in local_table.symbols: @@ -2398,16 +2376,16 @@ def process_declarations(self, parent, nodes, arg_list, for node in nodes: if isinstance(node, Fortran2003.Implicit_Part): for comment in walk(node, Fortran2003.Comment): - self.process_comment(comment, preceding_comments, - self._last_symbol_parsed_and_span) + self.process_comment(comment, preceding_comments) elif isinstance(node, Fortran2003.Derived_Type_Def): sym = self._process_derived_type_decln(parent, node, - visibility_map, - preceding_comments) + visibility_map) + sym.preceding_comment = \ + self._comments_list_to_string(preceding_comments) preceding_comments = [] derived_type_span = (node.children[0].item.span[0], node.children[-1].item.span[1]) - self._last_symbol_parsed_and_span = (sym, derived_type_span) + self._last_psyir_parsed_and_span = (sym, derived_type_span) # INCLUDE statements are *not* part of the Fortran language and # can appear anywhere. Therefore we have to do a walk to make sure we @@ -2426,8 +2404,7 @@ def process_declarations(self, parent, nodes, arg_list, if isinstance(node, Fortran2003.Implicit_Part): for comment in walk(node, Fortran2003.Comment): - self.process_comment(comment, preceding_comments, - self._last_symbol_parsed_and_span) + self.process_comment(comment, preceding_comments) continue # Anything other than a PARAMETER statement or an # IMPLICIT NONE means we can't handle this code. @@ -2450,9 +2427,11 @@ def process_declarations(self, parent, nodes, arg_list, elif isinstance(node, Fortran2003.Type_Declaration_Stmt): try: - self._process_decln(parent, parent.symbol_table, node, - visibility_map, statics_list, - preceding_comments) + tsym = self._process_decln(parent, parent.symbol_table, + node, visibility_map, + statics_list) + tsym.preceding_comment = self._comments_list_to_string( + preceding_comments) preceding_comments = [] except NotImplementedError: # Found an unsupported variable declaration. Create a @@ -2503,7 +2482,7 @@ def process_declarations(self, parent, nodes, arg_list, initial_value=init) new_symbol.preceding_comment \ = '\n'.join(preceding_comments) - self._last_symbol_parsed_and_span\ + self._last_psyir_parsed_and_span\ = (new_symbol, node.item.span) parent.symbol_table.add(new_symbol) @@ -2840,8 +2819,7 @@ def process_nodes(self, parent, nodes): # If the child is a comment, attach it to the preceding node if # it is an inline comment or store it for the next node. if isinstance(child, Fortran2003.Comment): - self.process_comment(child, preceding_comments, - self._last_node_parsed_and_span) + self.process_comment(child, preceding_comments) continue try: psy_child = self._create_child(child, parent) @@ -2870,7 +2848,7 @@ def process_nodes(self, parent, nodes): preceding_comments = [] if isinstance(psy_child, CommentableMixin): if child.item is not None: - self._last_node_parsed_and_span = (psy_child, + self._last_psyir_parsed_and_span = (psy_child, child.item.span) # If the fparser2 node has no span, try to build one # from the spans of the first and last children. @@ -2881,7 +2859,7 @@ def process_nodes(self, parent, nodes): and child.children[-1].item is not None)): span = (child.children[0].item.span[0], child.children[-1].item.span[1]) - self._last_node_parsed_and_span = (psy_child, span) + self._last_psyir_parsed_and_span = (psy_child, span) 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. @@ -3273,8 +3251,7 @@ def _do_construct_handler(self, node, parent): found_do_stmt = False for child in node.content: if isinstance(child, Fortran2003.Comment) and not found_do_stmt: - self.process_comment(child, preceding_comments, - self._last_node_parsed_and_span) + self.process_comment(child, preceding_comments) continue if isinstance(child, Fortran2003.Nonlabel_Do_Stmt): found_do_stmt = True @@ -3327,8 +3304,7 @@ def _if_construct_handler(self, node, parent): preceding_comments = [] for child in node.content[:clause_indices[0]]: if isinstance(child, Fortran2003.Comment): - self.process_comment(child, preceding_comments, - self._last_node_parsed_and_span) + self.process_comment(child, preceding_comments) # NOTE: The comments are added to the IfBlock node. # NOTE: Comments before the 'else[if]' statements are not handled. @@ -5304,8 +5280,7 @@ def _subroutine_handler(self, node, parent): preceding_comments = [] for child in node.children: if isinstance(child, Fortran2003.Comment): - self.process_comment(child, preceding_comments, - self._last_node_parsed_and_span) + self.process_comment(child, preceding_comments) continue if isinstance(child, (Fortran2003.Subroutine_Stmt, Fortran2003.Function_Stmt)): @@ -5369,9 +5344,9 @@ def _subroutine_handler(self, node, parent): for comment in walk(decl_list[-1], Fortran2003.Comment): if len(comment.tostr()) == 0: continue - if self._last_symbol_parsed_and_span is not None: + if self._last_psyir_parsed_and_span is not None: last_symbol, last_span \ - = self._last_symbol_parsed_and_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\ @@ -5647,8 +5622,7 @@ def _comments_list_to_string(self, comments): return '\n'.join([self._comment_to_string(comment) for comment in comments]) - def process_comment(self, comment, preceding_comments, - last_psyir_and_span): + def process_comment(self, comment, preceding_comments): ''' Process a comment and attach it to the last PSyIR object (Symbol or Node) if it is an inline comment. Otherwise append it to the @@ -5658,23 +5632,15 @@ def process_comment(self, comment, preceding_comments, :type comment: :py:class:`fparser.two.utils.Comment` :param preceding_comments: List of comments that precede the next node. :type preceding_comments: List[:py:class:`fparser.two.utils.Comment`] - :param last_psyir_and_span: Tuple containing the last PSyIR object and - its span. - :type last_psyir_and_span: Tuple[\ - Union[\ - :py:class:`psyclone.psyir.nodes.Node`,\ - :py:class:`psyclone.psyir.symbols.Symbol`\ - ],\ - Tuple[int, int]] ''' if len(comment.tostr()) == 0: return - if last_psyir_and_span is not None: - last_sym, last_span = last_psyir_and_span + if self._last_psyir_parsed_and_span is not None: + last_psyir, last_span = self._last_psyir_parsed_and_span if (last_span[1] is not None and last_span[1] == comment.item.span[0]): - last_sym.inline_comment = self._comment_to_string(comment) + last_psyir.inline_comment = self._comment_to_string(comment) return preceding_comments.append(comment) diff --git a/src/psyclone/psyir/symbols/symbol.py b/src/psyclone/psyir/symbols/symbol.py index ce6850f6de..241daaa599 100644 --- a/src/psyclone/psyir/symbols/symbol.py +++ b/src/psyclone/psyir/symbols/symbol.py @@ -167,8 +167,6 @@ def copy_properties(self, symbol_in): raise TypeError(f"Argument should be of type 'Symbol' but " f"found '{type(symbol_in).__name__}'.") self._interface = symbol_in.interface - self.preceding_comment = symbol_in.preceding_comment - self.inline_comment = symbol_in.inline_comment def specialise(self, subclass, **kwargs): '''Specialise this symbol so that it becomes an instance of the class diff --git a/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py b/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py index a8c7938f04..53f3b9f45d 100644 --- a/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py @@ -303,12 +303,25 @@ def test_gen_decls_comments(fortran_writer): when the symbol has a description. ''' - sym = DataSymbol("v1", datatype=INTEGER_TYPE) + sym = DataSymbol("v1", datatype=INTEGER_TYPE, + initial_value=Literal("1", INTEGER_TYPE), + is_constant=True) sym.preceding_comment = "Preceding comment" sym.inline_comment = "Inline comment" result = fortran_writer.gen_vardecl(sym) expected = ("! Preceding comment\n" - "integer :: v1 ! Inline comment") + "integer, parameter :: v1 = 1 ! Inline comment") + assert expected in result + + sym2 = DataSymbol("v2", datatype=INTEGER_TYPE, + initial_value=Literal("2", INTEGER_TYPE), + is_constant=True) + sym2.preceding_comment = "Preceding comment\nwith newline" + sym2.inline_comment = "Inline comment" + result = fortran_writer.gen_vardecl(sym2) + expected = ("! Preceding comment\n" + "! with newline\n" + "integer, parameter :: v2 = 2 ! Inline comment") assert expected in result diff --git a/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py b/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py index 2fa054320a..4604382b6c 100644 --- a/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_unsupported_declns_test.py @@ -231,8 +231,6 @@ def test_fw_preceding_and_inline_comment(fortran_writer): symbol.inline_comment = "This is an inline comment" expected = ("! This is a preceding comment\n" "integer :: var ! This is an inline comment") - assert (not isinstance(symbol, RoutineSymbol) - and not symbol.name.startswith("_PSYCLONE_INTERNAL")) assert expected in fortran_writer.gen_vardecl(symbol) # include_visibility=True case diff --git a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py index 673ac283ba..db15ada8e8 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py @@ -48,7 +48,7 @@ from psyclone.psyir.nodes import ( KernelSchedule, CodeBlock, Assignment, ArrayOfStructuresReference, StructureReference, Member, StructureMember, ArrayOfStructuresMember, - ArrayMember, Literal, Reference, Range, IntrinsicCall) + ArrayMember, Literal, Reference, Range, IntrinsicCall, Container) from psyclone.psyir.symbols import ( SymbolError, UnresolvedType, StructureType, DataTypeSymbol, ScalarType, RoutineSymbol, Symbol, ArrayType, UnsupportedFortranType, DataSymbol, @@ -242,39 +242,36 @@ def test_existing_symbol_derived_type_def(f2008_parser): def test_preceding_comments(f2008_parser): ''' Check that the frontend correctly handles comments that precede a derived type definition. ''' - fake_parent = KernelSchedule.create("dummy_schedule") + fake_parent = Container("dummy_container") processor = Fparser2Reader() - comment = Fortran2003.Comment( - FortranStringReader("! This is a comment\n", - ignore_comments=False)) - other = Fortran2003.Comment( - FortranStringReader("! This is another comment\n", - ignore_comments=False)) fparser2spec = f2008_parser(FortranStringReader("subroutine my_sub\n" + "! This is a comment\n" + "! This is another comment\n" "type :: my_type\n" " integer :: flag\n" - "end type my_type\n" - "end subroutine my_sub\n")) - type_decl = walk(fparser2spec, types=Fortran2003.Derived_Type_Def) - typ = processor._process_derived_type_decln(fake_parent, type_decl[0], - dict(), - preceding_comments=[comment, - other]) + "end type my_type ! Inline comment\n" + "end subroutine my_sub\n", + ignore_comments=False)) + sub_decl = walk(fparser2spec, types=Fortran2003.Subroutine_Subprogram) + sub = processor._subroutine_handler(sub_decl[0], fake_parent) + typ = sub.symbol_table.lookup("my_type") assert typ.preceding_comment == ("This is a comment\n" "This is another comment") + assert typ.inline_comment == "Inline comment" - fake_parent = KernelSchedule.create("dummy_schedule") + fake_parent = Container("dummy_container") processor = Fparser2Reader() fparser2spec = f2008_parser(FortranStringReader("subroutine my_sub\n" "type :: my_type\n" " integer :: flag\n" "end type my_type\n" - "end subroutine my_sub\n")) - type_decl = walk(fparser2spec, types=Fortran2003.Derived_Type_Def) - typ = processor._process_derived_type_decln(fake_parent, type_decl[0], - dict(), - preceding_comments=None) + "end subroutine my_sub\n", + ignore_comments=False)) + sub_decl = walk(fparser2spec, types=Fortran2003.Subroutine_Subprogram) + sub = processor._subroutine_handler(sub_decl[0], fake_parent) + typ = sub.symbol_table.lookup("my_type") assert typ.preceding_comment == "" + assert typ.inline_comment == "" @pytest.mark.usefixtures("f2008_parser") From 8bf76881f7f4b6f5f123d21eff04e12ce82a1822 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Tue, 17 Dec 2024 14:07:20 +0100 Subject: [PATCH 16/20] #1247 More edits wrt Sergi's review. --- src/psyclone/psyir/frontend/fortran.py | 97 ++++++------------- src/psyclone/psyir/frontend/fparser2.py | 67 ++++++------- src/psyclone/psyir/symbols/datatypes.py | 19 +++- .../tests/psyir/frontend/fortran_test.py | 14 +-- .../psyir/frontend/fparser2_comment_test.py | 39 +++----- .../frontend/fparser2_derived_type_test.py | 17 ++-- .../tests/psyir/frontend/fparser2_test.py | 14 --- .../tests/psyir/symbols/datatype_test.py | 33 +++++++ 8 files changed, 138 insertions(+), 162 deletions(-) diff --git a/src/psyclone/psyir/frontend/fortran.py b/src/psyclone/psyir/frontend/fortran.py index ca4b9c9004..e156b381e4 100644 --- a/src/psyclone/psyir/frontend/fortran.py +++ b/src/psyclone/psyir/frontend/fortran.py @@ -44,7 +44,7 @@ from fparser.two import Fortran2003, pattern_tools from fparser.two.parser import ParserFactory from fparser.two.symbol_table import SYMBOL_TABLES -from fparser.two.utils import NoMatchError, walk +from fparser.two.utils import NoMatchError from psyclone.configuration import Config from psyclone.psyir.frontend.fparser2 import Fparser2Reader from psyclone.psyir.nodes import Schedule, Assignment, Routine @@ -55,14 +55,32 @@ class FortranReader(): ''' PSyIR Fortran frontend. This frontend translates Fortran from a string or a file into PSyIR using the fparser2 utilities. + :param free_form: If parsing free-form code or not (default True). + :param ignore_comments: If comments should be ignored or not + (default True). + :param ignore_directives: If directives should be ignored or not + (default True). Only has an effect + if ignore_comments is False. + :param last_comments_as_codeblocks: If the last comments in the + a given block (e.g. subroutine, + do, if-then body, etc.) should + be kept as code blocks or lost + (default False). + Only has an effect if ignore_comments + is False. ''' # Save parser object across instances to reduce the initialisation time _parser = None - def __init__(self): + def __init__(self, free_form: bool = True, ignore_comments: bool = True, + ignore_directives: bool = True, + last_comments_as_codeblocks: bool = False): if not self._parser: self._parser = ParserFactory().create(std="f2008") - self._processor = Fparser2Reader() + self._free_form = free_form + self._ignore_comments = ignore_comments + self._processor = Fparser2Reader(ignore_directives, + last_comments_as_codeblocks) SYMBOL_TABLES.clear() @staticmethod @@ -85,26 +103,11 @@ def validate_name(name: str): raise ValueError( f"Invalid Fortran name '{name}' found.") - def psyir_from_source(self, source_code: str, free_form: bool = True, - ignore_comments: bool = True, - ignore_directives: bool = True, - last_comments_as_codeblocks: bool = False): + def psyir_from_source(self, source_code: str): ''' Generate the PSyIR tree representing the given Fortran source code. - :param source_code: text representation of the code to be parsed. - :param free_form: If parsing free-form code or not (default True). - :param ignore_comments: If comments should be ignored or not - (default True). - :param ignore_directives: If directives should be ignored or not - (default True). Only has an effect - if ignore_comments is False. - :param last_comments_as_codeblocks: If the last comments in the - a given block (e.g. subroutine, - do, if-then body, etc.) should - be kept as code blocks or lost - (default False). - Only has an effect if - ignore_comments is False. + :param str source_code: text representation of the code to be parsed. + :returns: PSyIR representing the provided Fortran source code. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -112,16 +115,11 @@ def psyir_from_source(self, source_code: str, free_form: bool = True, SYMBOL_TABLES.clear() string_reader = FortranStringReader( source_code, include_dirs=Config.get().include_paths, - ignore_comments=ignore_comments) + ignore_comments=self._ignore_comments) # Set reader to free format. - string_reader.set_format(FortranFormat(free_form, False)) + string_reader.set_format(FortranFormat(self._free_form, False)) parse_tree = self._parser(string_reader) - if (not ignore_comments) and ignore_directives: - self._strip_directives(parse_tree) - self._processor.last_comments_as_codeblocks\ - = last_comments_as_codeblocks - psyir = self._processor.generate_psyir(parse_tree) return psyir @@ -215,34 +213,12 @@ def psyir_from_statement(self, source_code: str, self._processor.process_nodes(fake_parent, exec_part.children) return fake_parent[0].detach() - def psyir_from_file(self, file_path, free_form=True, - ignore_comments: bool = True, - ignore_directives: bool = True, - last_comments_as_codeblocks: bool = False): + def psyir_from_file(self, file_path): ''' Generate the PSyIR tree representing the given Fortran file. :param file_path: path of the file to be read and parsed. :type file_path: str or any Python Path format. - :param free_form: If parsing free-form code or not (default True). - :type free_form: bool - - :param ignore_comments: If comments should be ignored or not - (default True). - :type ignore_comments: bool - :param ignore_directives: If directives should be ignored or not - (default True). Only has an effect - if ignore_comments is False. - :type ignore_directives: bool - :param last_comments_as_codeblocks: If the last comments in the - a given block (e.g. subroutine, - do, if-then body, etc.) should - be kept as code blocks or lost - (default False). - Only has an effect if - ignore_comments is False. - :type last_comments_as_codeblocks: bool - :returns: PSyIR representing the provided Fortran file. :rtype: :py:class:`psyclone.psyir.nodes.Node` @@ -258,29 +234,14 @@ def psyir_from_file(self, file_path, free_form=True, # fparser to keep the filename information in the tree reader = FortranFileReader(file_path, include_dirs=Config.get().include_paths, - ignore_comments=ignore_comments) - reader.set_format(FortranFormat(free_form, False)) + ignore_comments=self._ignore_comments) + reader.set_format(FortranFormat(self._free_form, False)) parse_tree = self._parser(reader) _, filename = os.path.split(file_path) - if (not ignore_comments) and ignore_directives: - self._strip_directives(parse_tree) - self._processor.last_comments_as_codeblocks\ - = last_comments_as_codeblocks - psyir = self._processor.generate_psyir(parse_tree, filename) return psyir - def _strip_directives(self, node): - '''Remove all '!$' directives from the comments in the fparser tree. - - :param node: the fparser node to process. - ''' - for comment in walk(node, Fortran2003.Comment): - if comment.tostr().startswith("!$"): - comment.items[0] = "" - return node - # For Sphinx AutoAPI documentation generation __all__ = ['FortranReader'] diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index f3e400e013..0da78c087a 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -851,6 +851,17 @@ class Fparser2Reader(): ''' Class to encapsulate the functionality for processing the fparser2 AST and convert the nodes to PSyIR. + + :param ignore_directives: If directives should be ignored or not + (default True). Only has an effect + if comments were not ignored. + :param last_comments_as_codeblocks: If the last comments in the + a given block (e.g. subroutine, + do, if-then body, etc.) should + be kept as code blocks or lost + (default False). + Only has an effect if comments were + not ignored. ''' unary_operators = OrderedDict([ @@ -935,7 +946,8 @@ class SelectTypeInfo: num_clauses: int = -1 default_idx: int = -1 - def __init__(self): + def __init__(self, ignore_directives: bool = True, + last_comments_as_codeblocks: bool = False): # Map of fparser2 node types to handlers (which are class methods) self.handlers = { Fortran2003.Allocate_Stmt: self._allocate_handler, @@ -981,34 +993,10 @@ def __init__(self): } # Used to attach inline comments to the PSyIR symbols and nodes self._last_psyir_parsed_and_span = None - self._last_comments_as_codeblocks = False - - @property - def last_comments_as_codeblocks(self): - ''' - :returns: whether the last comments in a given block (e.g. subroutine, - do, if-then body, etc.) should be kept as code blocks or lost - (default False). - Only has an effect if the fparser2 parse tree has comments. - ''' - return self._last_comments_as_codeblocks - - @last_comments_as_codeblocks.setter - def last_comments_as_codeblocks(self, value): - '''Setter for the last_comments_as_codeblocks property. - - :param value: whether the last comments in a given block (e.g. - subroutine, do, if-then body, etc.) should be kept as - code blocks or lost. - Only has an effect if the fparser2 parse tree has - comments. - :type value: bool - ''' - if not isinstance(value, bool): - raise TypeError( - f"The value of the last_comments_as_codeblocks property must " - f"be a boolean but found '{type(value).__name__}'.") - self._last_comments_as_codeblocks = value + # Whether to ignore directives when processing the fparser2 AST + self._ignore_directives = ignore_directives + # Whether to keep the last comments in a given block as code blocks + self._last_comments_as_codeblocks = last_comments_as_codeblocks @staticmethod def nodes_to_code_block(parent, fp2_nodes, message=None): @@ -2048,8 +2036,6 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): preceding_comments = [] for child in decl.children: - if isinstance(child, Fortran2003.Derived_Type_Stmt): - continue if isinstance(child, Fortran2003.Comment): self.process_comment(child, preceding_comments) continue @@ -2057,7 +2043,7 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): for component in walk(child, Fortran2003.Data_Component_Def_Stmt): csym = self._process_decln(parent, local_table, - component) + component) csym.preceding_comment = self._comments_list_to_string( preceding_comments) preceding_comments = [] @@ -2428,8 +2414,8 @@ def process_declarations(self, parent, nodes, arg_list, elif isinstance(node, Fortran2003.Type_Declaration_Stmt): try: tsym = self._process_decln(parent, parent.symbol_table, - node, visibility_map, - statics_list) + node, visibility_map, + statics_list) tsym.preceding_comment = self._comments_list_to_string( preceding_comments) preceding_comments = [] @@ -2849,7 +2835,8 @@ def process_nodes(self, parent, nodes): if isinstance(psy_child, CommentableMixin): if child.item is not None: self._last_psyir_parsed_and_span = (psy_child, - child.item.span) + child.item.span + ) # If the fparser2 node has no span, try to build one # from the spans of the first and last children. elif (len(child.children) != 0 @@ -2859,14 +2846,16 @@ def process_nodes(self, parent, nodes): and child.children[-1].item is not None)): span = (child.children[0].item.span[0], child.children[-1].item.span[1]) - self._last_psyir_parsed_and_span = (psy_child, span) + self._last_psyir_parsed_and_span = (psy_child, + span) 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 self.last_comments_as_codeblocks and len(preceding_comments) != 0: + if self._last_comments_as_codeblocks and len(preceding_comments) != 0: self.nodes_to_code_block(parent, preceding_comments) def _create_child(self, child, parent=None): @@ -5334,7 +5323,6 @@ def _subroutine_handler(self, node, parent): arg_list = [] self.process_declarations(routine, decl_list, arg_list) - # TODO: fparser issue # 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. @@ -5442,7 +5430,6 @@ def _subroutine_handler(self, node, parent): # valid. pass else: - # TODO: fparser issue # Put the comments from the end of the declarations part # at the start of the execution part manually self.process_nodes(routine, lost_comments + sub_exec.content) @@ -5636,6 +5623,8 @@ def process_comment(self, comment, preceding_comments): ''' if len(comment.tostr()) == 0: return + if self._ignore_directives and comment.tostr().startswith("!$"): + return if self._last_psyir_parsed_and_span is not None: last_psyir, last_span = self._last_psyir_parsed_and_span if (last_span[1] is not None diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index a025906f31..07b6ac9a53 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -48,6 +48,7 @@ from psyclone.psyir.symbols.data_type_symbol import DataTypeSymbol from psyclone.psyir.symbols.datasymbol import DataSymbol from psyclone.psyir.symbols.symbol import Symbol +from psyclone.psyir.commentable_mixin import CommentableMixin class DataType(metaclass=abc.ABCMeta): @@ -909,7 +910,7 @@ class StructureType(DataType): ''' @dataclass(frozen=True) - class ComponentType: + class ComponentType(CommentableMixin): ''' Represents a member of a StructureType. @@ -929,8 +930,20 @@ class ComponentType: datatype: Union[DataType, DataTypeSymbol] visibility: Symbol.Visibility initial_value: Any - preceding_comment: str = "" - inline_comment: str = "" + + def __init__(self, name: str, + datatype: Union[DataType, DataTypeSymbol], + visibility: Symbol.Visibility, initial_value: Any = None, + preceding_comment: str = "", inline_comment: str = ""): + # pylint: disable=too-many-arguments + # Using object.__setattr__ due to frozen=True in dataclass, which + # prevents setting attributes directly. + object.__setattr__(self, 'name', name) + object.__setattr__(self, 'datatype', datatype) + object.__setattr__(self, 'visibility', visibility) + object.__setattr__(self, 'initial_value', initial_value) + object.__setattr__(self, '_preceding_comment', preceding_comment) + object.__setattr__(self, '_inline_comment', inline_comment) def __init__(self): self._components = OrderedDict() diff --git a/src/psyclone/tests/psyir/frontend/fortran_test.py b/src/psyclone/tests/psyir/frontend/fortran_test.py index 5f9c56f165..9c81ae7905 100644 --- a/src/psyclone/tests/psyir/frontend/fortran_test.py +++ b/src/psyclone/tests/psyir/frontend/fortran_test.py @@ -112,9 +112,8 @@ def test_fortran_psyir_from_source_fixed_form(): Test we parse also fixed-form fortran code when enabling the right option. ''' - fortran_reader = FortranReader() - file_container = fortran_reader.psyir_from_source(FIXED_FORM_CODE, - free_form=False) + fortran_reader = FortranReader(free_form=False) + file_container = fortran_reader.psyir_from_source(FIXED_FORM_CODE) assert isinstance(file_container, FileContainer) subroutine = file_container.children[0] assert isinstance(subroutine, Routine) @@ -252,8 +251,8 @@ def test_fortran_psyir_from_file(fortran_reader, tmpdir_factory): # Check that comments can be preserved, and that directives are still # ignored by default - file_container = fortran_reader.psyir_from_file( - filename, ignore_comments=False) + fortran_reader = FortranReader(ignore_comments=False) + file_container = fortran_reader.psyir_from_file(filename) assert isinstance(file_container, FileContainer) for node in file_container.walk(CommentableMixin): if isinstance(node, Loop): @@ -264,8 +263,9 @@ def test_fortran_psyir_from_file(fortran_reader, tmpdir_factory): assert node.preceding_comment == "" # Check that directives can be preserved - file_container = fortran_reader.psyir_from_file( - filename, ignore_comments=False, ignore_directives=False) + fortran_reader = FortranReader(ignore_comments=False, + ignore_directives=False) + file_container = fortran_reader.psyir_from_file(filename) assert isinstance(file_container, FileContainer) for node in file_container.walk(CommentableMixin): if isinstance(node, Loop): diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index e7d2a1c8ed..0c661bbe4c 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -160,10 +160,10 @@ def test_no_comments(): @pytest.mark.parametrize("last_comments_as_codeblocks", [True, False]) def test_comments_and_codeblocks(last_comments_as_codeblocks): """Test that the FortranReader is able to read comments""" - reader = FortranReader() - psyir = reader.psyir_from_source( - CODE, ignore_comments=False, + reader = FortranReader( + ignore_comments=False, last_comments_as_codeblocks=last_comments_as_codeblocks) + psyir = reader.psyir_from_source(CODE) module = psyir.children[0] assert ( @@ -458,11 +458,12 @@ def test_comments_and_codeblocks(last_comments_as_codeblocks): @pytest.mark.parametrize("last_comments_as_codeblocks", [True, False]) def test_write_comments(last_comments_as_codeblocks): """Test that the comments are written back to the code""" - reader = FortranReader() + reader = FortranReader( + ignore_comments=False, + last_comments_as_codeblocks=last_comments_as_codeblocks + ) writer = FortranWriter() - psyir = reader.psyir_from_source( - CODE, ignore_comments=False, - last_comments_as_codeblocks=last_comments_as_codeblocks) + psyir = reader.psyir_from_source(CODE) generated_code = writer(psyir) if last_comments_as_codeblocks: assert generated_code == EXPECTED_WITH_COMMENTS_AND_CODEBLOCKS @@ -485,10 +486,8 @@ def test_write_comments(last_comments_as_codeblocks): def test_no_directives(): """Test that the FortranReader is without directives by default""" - reader = FortranReader() - psyir = reader.psyir_from_source( - CODE_WITH_DIRECTIVE, ignore_comments=False - ) + reader = FortranReader(ignore_comments=False) + psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE) loop = psyir.walk(Loop)[0] assert loop.preceding_comment == "Comment on loop 'do i = 1, 10'" @@ -496,10 +495,8 @@ def test_no_directives(): def test_directives(): """Test that the FortranReader is able to read directives""" - reader = FortranReader() - psyir = reader.psyir_from_source( - CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False - ) + reader = FortranReader(ignore_comments=False, ignore_directives=False) + psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE) loop = psyir.walk(Loop)[0] assert ( @@ -528,11 +525,9 @@ def test_directives(): ) def test_write_directives(): """Test that the directives are written back to the code""" - reader = FortranReader() + reader = FortranReader(ignore_comments=False, ignore_directives=False) writer = FortranWriter() - psyir = reader.psyir_from_source( - CODE_WITH_DIRECTIVE, ignore_comments=False, ignore_directives=False - ) + psyir = reader.psyir_from_source(CODE_WITH_DIRECTIVE) generated_code = writer(psyir) assert generated_code == EXPECTED_WITH_DIRECTIVES @@ -552,10 +547,8 @@ def test_write_directives(): def test_inline_comment(): """Test that the FortranReader is able to read inline comments""" - reader = FortranReader() - psyir = reader.psyir_from_source( - CODE_WITH_INLINE_COMMENT, ignore_comments=False - ) + reader = FortranReader(ignore_comments=False) + psyir = reader.psyir_from_source(CODE_WITH_INLINE_COMMENT) routine = psyir.walk(Routine)[0] sym_a = routine.symbol_table.lookup("a") diff --git a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py index db15ada8e8..54dbe391e0 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py @@ -244,14 +244,15 @@ def test_preceding_comments(f2008_parser): a derived type definition. ''' fake_parent = Container("dummy_container") processor = Fparser2Reader() - fparser2spec = f2008_parser(FortranStringReader("subroutine my_sub\n" - "! This is a comment\n" - "! This is another comment\n" - "type :: my_type\n" - " integer :: flag\n" - "end type my_type ! Inline comment\n" - "end subroutine my_sub\n", - ignore_comments=False)) + fparser2spec = f2008_parser( + FortranStringReader("subroutine my_sub\n" + "! This is a comment\n" + "! This is another comment\n" + "type :: my_type\n" + " integer :: flag\n" + "end type my_type ! Inline comment\n" + "end subroutine my_sub\n", + ignore_comments=False)) sub_decl = walk(fparser2spec, types=Fortran2003.Subroutine_Subprogram) sub = processor._subroutine_handler(sub_decl[0], fake_parent) typ = sub.symbol_table.lookup("my_type") diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py index 1e38775983..f102dd7083 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py @@ -151,20 +151,6 @@ def test_get_arg_names(parser): # Class Fparser2Reader -def test_last_comments_as_codeblocks(): - '''Test that the last_comments_as_codeblocks property and setter works as - expected.''' - processor = Fparser2Reader() - # Default value - assert not processor.last_comments_as_codeblocks - # Setter expects a boolean - with pytest.raises(TypeError) as error: - processor.last_comments_as_codeblocks = "false" - assert ("The value of the last_comments_as_codeblocks property must " - "be a boolean but found 'str'." in str(error.value)) - processor.last_comments_as_codeblocks = True - assert processor.last_comments_as_codeblocks - def test_get_routine_schedules_wrong_module(parser): '''Test that get_routine_schedules() raises the expected errors if there diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 32fc56c411..989e8d83b7 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -1029,3 +1029,36 @@ def test_structuretype_replace_symbols(): table.add(newtsymbol) stype.replace_symbols_using(table) assert stype.components["barry"].datatype is newtsymbol + + +def test_structuretype_componenttype_eq(): + '''Test that the equality operator of StructureType.ComponentType does + not take the preceding_comment and inline_comment into account. + ''' + comp1 = StructureType.ComponentType("fred", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None) + comp2 = StructureType.ComponentType("fred", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None) + assert comp1 == comp2 + + comp1 = StructureType.ComponentType("fred", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None, + preceding_comment="A comment") + comp2 = StructureType.ComponentType("fred", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None, + preceding_comment="Another comment") + assert comp1 == comp2 + + comp1 = StructureType.ComponentType("fred", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None, + inline_comment="A comment") + comp2 = StructureType.ComponentType("fred", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None, + inline_comment="Another comment") + assert comp1 == comp2 + + comp1 = StructureType.ComponentType("fred", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None) + comp2 = StructureType.ComponentType("george", INTEGER_TYPE, + Symbol.Visibility.PUBLIC, None) + assert comp1 != comp2 From c0b1b7125c93ddb58c81aea7760a057e41ac6386 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Wed, 18 Dec 2024 17:37:23 +0100 Subject: [PATCH 17/20] #1247 Forgotten merge edits... --- src/psyclone/generator.py | 3 ++- src/psyclone/tests/psyir/frontend/fparser2_test.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index ee16b38ead..95818823e4 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -728,7 +728,8 @@ def code_transformation_mode(input_file, recipe_file, output_file, sys.exit(1) # Parse file - psyir = FortranReader(resolve_mods).psyir_from_file(input_file) + psyir = FortranReader(resolve_modules=resolve_mods)\ + .psyir_from_file(input_file) # Modify file if trans_recipe: diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py index b7fbb66eab..a58b01537a 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py @@ -1694,7 +1694,7 @@ def test_process_use_stmts_resolving_external_imports( # Add the path to the include_path and set up a frontend instance # witth the module_to_resolve names monkeypatch.setattr(Config.get(), '_include_paths', [tmpdir]) - processor = Fparser2Reader(value) + processor = Fparser2Reader(resolve_modules=value) reader = FortranStringReader(''' module test use other1 From a98eba547e0efd4089bd91a75aedda2012954da9 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Thu, 19 Dec 2024 19:15:19 +0100 Subject: [PATCH 18/20] #1247 Edits wrt Sergi's review --- src/psyclone/psyir/commentable_mixin.py | 10 +++++ src/psyclone/psyir/frontend/fparser2.py | 2 +- src/psyclone/psyir/symbols/datatypes.py | 45 +++++++++---------- .../tests/psyir/commentable_mixin_test.py | 7 +++ .../psyir/frontend/fparser2_comment_test.py | 14 ++++++ .../tests/psyir/symbols/datatype_test.py | 16 +++---- 6 files changed, 62 insertions(+), 32 deletions(-) diff --git a/src/psyclone/psyir/commentable_mixin.py b/src/psyclone/psyir/commentable_mixin.py index 13dde00b28..a3f1ca3ab8 100644 --- a/src/psyclone/psyir/commentable_mixin.py +++ b/src/psyclone/psyir/commentable_mixin.py @@ -63,6 +63,8 @@ def preceding_comment(self): def preceding_comment(self, comment): ''' :param str comment: comment preceding this statement. + + :raises TypeError: if the comment is not a string. ''' if not isinstance(comment, str): raise TypeError(f"The preceding_comment must be a string but" @@ -73,6 +75,8 @@ def append_preceding_comment(self, comment): ''' :param str comment: comment to append after an newline in this statement-preceding comment. + + :raises TypeError: if the comment is not a string. ''' if not isinstance(comment, str): raise TypeError(f"The preceding_comment must be a string but" @@ -94,10 +98,16 @@ def inline_comment(self): def inline_comment(self, comment): ''' :param str comment: inline comment associated with this statement. + + :raises TypeError: if the comment is not a string. + :raises ValueError: if the comment contains a newline character. ''' if not isinstance(comment, str): raise TypeError(f"The inline_comment must be a string but" f" found '{type(comment).__name__}'.") + if '\n' in comment: + raise ValueError(f"The inline_comment must be a single line but " + f"found a newline character in '{comment}'.") self._inline_comment = comment diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 9b20eb7765..38a9308307 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -53,6 +53,7 @@ from psyclone.configuration import Config from psyclone.errors import InternalError, GenerationError +from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.nodes import ( ArrayMember, ArrayOfStructuresReference, ArrayReference, Assignment, BinaryOperation, Call, CodeBlock, Container, Directive, FileContainer, @@ -60,7 +61,6 @@ Reference, Return, Routine, Schedule, StructureReference, UnaryOperation, WhileLoop) from psyclone.psyir.nodes.array_mixin import ArrayMixin -from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import ( ArgumentInterface, ArrayType, AutomaticInterface, CHARACTER_TYPE, CommonBlockInterface, ContainerSymbol, DataSymbol, DataTypeSymbol, diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index 07b6ac9a53..3121343544 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -45,10 +45,10 @@ from typing import Any, Union from psyclone.errors import InternalError +from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.symbols.data_type_symbol import DataTypeSymbol from psyclone.psyir.symbols.datasymbol import DataSymbol from psyclone.psyir.symbols.symbol import Symbol -from psyclone.psyir.commentable_mixin import CommentableMixin class DataType(metaclass=abc.ABCMeta): @@ -919,11 +919,6 @@ class ComponentType(CommentableMixin): :param visibility: whether this member is public or private. :param initial_value: the initial value of this member (if any). :type initial_value: Optional[:py:class:`psyclone.psyir.nodes.Node`] - :param preceding_comment: a comment that precedes this component. - :type preceding_comment: Optional[str] - :param inline_comment: a comment that follows this component on the - same line. - :type inline_comment: Optional[str] ''' name: str # Use Union for compatibility with Python < 3.10 @@ -931,26 +926,24 @@ class ComponentType(CommentableMixin): visibility: Symbol.Visibility initial_value: Any - def __init__(self, name: str, - datatype: Union[DataType, DataTypeSymbol], - visibility: Symbol.Visibility, initial_value: Any = None, - preceding_comment: str = "", inline_comment: str = ""): - # pylint: disable=too-many-arguments - # Using object.__setattr__ due to frozen=True in dataclass, which - # prevents setting attributes directly. - object.__setattr__(self, 'name', name) - object.__setattr__(self, 'datatype', datatype) - object.__setattr__(self, 'visibility', visibility) - object.__setattr__(self, 'initial_value', initial_value) - object.__setattr__(self, '_preceding_comment', preceding_comment) - object.__setattr__(self, '_inline_comment', inline_comment) - def __init__(self): self._components = OrderedDict() def __str__(self): return "StructureType<>" + def __copy__(self): + ''' + :returns: a copy of this StructureType. + :rtype: :py:class:`psyclone.psyir.symbols.StructureType` + ''' + new = StructureType() + for name, component in self.components.items(): + new.add(name, component.datatype, component.visibility, + component.initial_value, component.preceding_comment, + component.inline_comment) + return new + @staticmethod def create(components): ''' @@ -1057,9 +1050,15 @@ def add(self, name, datatype, visibility, initial_value, f"be a 'str' but got " f"'{type(inline_comment).__name__}'") - self._components[name] = self.ComponentType( - name, datatype, visibility, initial_value, preceding_comment, - inline_comment) + self._components[name] = self.ComponentType(name, datatype, visibility, + initial_value) + # Use object.__setattr__ due to the frozen nature of ComponentType + object.__setattr__(self._components[name], + "_preceding_comment", + preceding_comment) + object.__setattr__(self._components[name], + "_inline_comment", + inline_comment) def lookup(self, name): ''' diff --git a/src/psyclone/tests/psyir/commentable_mixin_test.py b/src/psyclone/tests/psyir/commentable_mixin_test.py index 1fdf0fe2cd..69b192a6e1 100644 --- a/src/psyclone/tests/psyir/commentable_mixin_test.py +++ b/src/psyclone/tests/psyir/commentable_mixin_test.py @@ -68,6 +68,13 @@ def test_statement_comment_properties(): assert "The inline_comment must be a string but found 'int'." \ in str(err.value) + # Check that inline_comment cannot contain '\n' + with pytest.raises(ValueError) as err: + statement.inline_comment = "My inline\ncomment" + assert ("The inline_comment must be a single line but " + "found a newline character in 'My inline\ncomment'." + in str(err.value)) + # Check the append_preceding_comment method statement._preceding_comment = None # Uninitialised preceding_comment with pytest.raises(TypeError) as err: diff --git a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py index 0c661bbe4c..48fa5b771b 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_comment_test.py @@ -541,6 +541,10 @@ def test_write_directives(): a = 1 ! Inline comment on 'a = 1' ! Preceding comment on 'i = 1' i = 1 ! Inline comment on 'i = 1' + + a = & ! First line of inline comment + i & ! Second line of inline comment + + 1 ! Third line of inline comment end subroutine test_sub """ @@ -567,3 +571,13 @@ def test_inline_comment(): assert "i = 1" in assignment.debug_string() assert assignment.preceding_comment == "Preceding comment on 'i = 1'" assert assignment.inline_comment == "Inline comment on 'i = 1'" + + # When processing + # a = & ! First line of inline comment + # i & ! Second line of inline comment + # + 1 ! Third line of inline comment + # only the third comment is kept as inline comment + assignment = routine.walk(Assignment)[2] + assert "a = i + 1" in assignment.debug_string() + assert assignment.preceding_comment == "" + assert assignment.inline_comment == "Third line of inline comment" diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 989e8d83b7..3025775ffc 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -1042,19 +1042,19 @@ def test_structuretype_componenttype_eq(): assert comp1 == comp2 comp1 = StructureType.ComponentType("fred", INTEGER_TYPE, - Symbol.Visibility.PUBLIC, None, - preceding_comment="A comment") + Symbol.Visibility.PUBLIC, None) + object.__setattr__(comp1, "_preceding_comment", "A comment") comp2 = StructureType.ComponentType("fred", INTEGER_TYPE, - Symbol.Visibility.PUBLIC, None, - preceding_comment="Another comment") + Symbol.Visibility.PUBLIC, None) + object.__setattr__(comp2, "_preceding_comment", "Another comment") assert comp1 == comp2 comp1 = StructureType.ComponentType("fred", INTEGER_TYPE, - Symbol.Visibility.PUBLIC, None, - inline_comment="A comment") + Symbol.Visibility.PUBLIC, None) + object.__setattr__(comp1, "_inline_comment", "A comment") comp2 = StructureType.ComponentType("fred", INTEGER_TYPE, - Symbol.Visibility.PUBLIC, None, - inline_comment="Another comment") + Symbol.Visibility.PUBLIC, None) + object.__setattr__(comp2, "_inline_comment", "Another comment") assert comp1 == comp2 comp1 = StructureType.ComponentType("fred", INTEGER_TYPE, From f268ddf94c1dfda26ae8135576ee4af91575fc95 Mon Sep 17 00:00:00 2001 From: JulienRemy Date: Mon, 23 Dec 2024 14:05:44 +0100 Subject: [PATCH 19/20] #1247 Missing test for `StructureType.__copy__` --- src/psyclone/tests/psyir/symbols/datatype_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 3025775ffc..2bba4e0521 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -1062,3 +1062,17 @@ def test_structuretype_componenttype_eq(): comp2 = StructureType.ComponentType("george", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None) assert comp1 != comp2 + + +def test_structuretype___copy__(): + '''Test the __copy__ method of StructureType.''' + stype = StructureType.create([ + ("nancy", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None), + ("peggy", REAL_TYPE, Symbol.Visibility.PRIVATE, + Literal("1.0", REAL_TYPE))]) + copied = stype.__copy__() + assert copied == stype + assert copied is not stype + # The components should be the same objects + assert copied.components["nancy"] == stype.components["nancy"] + assert copied.components["peggy"] == stype.components["peggy"] From 1e94f0a8656bcfe0b1be41a0ed9a7741845230e7 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Mon, 30 Dec 2024 16:33:25 +0000 Subject: [PATCH 20/20] #1247 Update changelog and some docstrings --- changelog | 4 ++++ src/psyclone/psyir/frontend/fparser2.py | 23 +++++++++-------------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/changelog b/changelog index 2ea22786bc..bc8831520e 100644 --- a/changelog +++ b/changelog @@ -9,6 +9,10 @@ frontend to chase down some (or all) module imports when constructing the PSyIR for existing code. + 4) PR #2821 for #1247. Adds support for parsing comments from the fparser2 + AST tree. There are two new FortranReader arguments to select the behaviour + for comments and directives (by default they are not parsed). + release 3.0.0 6th of December 2024 1) PR #2477 for #2463. Add support for Fortran Namelist statements. diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 38a9308307..c5bb2be430 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -852,17 +852,13 @@ class Fparser2Reader(): Class to encapsulate the functionality for processing the fparser2 AST and convert the nodes to PSyIR. - :param ignore_directives: If directives should be ignored or not - (default True). Only has an effect - if comments were not ignored. - :param last_comments_as_codeblocks: If the last comments in the - a given block (e.g. subroutine, - do, if-then body, etc.) should - be kept as code blocks or lost - (default False). - Only has an effect if comments were - not ignored. - + :param ignore_directives: Whether directives should be ignored or not + (default True). Only has an effect if comments were not ignored when + creating the fparser2 AST. + :param last_comments_as_codeblocks: Whether the last comments in the a + given block (e.g. subroutine, do, if-then body, etc.) should be kept as + CodeBlocks or lost (default False). Only has an effect if comments + were not ignored when creating the fparser2 AST. :param resolve_modules: Whether to resolve modules while parsing a file, for more precise control it also accepts a list of module names. Defaults to False. @@ -870,7 +866,6 @@ class Fparser2Reader(): :raises TypeError: if the constructor argument is not of the expected type. ''' - unary_operators = OrderedDict([ ('+', UnaryOperation.Operator.PLUS), ('-', UnaryOperation.Operator.MINUS), @@ -955,7 +950,7 @@ class SelectTypeInfo: def __init__(self, ignore_directives: bool = True, last_comments_as_codeblocks: bool = False, - resolve_modules=False): + resolve_modules: bool = False): if isinstance(resolve_modules, bool): self._resolve_all_modules = resolve_modules self._modules_to_resolve = [] @@ -1015,7 +1010,7 @@ def __init__(self, ignore_directives: bool = True, self._last_psyir_parsed_and_span = None # Whether to ignore directives when processing the fparser2 AST self._ignore_directives = ignore_directives - # Whether to keep the last comments in a given block as code blocks + # Whether to keep the last comments in a given block as CodeBlocks self._last_comments_as_codeblocks = last_comments_as_codeblocks @staticmethod