Repository: stfc/PSyclone

Issue:
Unary operator output without brackets
The following invalid code is being output by the Fortran Writer

`d(j) = d(j) + -a(i) * y`

There should be brackets around the unary '-' operator.

The PSyIR looks like:

```
Assignment[]
    ArrayReference[name:'d']
        Reference[name:'j']
    BinaryOperation[operator:'ADD']
        ArrayReference[name:'d']
            Reference[name:'j']
        BinaryOperation[operator:'MUL']
            UnaryOperation[operator:'MINUS']
                ArrayReference[name:'a']
                    Reference[name:'i']
            Reference[name:'y']
```


Discussion:

Patch:
diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py
index d1dccfe2f..f373830f1 100644
--- a/src/psyclone/psyir/backend/fortran.py
+++ b/src/psyclone/psyir/backend/fortran.py
@@ -34,6 +34,7 @@
 # Authors R. W. Ford and S. Siso, STFC Daresbury Lab
 # Modified J. Henrichs, Bureau of Meteorology
 # Modified A. R. Porter, A. B. G. Chalk and N. Nobre, STFC Daresbury Lab
+# Modified J. Remy, Université Grenoble Alpes, Inria
 
 '''PSyIR Fortran backend. Implements a visitor that generates Fortran code
 from a PSyIR tree. '''
@@ -1497,13 +1498,32 @@ class FortranWriter(LanguageWriter):
             fort_oper = self.get_operator(node.operator)
             # If the parent node is a UnaryOperation or a BinaryOperation
             # such as '-' or '**' then we need parentheses. This ensures we
-            # don't generate invalid Fortran such as 'a ** -b' or 'a - -b'.
+            # don't generate invalid Fortran such as 'a ** -b', 'a - -b' or
+            # '-a ** b' which is '-(a ** b)' instead of intended '(-a) ** b'.
+            # Also consider the grandparent node to avoid generating invalid
+            # Fortran such as 'a + -b * c' instead of intended 'a + (-b) * c'.
             parent = node.parent
             if isinstance(parent, UnaryOperation):
                 return f"({fort_oper}{content})"
             if isinstance(parent, BinaryOperation):
-                if node is parent.children[1]:
+                parent_fort_oper = self.get_operator(parent.operator)
+                if (node is parent.children[1] or
+                        (parent_fort_oper == "**" and fort_oper == "-")):
                     return f"({fort_oper}{content})"
+                grandparent = parent.parent
+                # Case: 'a op1 (-b) op2 c'
+                # and precedence(op2) > precedence(op1)
+                # implying that '(-b) op2 c' is not parenthesized.
+                if isinstance(grandparent, BinaryOperation):
+                    grandparent_fort_oper = self.get_operator(
+                        grandparent.operator
+                    )
+                    if (parent is grandparent.children[1]
+                        and node is parent.children[0]
+                        and (precedence(parent_fort_oper)
+                             > precedence(grandparent_fort_oper))
+                            and fort_oper == "-"):
+                        return f"({fort_oper}{content})"
             return f"{fort_oper}{content}"
 
         except KeyError as error:


Test patch:
diff --git a/src/psyclone/tests/psyad/transformations/test_assignment_trans.py b/src/psyclone/tests/psyad/transformations/test_assignment_trans.py
index 15201f9fe..0f1e69cec 100644
--- a/src/psyclone/tests/psyad/transformations/test_assignment_trans.py
+++ b/src/psyclone/tests/psyad/transformations/test_assignment_trans.py
@@ -32,6 +32,7 @@
 # -----------------------------------------------------------------------------
 # Authors: R. W. Ford, A. R. Porter, N. Nobre and S. Siso, STFC Daresbury Lab
 # Modified by J. Henrichs, Bureau of Meteorology
+# Modified by J. Remy, Université Grenoble Alpes, Inria
 
 '''Module to test the psyad assignment transformation.'''
 
@@ -565,8 +566,6 @@ def test_single_valued_sub(tmpdir):
     check_adjoint(tl_fortran, active_variables, ad_fortran, tmpdir)
 
 
-@pytest.mark.xfail(reason="issue #1333 Unary '-' should be enclosed in "
-                   "brackets.")
 def test_multi_valued_sub(tmpdir):
     '''Test the transformation works when there are multiple active
     variable on the rhs (B, C, D) that have unary plus and minus
diff --git a/src/psyclone/tests/psyir/backend/fortran_test.py b/src/psyclone/tests/psyir/backend/fortran_test.py
index b8ebca14c..f701ce766 100644
--- a/src/psyclone/tests/psyir/backend/fortran_test.py
+++ b/src/psyclone/tests/psyir/backend/fortran_test.py
@@ -32,7 +32,8 @@
 # POSSIBILITY OF SUCH DAMAGE.
 # -----------------------------------------------------------------------------
 # Author R. W. Ford, STFC Daresbury Lab
-# Modified by A. R. Porter and S. Siso, STFC Daresbury Lab
+# Modified by A. R. Porter and S. Siso, STFC Daresbury Lab,
+# Modified by J. Remy, Université Grenoble Alpes, Inria
 # -----------------------------------------------------------------------------
 
 '''Performs pytest tests on the psyclone.psyir.backend.fortran module'''
@@ -1131,6 +1132,8 @@ def test_fw_mixed_operator_precedence(fortran_reader, fortran_writer, tmpdir):
         "    a = b**(-b + c)\n"
         "    a = (-b)**c\n"
         "    a = -(-b)\n"
+        "    a = b * (-c) + d\n"
+        "    a = b + (-c) * d\n"
         "end subroutine tmp\n"
         "end module test")
     schedule = fortran_reader.psyir_from_source(code)
@@ -1147,8 +1150,10 @@ def test_fw_mixed_operator_precedence(fortran_reader, fortran_writer, tmpdir):
         "    a = LOG(b * c)\n"
         "    a = b ** (-c)\n"
         "    a = b ** (-b + c)\n"
-        "    a = -b ** c\n"
-        "    a = -(-b)\n")
+        "    a = (-b) ** c\n"
+        "    a = -(-b)\n"
+        "    a = b * (-c) + d\n"
+        "    a = b + (-c) * d\n")
     assert expected in result
     assert Compile(tmpdir).string_compiles(result)