(Closes #2125) Fix bugs in SymbolTable deep copy and InlineTrans when array bounds reference formal arg

src/psyclone/psyir/symbols/datasymbol.py

@@ -303,10 +303,21 @@ def copy(self):
 
         '''
         if self.initial_value is not None:
+            # Ensure any References in the initial-value expression are
+            # also copied. At this stage they will still point to the
+            # same Symbols as the original.
             new_init_value = self.initial_value.copy()
         else:
             new_init_value = None
-        return DataSymbol(self.name, self.datatype, visibility=self.visibility,
+        if self.is_array:
+            # Ensure any References in the shape definition of an ArrayType
+            # are also copied. At this stage they will still point to the
+            # same Symbols as the original.
+            new_datatype = self.datatype.copy()
+        else:
+            new_datatype = self.datatype
+        return DataSymbol(self.name, new_datatype,
+                          visibility=self.visibility,
                           interface=self.interface,
                           is_constant=self.is_constant,
                           initial_value=new_init_value)

src/psyclone/psyir/symbols/datatypes.py

@@ -38,6 +38,7 @@
 ''' This module contains the datatype definitions.'''
 
 import abc
+import copy
 from collections import OrderedDict, namedtuple
 from enum import Enum
 
@@ -67,6 +68,13 @@ def __eq__(self, other):
         '''
         return type(other) is type(self)
 
+    def copy(self):
+        '''
+        :returns: a shallow copy of this datatype.
+        :rtype: :py:class:`psyclone.psyir.symbols.datatypes.DataType`
+        '''
+        return copy.copy(self)
+
 
 class UnresolvedType(DataType):
     # pylint: disable=too-few-public-methods
@@ -116,7 +124,7 @@ def __str__(self):
     @property
     def declaration(self):
         '''
-        :returns: the original declaration of the symbol. This is obviously \
+        :returns: the original declaration of the symbol. This is obviously
                   language specific and hence this class must be subclassed.
         :rtype: str
         '''
@@ -380,6 +388,13 @@ class Extent(Enum):
         DEFERRED = 1
         ATTRIBUTE = 2
 
+        def copy(self):
+            '''
+            :returns: a shallow copy of self.
+            :rtype: :py:class:`psyclone.psyir.symbols.ArrayType.Extent`
+            '''
+            return copy.copy(self)
+
     #: namedtuple used to store lower and upper limits of an array dimension
     ArrayBounds = namedtuple("ArrayBounds", ["lower", "upper"])
 
@@ -456,7 +471,7 @@ def _dangling_parent(var):
                 # The lower bound is 1 by default.
                 self._shape.append(
                     ArrayType.ArrayBounds(
-                        _dangling_parent(one),
+                        _dangling_parent(one.copy()),
                         _dangling_parent(_node_from_int(dim))))
             elif isinstance(dim, tuple):
                 self._shape.append(
@@ -709,6 +724,27 @@ def __eq__(self, other):
 
         return True
 
+    def copy(self):
+        '''
+        Create a shallow copy of this ArrayType. (Any References will be
+        re-created but the target Symbols will remain unchanged.)
+
+        :returns: a copy of this ArrayType.
+        :rtype: :py:class:`psyclone.psyir.datatype.ArrayType`
+
+        '''
+        new_shape = []
+        for dim in self.shape:
+            if isinstance(dim, ArrayType.ArrayBounds):
+                new_bounds = ArrayType.ArrayBounds(dim.lower.copy(),
+                                                   dim.upper.copy())
+                new_shape.append(new_bounds)
+            else:
+                # This dimension is specified with an ArrayType.Extent
+                # so no need to copy.
+                new_shape.append(dim)
+        return ArrayType(self.datatype, new_shape)
+
 
 class StructureType(DataType):
     '''

src/psyclone/psyir/symbols/symbol_table.py

@@ -52,6 +52,7 @@
 from psyclone.psyir.symbols import (
     DataSymbol, ContainerSymbol, DataTypeSymbol, GenericInterfaceSymbol,
     ImportInterface, RoutineSymbol, Symbol, SymbolError, UnresolvedInterface)
+from psyclone.psyir.symbols.datatypes import ArrayType
 from psyclone.psyir.symbols.intrinsic_symbol import IntrinsicSymbol
 from psyclone.psyir.symbols.typed_symbol import TypedSymbol
 
@@ -145,14 +146,12 @@ def parent_symbol_table(self, scope_limit=None):
         '''If this symbol table is enclosed in another scope, return the
         symbol table of the next outer scope. Otherwise return None.
 
-        :param scope_limit: optional Node which limits the symbol \
-            search space to the symbol tables of the nodes within the \
-            given scope. If it is None (the default), the whole \
-            scope (all symbol tables in ancestor nodes) is searched \
-            otherwise ancestors of the scope_limit node are not \
-            searched.
-        :type scope_limit: :py:class:`psyclone.psyir.nodes.Node` or \
-            `NoneType`
+        :param scope_limit: optional Node which limits the symbol search
+            space to the symbol tables of the nodes within the given scope.
+            If it is None (the default), the whole scope (all symbol tables
+            in ancestor nodes) is searched otherwise ancestors of the
+            scope_limit node are not searched.
+        :type scope_limit: Optional[:py:class:`psyclone.psyir.nodes.Node`]
 
         :returns: the 'parent' SymbolTable of the current SymbolTable (i.e.
                   the one that encloses this one in the PSyIR hierarchy).
@@ -303,6 +302,40 @@ def deep_copy(self):
                                      routine.from_container))
             symbol.routines = new_routines
 
