Repository: stfc/PSyclone (python)

Issue:
[NEMO] Implicitly-declared subroutine arguments are lost
Chris has found that if PSyclone encounters code such as:

    module my_mod

    contains

    subroutine my_sub(iarg)
      write(*,*) iarg
    end subroutine my_sub

    end module my_mod

then PSyclone loses the `iarg` argument to the subroutine (presumably because it is never explicitly declared) and the resulting code is wrong.

Pull request description:
(Closes #1891) Catch implicit subroutine args and abort
A small PR that just tightens-up the fparser2 subroutine handler to make sure that we catch a subroutine that has arguments but no declarations. Since PSyclone does not support implicitly-declared data, we now raise an InternalError in this situation.

Patch:
diff --git a/changelog b/changelog
index e438ccb01..c96b36fa9 100644
--- a/changelog
+++ b/changelog
@@ -328,6 +328,9 @@
 	110) PR #2066 for #1587. Introduces new DebugWriter visitor for
 	fast generation of Fortran-like content in error messages.
 
+	111) PR #1914 for #1891. Improves error handling when there are
+	undeclared arguments.
+
 release 2.3.1 17th of June 2022
 
 	1) PR #1747 for #1720. Adds support for If blocks to PSyAD.
diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py
index d7d42b5db..81a1764a6 100644
--- a/src/psyclone/psyir/frontend/fparser2.py
+++ b/src/psyclone/psyir/frontend/fparser2.py
@@ -2103,10 +2103,11 @@ class Fparser2Reader():
             decls_str_list = [str(node) for node in nodes]
             arg_str_list = [arg.string.lower() for arg in arg_list]
             raise InternalError(
-                f"The kernel argument list:\n'{arg_str_list}'\n"
-                f"does not match the variable declarations:\n"
+                f"The argument list {arg_str_list} for routine '{parent.name}'"
+                f" does not match the variable declarations:\n"
                 f"{os.linesep.join(decls_str_list)}\n"
-                f"Specific PSyIR error is {str(info)}.") from info
+                f"(Note that PSyclone does not support implicit declarations.)"
+                f" Specific PSyIR error is {info}.") from info
 
         # fparser2 does not always handle Statement Functions correctly, this
         # loop checks for Stmt_Functions that should be an array statement
@@ -3979,22 +3980,23 @@ class Fparser2Reader():
             sub_spec = _first_type_match(node.content,
                                          Fortran2003.Specification_Part)
             decl_list = sub_spec.content
-            # TODO this if test can be removed once fparser/#211 is fixed
-            # such that routine arguments are always contained in a
-            # Dummy_Arg_List, even if there's only one of them.
-            if isinstance(node, (Fortran2003.Subroutine_Subprogram,
-                                 Fortran2003.Function_Subprogram)) and \
-               isinstance(node.children[0].children[2],
-                          Fortran2003.Dummy_Arg_List):
-                arg_list = node.children[0].children[2].children
-            else:
-                # Routine has no arguments
-                arg_list = []
         except ValueError:
             # Subroutine has no Specification_Part so has no
-            # declarations. Continue with empty lists.
+            # declarations. Continue with empty list.
             decl_list = []
+
+        # TODO this if test can be removed once fparser/#211 is fixed
+        # such that routine arguments are always contained in a
+        # Dummy_Arg_List, even if there's only one of them.
+        if isinstance(node, (Fortran2003.Subroutine_Subprogram,
+                             Fortran2003.Function_Subprogram)) and \
+           isinstance(node.children[0].children[2],
+                      Fortran2003.Dummy_Arg_List):
+            arg_list = node.children[0].children[2].children
+        else:
+            # Routine has no arguments
             arg_list = []
+
         self.process_declarations(routine, decl_list, arg_list)
 
         # If this is a function then work out the return type


Test patch:
diff --git a/src/psyclone/tests/generator_test.py b/src/psyclone/tests/generator_test.py
index 1f808c19a..fd35e164d 100644
--- a/src/psyclone/tests/generator_test.py
+++ b/src/psyclone/tests/generator_test.py
@@ -350,15 +350,16 @@ def test_kernel_parsing_internalerror(capsys):
     assert out == ""
     assert "In kernel file " in str(err)
     assert (
-        "PSyclone internal error: The kernel argument list:\n"
-        "'['i', 'j', 'cu', 'p', 'u']'\n"
-        "does not match the variable declarations:\n"
+        "PSyclone internal error: The argument list ['i', 'j', 'cu', 'p', "
+        "'u'] for routine 'compute_code' does not match the variable "
+        "declarations:\n"
         "IMPLICIT NONE\n"
         "INTEGER, INTENT(IN) :: I, J\n"
         "REAL(KIND = go_wp), INTENT(OUT), DIMENSION(:, :) :: cu\n"
         "REAL(KIND = go_wp), INTENT(IN), DIMENSION(:, :) :: p\n"
-        "Specific PSyIR error is \"Could not find 'u' in the Symbol "
-        "Table.\".\n" in str(err))
+        "(Note that PSyclone does not support implicit declarations.) Specific"
+        " PSyIR error is \"Could not find 'u' in the Symbol Table.\".\n"
+        in str(err))
 
 
 def test_script_file_too_short():
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py
index 5384b2e5a..f7278cd86 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py
@@ -42,45 +42,41 @@ to PSyIR.
 import pytest
 
 from fparser.common.readfortran import FortranStringReader
