diff --git a/src/psyclone/psyir/symbols/symbol_table.py b/src/psyclone/psyir/symbols/symbol_table.py index d3e09158e4..9c45b5f3ba 100644 --- a/src/psyclone/psyir/symbols/symbol_table.py +++ b/src/psyclone/psyir/symbols/symbol_table.py @@ -1583,12 +1583,12 @@ def resolve_imports(self, container_symbols=None, symbol_target=None): :type symbol_target: Optional[ :py:class:`psyclone.psyir.symbols.Symbol`] - :raises SymbolError: if a symbol name clash is found between multiple \ + :raises SymbolError: if a symbol name clash is found between multiple imports or an import and a local symbol. - :raises TypeError: if the provided container_symbols is not a list of \ + :raises TypeError: if the provided container_symbols is not a list of ContainerSymbols. :raises TypeError: if the provided symbol_target is not a Symbol. - :raises KeyError: if a symbol_target has been specified but this has \ + :raises KeyError: if a symbol_target has been specified but this has not been found in any of the searched containers. ''' @@ -1617,7 +1617,7 @@ def resolve_imports(self, container_symbols=None, symbol_target=None): try: external_container = c_symbol.find_container_psyir( local_node=self.node) - # pylint: disable=broad-except + # pylint: disable-next=broad-except except Exception: # Ignore this container if the associated module file has not # been found in the given include_path or any issue has arisen @@ -1625,6 +1625,11 @@ def resolve_imports(self, container_symbols=None, symbol_target=None): # TODO #11: It would be useful to log this. continue + if not external_container: + # Failed to get a Container (possibly due to parsing or raising + # errors). + continue + # Examine all Symbols defined within this external container for symbol in external_container.symbol_table.symbols: if symbol.visibility == Symbol.Visibility.PRIVATE: diff --git a/src/psyclone/psyir/transformations/inline_trans.py b/src/psyclone/psyir/transformations/inline_trans.py index 77084b9231..a0892a31ea 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -37,7 +37,7 @@ This module contains the InlineTrans transformation. ''' -from psyclone.errors import LazyString +from psyclone.errors import LazyString, InternalError from psyclone.psyGen import Transformation from psyclone.psyir.nodes import ( ArrayReference, ArrayOfStructuresReference, BinaryOperation, Call, @@ -133,10 +133,15 @@ def apply(self, node, options=None): :type options: Optional[Dict[str, Any]] :param bool options["force"]: whether or not to permit the inlining of Routines containing CodeBlocks. Default is False. + + :raises InternalError: if the merge of the symbol tables fails. + In theory this should never happen because validate() should + catch such a situation. + ''' self.validate(node, options) # The table associated with the scoping region holding the Call. - table = node.scope.symbol_table + table = node.ancestor(Routine).symbol_table # Find the routine to be inlined. orig_routine = node.get_callees()[0] @@ -161,8 +166,14 @@ def apply(self, node, options=None): # Shallow copy the symbols from the routine into the table at the # call site. - table.merge(routine_table, - symbols_to_skip=routine_table.argument_list[:]) + try: + table.merge(routine_table, + symbols_to_skip=routine_table.argument_list[:]) + except SymbolError as err: + raise InternalError( + f"Error copying routine symbols to call site. This should " + f"have been caught by the validate() method. Original error " + f"was {err}") # When constructing new references to replace references to formal # args, we need to know whether any of the actual arguments are array @@ -184,11 +195,6 @@ def apply(self, node, options=None): for ref in refs[:]: self._replace_formal_arg(ref, node, formal_args) - # Store the Routine level symbol table and node's current scope - # so we can merge symbol tables later if required. - ancestor_table = node.ancestor(Routine).scope.symbol_table - scope = node.scope - # Copy the nodes from the Routine into the call site. # TODO #924 - while doing this we should ensure that any References # to common/shared Symbols in the inlined code are updated to point @@ -206,7 +212,6 @@ def apply(self, node, options=None): for child in new_stmts: idx += 1 parent.addchild(child, idx) - table = parent.scope.symbol_table # Avoid a potential name clash with the original function table.rename_symbol( routine.return_symbol, table.next_available_name( @@ -221,16 +226,6 @@ def apply(self, node, options=None): idx += 1 parent.addchild(child, idx) - # If the scope we merged the inlined function's symbol table into - # is not a Routine scope then we now merge that symbol table into - # the ancestor Routine. This avoids issues like #2424 when - # applying ParallelLoopTrans to loops containing inlined calls. - if ancestor_table is not scope.symbol_table: - ancestor_table.merge(scope.symbol_table) - replacement = type(scope.symbol_table)() - scope.symbol_table.detach() - replacement.attach(scope) - def _replace_formal_arg(self, ref, call_node, formal_args): ''' Recursively combines any References to formal arguments in the supplied @@ -599,6 +594,7 @@ def validate(self, node, options=None): and the 'force' option is not True. :raises TransformationError: if the called routine has a named argument. + :raises TransformationError: if the call-site is not within a Routine. :raises TransformationError: if any of the variables declared within the called routine are of UnknownInterface. :raises TransformationError: if any of the variables declared within @@ -633,6 +629,13 @@ def validate(self, node, options=None): f"Cannot inline an IntrinsicCall ('{node.routine.name}')") name = node.routine.name + # The call site must be within a Routine (i.e. not detached) + parent_routine = node.ancestor(Routine) + if not parent_routine: + raise TransformationError( + f"Routine '{name}' cannot be inlined because the call site " + f"('{node.debug_string().strip()}') is not inside a Routine.") + # Check that we can find the source of the routine being inlined. # TODO #924 allow for multiple routines (interfaces). try: @@ -661,8 +664,8 @@ def validate(self, node, options=None): # CodeBlocks to be included. raise TransformationError( f"Routine '{name}' contains one or more CodeBlocks and " - "therefore cannot be inlined. (If you are confident that " - "the code may safely be inlined despite this then use " + f"therefore cannot be inlined. (If you are confident that " + f"the code may safely be inlined despite this then use " "`options={'force': True}` to override.)") # Support for routines with named arguments is not yet implemented. @@ -673,7 +676,7 @@ def validate(self, node, options=None): f"Routine '{routine.name}' cannot be inlined because it " f"has a named argument '{arg}' (TODO #924).") - table = node.scope.symbol_table + table = parent_routine.symbol_table routine_table = routine.symbol_table for sym in routine_table.datasymbols: @@ -693,7 +696,10 @@ def validate(self, node, options=None): raise TransformationError( f"Routine '{routine.name}' cannot be inlined because it " f"contains a Symbol '{sym.name}' with an UnknownInterface:" - f" '{sym.datatype.declaration}'") + f" '{sym.datatype.declaration}'. You may be able to work " + f"around this limitation by adding the name of the module " + f"containing this Symbol to RESOLVE_IMPORTS in the " + f"transformation script.") # Check that there are no static variables in the routine (because # we don't know whether the routine is called from other places). if (isinstance(sym.interface, StaticInterface) and diff --git a/src/psyclone/tests/psyir/symbols/symbol_table_test.py b/src/psyclone/tests/psyir/symbols/symbol_table_test.py index 6e29095b56..f220e7b8f9 100644 --- a/src/psyclone/tests/psyir/symbols/symbol_table_test.py +++ b/src/psyclone/tests/psyir/symbols/symbol_table_test.py @@ -2877,6 +2877,20 @@ def test_resolve_imports(fortran_reader, tmpdir, monkeypatch): assert a_2.visibility == symbols.Symbol.Visibility.PRIVATE +def test_resolve_imports_missing_container(monkeypatch): + ''' + Test that a clean failure to get Container PSyIR does not cause problems. + ''' + table = symbols.SymbolTable() + csym = symbols.ContainerSymbol("a_mod") + # Monkeypatch the find_container_psyir() method of this ContainerSymbol + # so that it returns None. + monkeypatch.setattr(csym, "find_container_psyir", lambda local_node: None) + table.add(csym) + # Resolving imports should run without problems. + table.resolve_imports() + + @pytest.mark.usefixtures("clear_module_manager_instance") def test_resolve_imports_different_capitalization( fortran_reader, tmpdir, monkeypatch): diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index 2e61e19530..4b8d5ff043 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -41,6 +41,7 @@ import pytest from psyclone.configuration import Config +from psyclone.errors import InternalError from psyclone.psyir.nodes import Call, IntrinsicCall, Reference, Routine, Loop from psyclone.psyir.symbols import ( AutomaticInterface, DataSymbol, UnresolvedType) @@ -1271,6 +1272,36 @@ def test_apply_callsite_rename_container(fortran_reader, fortran_writer): " i = i * a_mod_1\n" in output) +def test_apply_internal_error(fortran_reader, monkeypatch): + ''' + Test that we raise the expected error in apply if we find a situation that + should have been caught by validate. + ''' + code = ( + "module test_mod\n" + "contains\n" + " subroutine run_it()\n" + " use some_mod, only: a_clash\n" + " integer :: i\n" + " i = 10\n" + " call sub(i)\n" + " end subroutine run_it\n" + " subroutine sub(idx)\n" + " use other_mod, only: a_clash\n" + " integer :: idx\n" + " idx = idx + trouble\n" + " end subroutine sub\n" + "end module test_mod\n") + psyir = fortran_reader.psyir_from_source(code) + call = psyir.walk(Call)[0] + inline_trans = InlineTrans() + monkeypatch.setattr(inline_trans, "validate", lambda _a, _b: None) + with pytest.raises(InternalError) as err: + inline_trans.apply(call) + assert ("Error copying routine symbols to call site. This should have " + "been caught" in str(err.value)) + + def test_validate_non_local_import(fortran_reader): '''Test that we reject the case where the routine to be inlined accesses a symbol from an import in its parent container.''' @@ -2164,7 +2195,22 @@ def test_validate_named_arg(fortran_reader): f"end module inline_mod\n") -def test_apply_merges_symbol_table_with_routine(fortran_reader): +def test_validate_call_within_routine(fortran_reader): + ''' + Check that validate raises the expected error if the call is not within + a Routine. + ''' + psyir = fortran_reader.psyir_from_source(CALL_IN_SUB_USE) + call = psyir.walk(Call)[0] + inline_trans = InlineTrans() + with pytest.raises(TransformationError) as err: + inline_trans.validate(call.detach()) + assert ("Routine 'sub' cannot be inlined because the call site ('call " + "sub(a)') is not inside a Routine" in str(err.value)) + + +def test_apply_merges_symbol_table_with_routine(fortran_reader, + fortran_writer): ''' Check that the apply method merges the inlined function's symbol table to the containing Routine when the call node is inside a child ScopingNode.