Repository: stfc/PSyclone

Issue:
[PSyIR-FE] Bare 'DO' causes crash in frontend
PSyclone internal error: Unrecognised form of DO loop - failed to find Loop_Control element in the node 

    DO
        IF(..) EXIT
        ...
    END DO

This should result in a code block, not a crash.

Discussion:

Patch:
diff --git a/changelog b/changelog
index f8cffbb72..3fa24aacc 100644
--- a/changelog
+++ b/changelog
@@ -29,6 +29,8 @@
 	12) #794 for #788. Re-structuring and tidying of DynArgDescriptor03
 	to become LFRicArgDescriptor.
 
+	13) #807 for #805. Put fparser2 bare DO constructs inside CodeBlocks.
+
 release 1.9.0 20th May 2020
 
 	1) #602 for #597. Modify Node.ancestor() to optionally include
diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py
index 3df64b171..7e270efcc 100644
--- a/src/psyclone/psyir/frontend/fparser2.py
+++ b/src/psyclone/psyir/frontend/fparser2.py
@@ -1769,14 +1769,13 @@ class Fparser2Reader(object):
         :returns: PSyIR representation of node
         :rtype: :py:class:`psyclone.psyir.nodes.Loop`
 
-        :raises InternalError: if the fparser2 tree has an unexpected \
-            structure.
+        :raises NotImplementedError: if the fparser2 tree has an unsupported \
+            structure (e.g. DO WHILE or a DO with no loop control).
         '''
         ctrl = walk(node.content, Fortran2003.Loop_Control)
         if not ctrl:
-            raise InternalError(
-                "Unrecognised form of DO loop - failed to find Loop_Control "
-                "element in the node '{0}'.".format(str(node)))
+            # TODO #359 a DO with no loop control is put into a CodeBlock
+            raise NotImplementedError()
         if ctrl[0].items[0]:
             # If this is a DO WHILE then the first element of items will not
             # be None. (See `fparser.two.Fortran2003.Loop_Control`.)


Test patch:
diff --git a/src/psyclone/tests/psyir/frontend/bound_intrinsic_test.py b/src/psyclone/tests/psyir/frontend/fparser2_bound_intrinsic_test.py
similarity index 100%
rename from src/psyclone/tests/psyir/frontend/bound_intrinsic_test.py
rename to src/psyclone/tests/psyir/frontend/fparser2_bound_intrinsic_test.py
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_do_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_do_handler_test.py
new file mode 100644
index 000000000..3dfca2c45
--- /dev/null
+++ b/src/psyclone/tests/psyir/frontend/fparser2_do_handler_test.py
@@ -0,0 +1,123 @@
+# -----------------------------------------------------------------------------
+# BSD 3-Clause License
+#
+# Copyright (c) 2020, Science and Technology Facilities Council.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# * Redistributions of source code must retain the above copyright notice, this
+#   list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright notice,
+#   this list of conditions and the following disclaimer in the documentation
+#   and/or other materials provided with the distribution.
+#
+# * Neither the name of the copyright holder nor the names of its
+#   contributors may be used to endorse or promote products derived from
+#   this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+# -----------------------------------------------------------------------------
+# Author A. R. Porter, STFC Daresbury Lab
+
+''' Module containing pytest tests for the handling of the DO
+construct in the PSyIR fparser2 frontend. '''
+
+from __future__ import absolute_import
+
+import pytest
+from fparser.common.readfortran import FortranStringReader
+from fparser.two import Fortran2003
+from psyclone.psyir.nodes import Schedule, CodeBlock, Loop, Assignment
+from psyclone.psyir.frontend.fparser2 import Fparser2Reader
+
+
+def test_handling_end_do_stmt(parser):
+    ''' Test that the fparser2 End_Do_Stmt is ignored.'''
+    reader = FortranStringReader('''
+      subroutine test()
+        integer :: i, a
+        do i=1,10
+            a=a+1
+        end do
+      end subroutine test
+        ''')
+    fparser2_tree = parser(reader)
+    processor = Fparser2Reader()
+    result = processor.generate_schedule("test", fparser2_tree)
+    assert len(result.children) == 1  # Just the loop (no end statement)
+
+
+def test_do_construct(parser):
+    ''' Check that do loop constructs are converted to the expected
+    PSyIR node. '''
+    reader = FortranStringReader('''
+      subroutine test()
+        integer :: i, sum
+        do i = 1, 10 , 2
+            sum = sum + i
+        end do
+      end subroutine test
+      ''')
+    fparser2_tree = parser(reader)
+    processor = Fparser2Reader()
+    result = processor.generate_schedule("test", fparser2_tree)
+    assert result.children[0]
+    new_loop = result.children[0]
+    assert isinstance(new_loop, Loop)
+    assert new_loop.variable.name == "i"
+    assert new_loop.start_expr.value == "1"
+    assert new_loop.stop_expr.value == "10"
+    assert new_loop.step_expr.value == "2"
+    assert len(new_loop.loop_body.children) == 1
+    assert isinstance(new_loop.loop_body[0], Assignment)
+
+
+@pytest.mark.usefixtures("f2008_parser")
+def test_do_construct_while():
+    ''' Check that do while constructs are placed in Codeblocks. '''
+    reader = FortranStringReader('''
+        do while(a .gt. b)\n
+            c = c + 1\n
+        end do\n
+        ''')
+    fparser2while = Fortran2003.Execution_Part.match(reader)[0][0]
+    processor = Fparser2Reader()
+    fake_parent = Schedule()
+    processor.process_nodes(fake_parent, [fparser2while])
+    assert isinstance(fake_parent.children[0], CodeBlock)
+
+
+@pytest.mark.usefixtures("parser")
+def test_unhandled_do(parser):
+    ''' Test that a DO without any control logic results in a CodeBlock. '''
+    lines = ["SUBROUTINE a_loop()",
+             "  integer :: niter_atgen, jp_maxniter_atgen",
+             "  real :: zh, zh_prev",
+             "  DO",
+             "    IF (niter_atgen >= jp_maxniter_atgen) THEN",
+             "      zh = - 1._wp",
+             "      EXIT",
+             "    END IF",
+             "    zh_prev = zh",
+             "    niter_atgen = niter_atgen + 1",
+             "  END DO",
+             "END SUBROUTINE a_loop"]
+    reader = FortranStringReader("\n".join(lines))
+    fp2spec = parser(reader)
+    processor = Fparser2Reader()
+    sched = processor.generate_schedule("a_loop", fp2spec)
+    assert isinstance(sched[0], CodeBlock)
diff --git a/src/psyclone/tests/psyir/frontend/fortran_use_test.py b/src/psyclone/tests/psyir/frontend/fparser2_fortran_use_test.py
similarity index 100%
rename from src/psyclone/tests/psyir/frontend/fortran_use_test.py
rename to src/psyclone/tests/psyir/frontend/fparser2_fortran_use_test.py
diff --git a/src/psyclone/tests/psyir/frontend/literals_test.py b/src/psyclone/tests/psyir/frontend/fparser2_literals_test.py
similarity index 100%
rename from src/psyclone/tests/psyir/frontend/literals_test.py
rename to src/psyclone/tests/psyir/frontend/fparser2_literals_test.py
diff --git a/src/psyclone/tests/psyir/frontend/size_intrinsic_test.py b/src/psyclone/tests/psyir/frontend/fparser2_size_intrinsic_test.py
similarity index 100%
rename from src/psyclone/tests/psyir/frontend/size_intrinsic_test.py
rename to src/psyclone/tests/psyir/frontend/fparser2_size_intrinsic_test.py
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py
index d9992c41d..4f46db84a 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py
@@ -51,7 +51,6 @@ from psyclone.errors import InternalError, GenerationError
 from psyclone.psyir.symbols import DataSymbol, ContainerSymbol, SymbolTable, \
     ArgumentInterface, SymbolError, ScalarType, ArrayType, INTEGER_TYPE, \
     REAL_TYPE, UnknownType, Symbol
-from psyclone.psyir.nodes import Loop
 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, \
@@ -2688,22 +2687,6 @@ def test_handling_return_stmt():
     assert not new_node.children
 
 
-def test_handling_end_do_stmt(parser):
-    ''' Test that the fparser2 End_Do_Stmt is ignored.'''
-    reader = FortranStringReader('''
-      subroutine test()
-        integer :: i, a
-        do i=1,10
-            a=a+1
-        end do
-      end subroutine test
-        ''')
-    fparser2_tree = parser(reader)
-    processor = Fparser2Reader()
-    result = processor.generate_schedule("test", fparser2_tree)
-    assert len(result.children) == 1  # Just the loop (no end statement)
-
-
 @pytest.mark.usefixtures("f2008_parser")
 def test_handling_end_subroutine_stmt():
     ''' Test that fparser2 End_Subroutine_Stmt are ignored.'''
@@ -2720,48 +2703,6 @@ def test_handling_end_subroutine_stmt():
     assert not fake_parent.children  # No new children created
 
 
-def test_do_construct(parser):
-    ''' Check that do loop constructs are converted to the expected
-    PSyIR node.
-
-    '''
-    reader = FortranStringReader('''
-      subroutine test()
-        integer :: i, sum
-        do i = 1, 10 , 2
-            sum = sum + i
-        end do
-      end subroutine test
-      ''')
-    fparser2_tree = parser(reader)
-    processor = Fparser2Reader()
-    result = processor.generate_schedule("test", fparser2_tree)
-    assert result.children[0]
-    new_loop = result.children[0]
-    assert isinstance(new_loop, Loop)
-    assert new_loop.variable.name == "i"
-    assert new_loop.start_expr.value == "1"
-    assert new_loop.stop_expr.value == "10"
-    assert new_loop.step_expr.value == "2"
-    assert len(new_loop.loop_body.children) == 1
-    assert isinstance(new_loop.loop_body[0], Assignment)
-
-
-@pytest.mark.usefixtures("f2008_parser")
-def test_do_construct_while():
-    ''' Check that do while constructs are placed in Codeblocks '''
-    reader = FortranStringReader('''
-        do while(a .gt. b)\n
-            c = c + 1\n
-        end do\n
-        ''')
-    fparser2while = Execution_Part.match(reader)[0][0]
-    processor = Fparser2Reader()
-    fake_parent = Schedule()
-    processor.process_nodes(fake_parent, [fparser2while])
-    assert isinstance(fake_parent.children[0], CodeBlock)
-
-
 # (1/4) fparser2reader::nodes_to_code_block
 def test_nodes_to_code_block_1(f2008_parser):
     '''Check that a statement codeblock that is at the "top level" in the