-from psyclone.psyir.symbols import (DataSymbol, ScalarType, UnknownFortranType,
-                                    RoutineSymbol)
-from psyclone.psyir.nodes import Container, Routine, CodeBlock, FileContainer
+from psyclone.errors import InternalError
 from psyclone.psyir.frontend.fparser2 import (Fparser2Reader,
                                               TYPE_MAP_FROM_FORTRAN)
+from psyclone.psyir.nodes import Container, Routine, CodeBlock, FileContainer
+from psyclone.psyir.symbols import (DataSymbol, ScalarType, UnknownFortranType,
+                                    RoutineSymbol)
 
+IN_OUTS = []
 # subroutine no declarations
-SUB1_IN = (
-    "subroutine sub1()\n"
-    "end subroutine\n")
-SUB1_OUT = (
-    "subroutine sub1()\n\n\n"
-    "end subroutine sub1\n")
+IN_OUTS.append(
+    (("subroutine sub1()\n"
+      "end subroutine\n"),
+     ("subroutine sub1()\n\n\n"
+      "end subroutine sub1\n")))
 # subroutine with symbols/declarations
-SUB2_IN = (
-    "subroutine sub1(a)\n"
-    "real :: a\n"
-    "end subroutine\n")
-SUB2_OUT = (
-    "subroutine sub1(a)\n"
-    "  real :: a\n\n\n"
-    "end subroutine sub1\n")
+IN_OUTS.append(
+    (("subroutine sub1(a)\n"
+      "real :: a\n"
+      "end subroutine\n"),
+     ("subroutine sub1(a)\n"
+      "  real :: a\n\n\n"
+      "end subroutine sub1\n")))
 # subroutine with executable content
-SUB3_IN = (
-    "subroutine sub1()\n"
-    "real :: a\n"
-    "a=0.0\n"
-    "end subroutine\n")
-SUB3_OUT = (
-    "subroutine sub1()\n"
-    "  real :: a\n\n"
-    "  a = 0.0\n\n"
-    "end subroutine sub1\n")
+IN_OUTS.append(
+    (("subroutine sub1()\n"
+      "real :: a\n"
+      "a=0.0\n"
+      "end subroutine\n"),
+     ("subroutine sub1()\n"
+      "  real :: a\n\n"
+      "  a = 0.0\n\n"
+      "end subroutine sub1\n")))
 
 
-@pytest.mark.parametrize("code,expected",
-                         [(SUB1_IN, SUB1_OUT),
-                          (SUB2_IN, SUB2_OUT),
-                          (SUB3_IN, SUB3_OUT)])
+@pytest.mark.parametrize("code,expected", IN_OUTS)
 def test_subroutine_handler(parser, fortran_writer, code, expected):
     '''Test that subroutine_handler handles valid Fortran subroutines.'''
 
@@ -96,6 +92,45 @@ def test_subroutine_handler(parser, fortran_writer, code, expected):
     assert expected == result
 
 
+def test_subroutine_implicit_args(parser):
+    """Check that we raise the expected error when we encounter a
+    subroutine argument without an explicit declaration.
+
+    """
+    code = '''
+subroutine sub1(idx)
+end subroutine'''
+    processor = Fparser2Reader()
+    reader = FortranStringReader(code)
+    parse_tree = parser(reader)
+    subroutine = parse_tree.children[0]
+    with pytest.raises(InternalError) as err:
+        _ = processor._subroutine_handler(subroutine, None)
+    assert ("The argument list ['idx'] for routine 'sub1' does not match "
+            "the variable declarations:" in str(err.value))
+
+
+def test_subroutine_some_implicit_args(parser):
+    """Check that we raise the expected error when we encounter a
+    subroutine which has declarations but has omitted to declare one
+    of its arguments.
+
+    """
+    code = '''
+subroutine sub1(var, idx)
+    real, intent(in) :: var
+end subroutine'''
+    processor = Fparser2Reader()
+    reader = FortranStringReader(code)
+    parse_tree = parser(reader)
+    subroutine = parse_tree.children[0]
+    with pytest.raises(InternalError) as err:
+        _ = processor._subroutine_handler(subroutine, None)
+    err_msg = str(err.value)
+    assert "The argument list ['var', 'idx'] for routine 'sub1'" in err_msg
+    assert "Could not find 'idx' in the Symbol Table" in err_msg
+
+
 def test_function_handler(fortran_reader, fortran_writer):
     '''Test that subroutine_handler correctly handles a function defined
     within a module.
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py
index 1a27ec2d7..124eb79cd 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py
@@ -626,12 +626,13 @@ def test_get_routine_schedules_unmatching_arguments(parser):
     # Test exception for unmatching argument list
     with pytest.raises(InternalError) as error:
         _ = processor.get_routine_schedules("dummy_code", ast)
-    assert ("PSyclone internal error: The kernel argument list:\n"
-            "'['f1', 'f2', 'f3', 'f4']'\n"
-            "does not match the variable declarations:\n"
+    assert ("PSyclone internal error: The argument list ['f1', 'f2', 'f3', "
+            "'f4'] for routine 'dummy_code' does not match the variable "
+            "declarations:\n"
             "REAL(KIND = wp), DIMENSION(:, :), INTENT(IN) :: f1\n"
             "REAL(KIND = wp), DIMENSION(:, :), INTENT(OUT) :: f2\n"
             "REAL(KIND = wp), DIMENSION(:, :) :: f3\n"
+            "(Note that PSyclone does not support implicit declarations.) "
             "Specific PSyIR error is \"Could not find 'f4' in the "
             "Symbol Table.\"." in str(error.value))