Repository: stfc/PSyclone (python)

Issue:
NanTestTrans fails if applied to a 'null' loop
We get an error saying that the loop variable is `None`. Without looking at the code, I think this case should probably be supported so I'm marking this as a bug.

Pull request:
#2012 Fix crash in access information collection when an LFRic domain loop is used.
This should fix #2012 . I looked at a few options of implementing this, but in `loop.py`, there is no `loop_type` or `iteration_space` defined that I could use (some tests failed because they did pass in a  plain `Loop`, not a `PSyLoop`). And I didn't want to test `isinstance(self, DynLoop)` or so in the base class `loop.py`. Only alternative would be to re-implement the `reference_access` in `DynLoop`, but this leads to some bad code duplication (the handling of the loop body would then need to be done in `DynLoop` and some other classes - PSyLoop or Loop or GOLoop/NemoLoop ... ). In short, I couldn't make up my mind about the best way, and this way should be safe, and has no code duplication. Let me know if you prefer a different implementation.

Patch:
diff --git a/changelog b/changelog
index 47ded6741..9eb0f69d6 100644
--- a/changelog
+++ b/changelog
@@ -270,6 +270,9 @@
 	92) PR #1976 for #1974. Fixes issues with literal constants and
 	array accesses in VariableAccessInfo for the LFRic API.
 
+	93) PR #2015 for #2012. Fixes crash in access-information
+	collection when there is an LFRic 'domain' loop.
+
 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/nodes/loop.py b/src/psyclone/psyir/nodes/loop.py
index 057c98960..9b9076269 100644
--- a/src/psyclone/psyir/nodes/loop.py
+++ b/src/psyclone/psyir/nodes/loop.py
@@ -1,7 +1,7 @@
 # -----------------------------------------------------------------------------
 # BSD 3-Clause License
 #
-# Copyright (c) 2017-2022, Science and Technology Facilities Council.
+# Copyright (c) 2017-2023, Science and Technology Facilities Council.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -155,6 +155,7 @@ class Loop(Statement):
 
     @classmethod
     def create(cls, variable, start, stop, step, children):
+        # pylint: disable=too-many-arguments
         '''Create a Loop instance given valid instances of a variable,
         start, stop and step nodes, and a list of child nodes for the
         loop body.
@@ -351,20 +352,24 @@ class Loop(Statement):
             :py:class:`psyclone.core.access_info.VariablesAccessInfo`
         '''
 
-        # It is important to first add the WRITE access, since this way
-        # the dependency analysis for declaring openmp private variables
-        # will automatically declare the loop variables to be private
-        # (write access before read)
-        var_accesses.add_access(Signature(self.variable.name),
-                                AccessType.WRITE, self)
-        var_accesses.add_access(Signature(self.variable.name),
-                                AccessType.READ, self)
-
-        # Accesses of the start/stop/step expressions
-        self.start_expr.reference_accesses(var_accesses)
-        self.stop_expr.reference_accesses(var_accesses)
-        self.step_expr.reference_accesses(var_accesses)
-        var_accesses.next_location()
+        # Only add the loop variable and start/stop/step values if this is
+        # not an LFRic domain loop. We need to access the variable directly
+        # to avoid a crash in the getter if the loop variable is not defined.
+        if self._variable:
+            # It is important to first add the WRITE access, since this way
+            # the dependency analysis for declaring openmp private variables
+            # will automatically declare the loop variables to be private
+            # (write access before read)
+            var_accesses.add_access(Signature(self.variable.name),
+                                    AccessType.WRITE, self)
+            var_accesses.add_access(Signature(self.variable.name),
+                                    AccessType.READ, self)
+
+            # Accesses of the start/stop/step expressions
+            self.start_expr.reference_accesses(var_accesses)
+            self.stop_expr.reference_accesses(var_accesses)
+            self.step_expr.reference_accesses(var_accesses)
+            var_accesses.next_location()
 
         for child in self.loop_body.children:
             child.reference_accesses(var_accesses)


Test patch:
diff --git a/src/psyclone/tests/core/access_info_test.py b/src/psyclone/tests/core/access_info_test.py
index 0a3ce2cbf..8668945b6 100644
--- a/src/psyclone/tests/core/access_info_test.py
+++ b/src/psyclone/tests/core/access_info_test.py
@@ -1,7 +1,7 @@
 # -----------------------------------------------------------------------------
 # BSD 3-Clause License
 #
-# Copyright (c) 2019-2022, Science and Technology Facilities Council.
+# Copyright (c) 2019-2023, Science and Technology Facilities Council.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -685,6 +685,19 @@ def test_variables_access_info_shape_bounds(fortran_reader, function):
     assert str(vai) == "a: READ, n: WRITE"
 
 
+# -----------------------------------------------------------------------------
+def test_variables_access_info_domain_loop():
+    '''Tests that LFRic domain loop (that do not have an actual loop
+    structure, so especially the loop variable is not defined) work as
+    expected.
+    '''
+    _, invoke = get_invoke("25.1_kern_two_domain.f90", "dynamo0.3", idx=0)
+    vai = VariablesAccessInfo(invoke.schedule)
+    assert str(vai) == ("a: READ, b: READ, f1: READWRITE, f2: READWRITE, "
+                        "map_w3: READ, ncell_2d_no_halos: READ, ndf_w3: READ, "
+                        "nlayers: READ, undf_w3: READ")
+
+
 # -----------------------------------------------------------------------------
 def test_lfric_access_info():
     '''Test some LFRic specific potential bugs: