(Closes #2845) fix for inline symbol bug

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,14 +1617,19 @@ 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
                 # during parsing.
                 # 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:

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

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):

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.