+        # Ensure any Symbols referenced in array shapes are also updated
+        # pylint: disable-next=import-outside-toplevel
+        from psyclone.psyir.nodes import Node, Reference
+        for symbol in new_st.symbols:
+            if not (symbol.is_array and isinstance(symbol.datatype,
+                                                   ArrayType)):
+                continue
+            for dim in symbol.datatype.shape:
+                if isinstance(dim, ArrayType.ArrayBounds):
+                    exprns = dim
+                else:
+                    exprns = [dim]
+                for bnd in exprns:
+                    if isinstance(bnd, Node):
+                        for ref in bnd.walk(Reference):
+                            try:
+                                ref.symbol = new_st.lookup(ref.symbol.name)
+                            except KeyError:
+                                # This dimensioning symbol isn't in the table
+                                # we are copying.
+                                pass
+
+        # Ensure any Symbols referenced in initial values are updated.
+        for symbol in new_st.symbols:
+            if not isinstance(symbol, DataSymbol):
+                continue
+            if symbol.initial_value:
+                for ref in symbol.initial_value.walk(Reference):
+                    try:
+                        ref.symbol = new_st.lookup(ref.symbol.name)
+                    except KeyError:
+                        # This symbol isn't in the table we are copying.
+                        pass
+
         # Set the default visibility
         new_st._default_visibility = self.default_visibility
 

src/psyclone/psyir/transformations/inline_trans.py

@@ -131,7 +131,8 @@ def apply(self, node, options=None):
         :type node: :py:class:`psyclone.psyir.nodes.Routine`
         :param options: a dictionary with options for transformations.
         :type options: Optional[Dict[str, Any]]
-
+        :param bool options["force"]: whether or not to permit the inlining
+            of Routines containing CodeBlocks. Default is False.
         '''
         self.validate(node, options)
         # The table associated with the scoping region holding the Call.
@@ -448,12 +449,18 @@ def _update_actual_indices(self, actual_arg, local_ref,
                     # The formal argument declaration has a shape.
                     local_shape = local_decln_shape[local_idx_posn]
                     local_decln_start = local_shape.lower
+                    if isinstance(local_decln_start, Node):
+                        # Ensure any references to formal arguments within
+                        # the declared array lower bound are updated.
+                        local_decln_start = self._replace_formal_arg(
+                            local_decln_start, call_node, formal_args)
                 elif (local_decln_shape[local_idx_posn] ==
                       ArrayType.Extent.DEFERRED):
                     # The formal argument is declared to be allocatable and
                     # therefore has the same bounds as the actual argument.
                     local_shape = None
                     local_decln_start = actual_start
+
             if not local_decln_start:
                 local_shape = None
                 local_decln_start = _ONE
@@ -609,35 +616,41 @@ def validate(self, node, options=None):
         :type node: subclass of :py:class:`psyclone.psyir.nodes.Call`
         :param options: a dictionary with options for transformations.
         :type options: Optional[Dict[str, Any]]
+        :param bool options["force"]: whether or not to ignore any CodeBlocks
+            in the candidate routine. Default is False.
 
-        :raises TransformationError: if the supplied node is not a Call or is \
+        :raises TransformationError: if the supplied node is not a Call or is
             an IntrinsicCall.
         :raises TransformationError: if the routine has a return value.
-        :raises TransformationError: if the routine body contains a Return \
+        :raises TransformationError: if the routine body contains a Return
             that is not the first or last statement.
-        :raises TransformationError: if the routine body contains a CodeBlock.
-        :raises TransformationError: if the called routine has a named \
+        :raises TransformationError: if the routine body contains a CodeBlock
+            and the 'force' option is not True.
+        :raises TransformationError: if the called routine has a named
             argument.
-        :raises TransformationError: if any of the variables declared within \
+        :raises TransformationError: if any of the variables declared within
             the called routine are of UnknownInterface.
-        :raises TransformationError: if any of the variables declared within \
+        :raises TransformationError: if any of the variables declared within
             the called routine have a StaticInterface.
-        :raises TransformationError: if any of the subroutine arguments is of \
+        :raises TransformationError: if any of the subroutine arguments is of
             UnsupportedType.
-        :raises TransformationError: if a symbol of a given name is imported \
+        :raises TransformationError: if a symbol of a given name is imported
             from different containers at the call site and within the routine.
-        :raises TransformationError: if the routine accesses an un-resolved \
+        :raises TransformationError: if the routine accesses an un-resolved
             symbol.
-        :raises TransformationError: if the number of arguments in the call \
+        :raises TransformationError: if the number of arguments in the call
             does not match the number of formal arguments of the routine.
-        :raises TransformationError: if a symbol declared in the parent \
+        :raises TransformationError: if a symbol declared in the parent
             container is accessed in the target routine.
-        :raises TransformationError: if the shape of an array formal argument \
+        :raises TransformationError: if the shape of an array formal argument
             does not match that of the corresponding actual argument.
 
         '''
         super().validate(node, options=options)
 
