Repository: stfc/PSyclone

Issue:
[NEMO] Dimensioning variable with DeferredType causes crash
The following code:

    subroutine mysub()
        use some_mod, only: jpi, jpj
        integer, dimension(jpi, jpj) :: some_array
    end subroutine

causes a crash in in `_parse_dimensions()` in the fparser2 frontend. This is because `jpi`/`jpj` have been found in the `use` statement and added to the symbol table with `DeferredType`. This then does not have an `intrinsic_type` which the code in `_parse_dimensions` expects:

                        sym = symbol_table.lookup(dim_name)
                        if (sym.datatype.intrinsic !=
                                ScalarType.Intrinsic.INTEGER or
                                isinstance(sym.datatype, ArrayType)):
                            _unsupported_type_error(dimensions)


Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index 583c0ab4b..f5d489943 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -48,7 +48,7 @@ before_install:
   # Specify "fparser_submodule" (without quotes) as an argument to
   # install_optional.sh in order to run the tests with the version of
   # fparser pointed to by the git submodule.
-  - ./bin/install_optional.sh
+  - ./bin/install_optional.sh fparser_submodule
   - pip install codecov
   # Some of the examples include Jupyter notebooks
   - pip install jupyter
diff --git a/changelog b/changelog
index 1de4d7c05..eaaa09a9a 100644
--- a/changelog
+++ b/changelog
@@ -37,6 +37,9 @@
 	15) #811 for #783. Removes potential duplication of orientation
 	pointer declarations.
 
+	16) #808 for #800. Adds support in the PSyIR for the case when a
+	variable used to dimension an array has deferred or unknown type.
+
 release 1.9.0 20th May 2020
 
 	1) #602 for #597. Modify Node.ancestor() to optionally include
diff --git a/external/fparser b/external/fparser
index 5ae2ab688..4be91d2e1 160000
--- a/external/fparser
+++ b/external/fparser
@@ -1,1 +1,1 @@
-Subproject commit 5ae2ab688f6e0558e2934348fa59737521159e1b
+Subproject commit 4be91d2e1078fa5588e84a3299131f297afba675
diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py
index 7e270efcc..1610180c7 100644
--- a/src/psyclone/psyir/frontend/fparser2.py
+++ b/src/psyclone/psyir/frontend/fparser2.py
@@ -900,9 +900,15 @@ class Fparser2Reader(object):
                     dim_name = dim.items[1].string.lower()
                     try:
                         sym = symbol_table.lookup(dim_name)
-                        if (sym.datatype.intrinsic !=
-                                ScalarType.Intrinsic.INTEGER or
-                                isinstance(sym.datatype, ArrayType)):
+                        if isinstance(sym.datatype, (UnknownType,
+                                                     DeferredType)):
+                            # Allow symbols of Unknown/DeferredType.
+                            pass
+                        elif not (isinstance(sym.datatype, ScalarType) and
+                                  sym.datatype.intrinsic ==
+                                  ScalarType.Intrinsic.INTEGER):
+                            # It's not of Unknown/DeferredType and it's not an
+                            # integer scalar.
                             _unsupported_type_error(dimensions)
                     except KeyError:
                         # We haven't seen this symbol before so create a new
diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py
index ea4e39e65..32b89f616 100644
--- a/src/psyclone/psyir/symbols/datatypes.py
+++ b/src/psyclone/psyir/symbols/datatypes.py
@@ -38,8 +38,8 @@
 
 from __future__ import absolute_import
 import abc
-import six
 from enum import Enum
+import six
 from psyclone.errors import InternalError
 
 
@@ -207,8 +207,8 @@ class ArrayType(DataType):
         system (e.g. using the SIZE intrinsic in Fortran). If it is \
         DataSymbol.Extent.DEFERRED then the extent is also unknown and may or \
         may not be defined at run-time (e.g. the array is ALLOCATABLE in \
-        Fortran). Otherwise it holds an integer literal or a reference to an \
-        integer symbol with the extent.
+        Fortran). Otherwise it holds an integer literal or a reference to a \
+        symbol (of integer or unknown/deferred type) that specifies the extent.
 
     :raises TypeError: if the arguments are of the wrong type.
 
@@ -227,33 +227,14 @@ class ArrayType(DataType):
 
     def __init__(self, datatype, shape):
 
-        from psyclone.psyir.symbols.datasymbol import DataSymbol
         if not isinstance(datatype, DataType):
             raise TypeError(
                 "ArrayType expected 'datatype' argument to be of type "
                 "DataType but found '{0}'."
                 "".format(type(datatype).__name__))
-        if not isinstance(shape, list):
-            raise TypeError(
-                "ArrayType expected 'shape' argument to be of type "
-                "list but found '{0}'."
-                "".format(type(shape).__name__))
-
-        for dimension in shape:
-            if isinstance(dimension, DataSymbol):
-                if not (dimension.is_scalar and
-                        dimension.datatype.intrinsic ==
-                        ScalarType.Intrinsic.INTEGER):
-                    raise TypeError(
-                        "DataSymbols that are part of another symbol shape can"
-                        " only be scalar integers, but found '{0}'."
-                        "".format(str(dimension)))
-            elif not isinstance(dimension, (self.Extent, int)):
-                raise TypeError(
-                    "DataSymbol shape list elements can only be "
-                    "'DataSymbol', 'integer' or ArrayType.Extent, but "
-                    "found '{0}'.".format(type(dimension).__name__))
-
+        # We do not have a setter for shape as it is an immutable property,
+        # therefore we have a separate validation routine.
+        self._validate_shape(shape)
         self._shape = shape
         self._intrinsic = datatype.intrinsic
         self._precision = datatype.precision
