Repository: stfc/PSyclone

Issue:
[psyir] Check on whether dimensioning symbol is local does not work when symbol is in an ancestor scope.
For the following Fortran:

    module my_mod
      integer :: dim
    contains
      subroutine init()
        dim = 10
      end subroutine
      subroutine work()
        real, dimension(dim) :: my_array
      end subroutine
    end module

Then PSyclone complains:

    If a local datasymbol is used as part of a dimension declaration then it should be a constant, but 'dim' is not.

The problem is in `ArrayType._validate_shape` where the Symbol being referred to in the shape specification is looked up. It is found to have a 'local' interface and not be a constant. Both of which are correct. However, no check is made on which SymbolTable the symbol is in.

Discussion:

Patch:
diff --git a/changelog b/changelog
index 863a4d892..45b2fd83f 100644
--- a/changelog
+++ b/changelog
@@ -375,6 +375,8 @@
 	120) PR #1105 for #1104. Adds support for program, module and subroutine
 	to Fparser2Reader
 
+	121) PR #1101 towards #1089. Remove check on arrays dimension symbols.
+
 release 1.9.0 20th May 2020
 
 	1) #602 for #597. Modify Node.ancestor() to optionally include
diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py
index 379f14821..e37098570 100644
--- a/src/psyclone/psyir/symbols/datatypes.py
+++ b/src/psyclone/psyir/symbols/datatypes.py
@@ -1,7 +1,7 @@
 # -----------------------------------------------------------------------------
 # BSD 3-Clause License
 #
-# Copyright (c) 2019-2020, Science and Technology Facilities Council.
+# Copyright (c) 2019-2021, Science and Technology Facilities Council.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -43,6 +43,7 @@ from enum import Enum
 import six
 from psyclone.errors import InternalError
 from psyclone.psyir.symbols import TypeSymbol, DataSymbol
+from psyclone.psyir.symbols.symbol import Symbol
 
 
 @six.add_metaclass(abc.ABCMeta)
@@ -377,19 +378,9 @@ class ArrayType(DataType):
                             "declaration then it should be a scalar "
                             "integer or an unknown type, but '{0}' is a "
                             "'{1}'.".format(symbol.name, symbol.datatype))
-                # Check that any references are not to a local
-                # datasymbol that is not constant (as this would have
+                # TODO #1089 - add check that any References are not to a
+                # local datasymbol that is not constant (as this would have
                 # no value).
-                references = dimension.walk(Reference)
-                if references:
-                    for reference in references:
-                        if reference.symbol.is_local and \
-                           not reference.symbol.is_constant:
-                            raise TypeError(
-                                "If a local datasymbol is used as part of a "
-                                "dimension declaration then it should be a "
-                                "constant, but '{0}' is not."
-                                "".format(reference.symbol.name))
             elif not isinstance(dimension, (self.Extent, int)):
                 raise TypeError(
                     "DataSymbol shape list elements can only be "
@@ -493,9 +484,6 @@ class StructureType(DataType):
         :raises TypeError: if any of the supplied values are of the wrong type.
 
         '''
-        # This import has to be here to avoid circular dependencies
-        # pylint: disable=import-outside-toplevel
-        from psyclone.psyir.symbols import Symbol, TypeSymbol
         if not isinstance(name, str):
             raise TypeError(
                 "The name of a component of a StructureType must be a 'str' "


Test patch:
diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py
index e03fa35d8..2adc12db0 100644
--- a/src/psyclone/tests/psyir/symbols/datatype_test.py
+++ b/src/psyclone/tests/psyir/symbols/datatype_test.py
@@ -1,7 +1,7 @@
 # -----------------------------------------------------------------------------
 # BSD 3-Clause License
 #
-# Copyright (c) 2020, Science and Technology Facilities Council.
+# Copyright (c) 2020-2021, Science and Technology Facilities Council.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -40,8 +40,9 @@ from __future__ import absolute_import
 import pytest
 from psyclone.psyir.symbols import DataType, DeferredType, ScalarType, \
     ArrayType, UnknownFortranType, DataSymbol, StructureType, \
-    INTEGER_TYPE, REAL_TYPE, Symbol, TypeSymbol
-from psyclone.psyir.nodes import Literal, BinaryOperation, Reference
+    INTEGER_TYPE, REAL_TYPE, Symbol, TypeSymbol, SymbolTable
+from psyclone.psyir.nodes import Literal, BinaryOperation, Reference, \
+    Container, KernelSchedule
 from psyclone.errors import InternalError
 
 
@@ -304,22 +305,9 @@ def test_arraytype_invalid_shape_dimension_2():
             in str(excinfo.value))
 
 
-@pytest.mark.xfail(reason="issue #948. Support for this check "
-                   "needs to be added")
+@pytest.mark.xfail(reason="issue #1089. Support for this check needs to be"
+                   "implemented")
 def test_arraytype_invalid_shape_dimension_3():
-    '''Test that the ArrayType class raises an exception when one of the
-    dimensions of the shape list argument is a local datasymbol that does
-    not have a constant value (as this will not be initialised).'''
-
-    scalar_type = ScalarType(ScalarType.Intrinsic.INTEGER, 4)
-    data_symbol = DataSymbol("var", scalar_type)
-    with pytest.raises(TypeError) as info:
-        _ = ArrayType(scalar_type, [data_symbol])
-    assert ("If a local datasymbol is used to declare a dimension then it "
-            "should be a constant, but 'var' is not." in str(info.value))
-
-
-def test_arraytype_invalid_shape_dimension_4():
     '''Test that the ArrayType class raises an exception when one of the
     dimensions of the shape list argument is a DataNode that contains
     a local datasymbol that does not have a constant value (as this
@@ -338,6 +326,21 @@ def test_arraytype_invalid_shape_dimension_4():
             "not." in str(info.value))
 
 
+def test_arraytype_shape_dim_from_parent_scope():
+    ''' Check that the shape checking in the ArrayType class permits the
+    use of a reference to a symbol in a parent scope. '''
+    cont = Container("test_mod")
+    dim_sym = cont.symbol_table.new_symbol("dim1", symbol_type=DataSymbol,
+                                           datatype=INTEGER_TYPE)
+    kernel1 = KernelSchedule.create("mod_1", SymbolTable(), [])
+    cont.addchild(kernel1)
+    kernel1.parent = cont
+    asym = kernel1.symbol_table.new_symbol(
+        "array1", symbol_type=DataSymbol,
+        datatype=ArrayType(INTEGER_TYPE, [Reference(dim_sym)]))
+    assert isinstance(asym, DataSymbol)
+
+
 def test_arraytype_str():
     '''Test that the ArrayType class str method works as expected.'''
     scalar_type = ScalarType(ScalarType.Intrinsic.INTEGER,