@@ -2843,36 +2784,6 @@ def test_nodes_to_code_block_4():
             in str(excinfo.value))
 
 
-@pytest.mark.usefixtures("f2008_parser")
-def test_missing_loop_control(monkeypatch):
-    ''' Check that encountering a loop in the fparser parse tree that is
-    missing a Loop_Control element raises an InternalError. '''
-    from fparser.two.utils import walk
-    reader = FortranStringReader('''
-        do while(a .gt. b)\n
-            c = c + 1\n
-        end do\n
-        ''')
-    fparser2while = Fortran2003.Execution_Part.match(reader)[0][0]
-    processor = Fparser2Reader()
-
-    # We have to break the fparser2 parse tree in order to trigger the
-    # internal error
-    ctrl = walk(fparser2while.content[0].items, Fortran2003.Loop_Control)
-    # 'items' is a tuple and therefore immutable so make a new list
-    item_list = list(fparser2while.content[0].items)
-    # Create a new tuple for the items member without the Loop_Control
-    item_list.remove(ctrl[0])
-    fparser2while.content[0].items = tuple(item_list)
-    monkeypatch.setattr(fparser2while, "tostr", lambda: "<fparser2while>")
-
-    fake_parent = Schedule()
-    with pytest.raises(InternalError) as err:
-        processor.process_nodes(fake_parent, [fparser2while])
-    assert "Unrecognised form of DO loop - failed to find Loop_Control " \
-        "element in the node '<fparser2while>'." in str(err.value)
-
-
 def test_get_symbol_table():
     '''Test that the utility function _get_symbol_table() works and fails
     as expected. '''