@@ -298,6 +279,46 @@ class ArrayType(DataType):
         '''
         return self._shape
 
+    def _validate_shape(self, extents):
+        '''
+        Check that the supplied shape specification is valid. This is not
+        implemented as a setter because the shape property is immutable.
+
+        :param extents: list of extents, one for each array dimension.
+        :type extents: list of :py:class:`psyclone.psyir.symbols.DataSymbol`, \
+            :py:class:`psyclone.psyir.symbols.ArrayType.Extent` or int
+
+        :raises TypeError: if extents is not a list.
+        :raises TypeError: if one or more of the supplied extents is a \
+            DataSymbol that is not a scalar integer or of Unknown/DeferredType.
+        :raises TypeError: if one or more of the supplied extents is not a \
+            DataSymbol, int or ArrayType.Extent.
+
+        '''
+        from psyclone.psyir.symbols.datasymbol import DataSymbol
+        if not isinstance(extents, list):
+            raise TypeError(
+                "ArrayType 'shape' must be of type list but found '{0}'."
+                "".format(type(extents).__name__))
+
+        for dimension in extents:
+            if isinstance(dimension, DataSymbol):
+                if isinstance(dimension.datatype, (UnknownType, DeferredType)):
+                    # We allow symbols of Unknown or Deferred Type
+                    continue
+                if not (dimension.is_scalar and
+                        dimension.datatype.intrinsic ==
+                        ScalarType.Intrinsic.INTEGER):
+                    raise TypeError(
+                        "DataSymbols that are part of another symbol shape can"
+                        " only be scalar integers, but found '{0}'."
+                        "".format(str(dimension)))
+            elif not isinstance(dimension, (self.Extent, int)):
+                raise TypeError(
+                    "DataSymbol shape list elements can only be "
+                    "'DataSymbol', 'integer' or ArrayType.Extent, but "
+                    "found '{0}'.".format(type(dimension).__name__))
+
     def __str__(self):
         '''
         :returns: a description of this array datatype.


Test patch:
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py
index 4f46db84a..ee73914d1 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py
@@ -50,7 +50,7 @@ from psyclone.psyGen import PSyFactory, Directive, KernelSchedule
 from psyclone.errors import InternalError, GenerationError
 from psyclone.psyir.symbols import DataSymbol, ContainerSymbol, SymbolTable, \
     ArgumentInterface, SymbolError, ScalarType, ArrayType, INTEGER_TYPE, \
-    REAL_TYPE, UnknownType, Symbol
+    REAL_TYPE, UnknownType, DeferredType, Symbol, UnresolvedInterface
 from psyclone.psyir.frontend.fparser2 import Fparser2Reader, \
     _get_symbol_table, _is_array_range_literal, _is_bound_full_extent, \
     _is_range_full_extent, _check_args, default_precision, \
@@ -937,6 +937,35 @@ def test_process_array_declarations():
     assert symbol.shape == [ArrayType.Extent.ATTRIBUTE,
                             ArrayType.Extent.ATTRIBUTE]
 
+    # Extent given by variable with UnknownType
+    udim = DataSymbol("udim", UnknownType("integer :: udim"))
+    fake_parent.symbol_table.add(udim)
+    reader = FortranStringReader("integer :: l11(udim)")
+    fparser2spec = Specification_Part(reader).content[0]
+    processor.process_declarations(fake_parent, [fparser2spec], [])
+    symbol = fake_parent.symbol_table.lookup("l11")
+    assert symbol.name == "l11"
+    assert len(symbol.shape) == 1
+    # Extent symbol should be udim
+    assert symbol.shape[0].name == "udim"
+    assert symbol.shape[0] is udim
+    assert isinstance(symbol.shape[0].datatype, UnknownType)
+
+    # Extent given by variable with DeferredType
+    ddim = DataSymbol("ddim", DeferredType(),
+                      interface=UnresolvedInterface())
+    fake_parent.symbol_table.add(ddim)
+    reader = FortranStringReader("integer :: l12(ddim)")
+    fparser2spec = Specification_Part(reader).content[0]
+    processor.process_declarations(fake_parent, [fparser2spec], [])
+    symbol = fake_parent.symbol_table.lookup("l12")
+    assert symbol.name == "l12"
+    assert len(symbol.shape) == 1
+    # Extent symbol should now be ddim
+    assert symbol.shape[0].name == "ddim"
+    assert symbol.shape[0] is ddim
+    assert isinstance(symbol.shape[0].datatype, DeferredType)
+
 
 @pytest.mark.usefixtures("f2008_parser")
 def test_process_not_supported_declarations():
@@ -1525,7 +1554,6 @@ def test_deferred_array_size():
 def test_unresolved_array_size():
     ''' Check that we handle the case where we do not find an explicit
     declaration of a symbol used in the definition of an array extent. '''
-    from psyclone.psyir.symbols import UnresolvedInterface
     fake_parent = KernelSchedule("dummy_schedule")
     processor = Fparser2Reader()
     reader = FortranStringReader("real, dimension(n) :: array3")
diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py
index 6c96b8823..27c406dca 100644
--- a/src/psyclone/tests/psyir/symbols/datatype_test.py
+++ b/src/psyclone/tests/psyir/symbols/datatype_test.py
@@ -222,7 +222,7 @@ def test_arraytype_invalid_shape():
     scalar_type = ScalarType(ScalarType.Intrinsic.REAL, 4)
     with pytest.raises(TypeError) as excinfo:
         _ = ArrayType(scalar_type, None)
-    assert ("ArrayType expected 'shape' argument to be of type list but "
+    assert ("ArrayType 'shape' must be of type list but "
             "found 'NoneType'." in str(excinfo.value))