+        options = {} if options is None else options
+        forced = options.get("force", False)
+
         # The node should be a Call.
         if not isinstance(node, Call):
             raise TransformationError(
@@ -665,10 +678,14 @@ def validate(self, node, options=None):
                     f"Routine '{name}' contains one or more "
                     f"Return statements and therefore cannot be inlined.")
 
-        if routine.walk(CodeBlock):
+        if routine.walk(CodeBlock) and not forced:
+            # N.B. we permit the user to specify the "force" option to allow
+            # CodeBlocks to be included.
             raise TransformationError(
-                f"Routine '{name}' contains one or more "
-                f"CodeBlocks and therefore cannot be inlined.")
+                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 "
+                "`options={'force': True}` to override.)")
 
         # Support for routines with named arguments is not yet implemented.
         # TODO #924.

src/psyclone/tests/psyir/backend/c_test.py

@@ -425,7 +425,7 @@ def test_cw_loop(fortran_reader):
     code = '''
         module test
         contains
-        subroutine tmp()
+        subroutine tmp(b)
           integer :: i, a
           integer, dimension(:) :: b
           do i = 1, 20, 2

src/psyclone/tests/psyir/symbols/datatype_test.py

@@ -85,6 +85,15 @@ def test_unresolvedtype_eq():
     assert data_type1 != NoType()
 
 
+def test_unresolvedtype_copy():
+    '''Test the copy() method of UnresolvedType (which is just inherited
+    from DataType).'''
+    dtype1 = UnresolvedType()
+    dtype2 = dtype1.copy()
+    assert isinstance(dtype2, UnresolvedType)
+    assert dtype2 is not dtype1
+
+
 # NoType class
 
 def test_notype():
@@ -268,6 +277,14 @@ def test_scalartype_immutable():
 
 # ArrayType class
 
+def test_arraytype_extent():
+    '''Test the ArrayType.Extent class. This is just an enum with a
+    copy() method. '''
+    xtent = ArrayType.Extent.ATTRIBUTE
+    ytent = xtent.copy()
+    assert isinstance(ytent, ArrayType.Extent)
+
+
 def test_arraytype():
     '''Test that the ArrayType class __init__ works as expected. Test the
     different dimension datatypes that are supported.'''
@@ -319,8 +336,9 @@ def test_arraytype():
     assert array_type.shape[1] == ArrayType.Extent.DEFERRED
     # Provided as an attribute extent
     array_type = ArrayType(
-        scalar_type, [ArrayType.Extent.ATTRIBUTE, ArrayType.Extent.ATTRIBUTE])
-    assert array_type.shape[1] == ArrayType.Extent.ATTRIBUTE
+        scalar_type, [ArrayType.Extent.ATTRIBUTE,
+                      (2, ArrayType.Extent.ATTRIBUTE)])
+    assert array_type.shape[1].upper == ArrayType.Extent.ATTRIBUTE
 
 
 def test_arraytype_invalid_datatype():
@@ -366,6 +384,7 @@ def test_arraytype_unsupportedtype():
     assert isinstance(atype, ArrayType)
     assert atype.datatype is utype
     assert atype.precision is None
+    assert utype.declaration == "integer, pointer :: var"
 
 
 def test_arraytype_invalid_shape():
@@ -572,6 +591,27 @@ def test_arraytype_eq():
     assert data_type1 != ArrayType(iscalar_type, [10, 10])
 
 
+def test_arraytype_copy():
+    '''Test the copy() method of ArrayType.'''
+    sym1 = DataSymbol("alimit", INTEGER_TYPE)
+    atype = ArrayType(INTEGER_TYPE, [Reference(sym1),
+                                     (Reference(sym1), Reference(sym1))])
+    acopy = atype.copy()
+    assert acopy == atype
+    assert acopy is not atype
+    # The Reference defining the upper bound should have been copied.
+    assert acopy.shape[0].upper is not atype.shape[0].upper
+    assert acopy.shape[1].lower is not atype.shape[1].lower
+    # But the new Reference should still refer to the same Symbol.
+    assert acopy.shape[0].upper.symbol is atype.shape[0].upper.symbol
+    assert acopy.shape[1].lower.symbol is atype.shape[1].lower.symbol
+    # When shape doesn't have set bounds.
+    btype = ArrayType(INTEGER_TYPE, [ArrayType.Extent.ATTRIBUTE])
+    bcopy = btype.copy()
+    assert bcopy == btype
+    assert bcopy is not btype
+
+
 # UnsupportedFortranType tests
 
 def test_unsupported_fortran_type():