Repository: stfc/PSyclone

Issue:
Swapping of symbols is case sensitive (and shouldn't be).
As reported by Simon:

     $ psyclone -api nemo -l output src/TOP/trc.F90
      Error, unexpected exception, please report to the authors:
      Description ...
      PSyclone SymbolTable error: Error when generating Container for module 'trc': Cannot swap symbols that have different names, got: 'obc_data' and 'OBC_DATA'
      Type ...
      <class 'psyclone.psyir.symbols.symbol.SymbolError'>
      Stacktrace ...
      [Stacktrace omitted]

When I rename 'OBC_DATA' to 'obc_data' in trc.F90, PSyclone seems to process the file well.


Discussion:

Patch:
diff --git a/changelog b/changelog
index 3800fe5a3..6938211e2 100644
--- a/changelog
+++ b/changelog
@@ -434,6 +434,9 @@
         140) PR #1144 for #1136. Add checks that PSyIR nodes are only children
         of one parent.
 
+	141) PR #1150 for #1146. Fixed bug in PSyIR, making the name
+	matching in the Node swap method case insensitive.
+
 release 1.9.0 20th May 2020
 
 	1) #602 for #597. Modify Node.ancestor() to optionally include
diff --git a/src/psyclone/psyir/symbols/symboltable.py b/src/psyclone/psyir/symbols/symboltable.py
index 5d9df53a1..a2fe85132 100644
--- a/src/psyclone/psyir/symbols/symboltable.py
+++ b/src/psyclone/psyir/symbols/symboltable.py
@@ -595,7 +595,7 @@ class SymbolTable(object):
 
         :raises TypeError: if either old/new_symbol are not Symbols.
         :raises SymbolError: if `old_symbol` and `new_symbol` don't have \
-                             the same name.
+                             the same name (after normalising).
         '''
         if not isinstance(old_symbol, Symbol):
             raise TypeError("Symbol to remove must be of type Symbol but "
@@ -603,7 +603,10 @@ class SymbolTable(object):
         if not isinstance(new_symbol, Symbol):
             raise TypeError("Symbol to add must be of type Symbol but "
                             "got '{0}'".format(type(new_symbol).__name__))
-        if old_symbol.name != new_symbol.name:
+        # The symbol table is not case sensitive so we must normalise the
+        # symbol names before comparing them.
+        if (self._normalize(old_symbol.name) !=
+                self._normalize(new_symbol.name)):
             raise SymbolError(
                 "Cannot swap symbols that have different names, got: '{0}' "
                 "and '{1}'".format(old_symbol.name, new_symbol.name))


Test patch:
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py
index 45295197a..b89c509e3 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_derived_type_test.py
@@ -154,8 +154,10 @@ def test_name_clash_derived_type_def(f2008_parser):
 
 @pytest.mark.usefixtures("f2008_parser")
 @pytest.mark.parametrize("use_stmt", ["use grid_mod, only: grid_type",
+                                      "use grid_mod, only: GRID_TYPE",
                                       "use grid_mod"])
-def test_parse_derived_type(use_stmt):
+@pytest.mark.parametrize("type_name", ["GRID_TYPE", "grid_type"])
+def test_parse_derived_type(use_stmt, type_name):
     ''' Check that the fronted correctly creates a TypeSymbol of type
     StructureType from the declaration of a derived type. '''
     fake_parent = KernelSchedule("dummy_schedule")
@@ -164,10 +166,11 @@ def test_parse_derived_type(use_stmt):
     reader = FortranStringReader("{0}\n"
                                  "type :: my_type\n"
                                  "  integer :: flag\n"
-                                 "  type(grid_type), private :: grid\n"
+                                 "  type({1}), private :: grid\n"
                                  "  real, dimension(3) :: posn\n"
                                  "end type my_type\n"
-                                 "type(my_type) :: var\n".format(use_stmt))
+                                 "type(my_type) :: var\n".format(use_stmt,
+                                                                 type_name))
     fparser2spec = Fortran2003.Specification_Part(reader)
     processor.process_declarations(fake_parent, fparser2spec.content, [])
     sym = symtab.lookup("my_type")
diff --git a/src/psyclone/tests/psyir/symbols/symboltable_test.py b/src/psyclone/tests/psyir/symbols/symboltable_test.py
index 72b21fd8a..4fde3e0ff 100644
--- a/src/psyclone/tests/psyir/symbols/symboltable_test.py
+++ b/src/psyclone/tests/psyir/symbols/symboltable_test.py
@@ -490,8 +490,9 @@ def test_swap_symbol():
     assert ("Cannot swap symbols that have different names, got: 'var1' and "
             "'var2'" in str(err.value))
     # Finally, check that the method correctly adds the new symbol to the
-    # table and removes the old one.
-    symbol3 = DataSymbol("var1", REAL_TYPE)
+    # table and removes the old one (even if the case of the name of the
+    # new symbol differs from the original).
+    symbol3 = DataSymbol("Var1", REAL_TYPE)
     sym_table.swap(symbol1, symbol3)
     assert sym_table.lookup("var1") is symbol3
     assert symbol1 not in sym_table._symbols
@@ -536,7 +537,9 @@ def test_swap_symbol_properties():
         sym_table.swap_symbol_properties(symbol1, symbol4)
     assert "Symbol 'var2' is not in the symbol table." in str(excinfo.value)
 
-    # Raise exception if both symbols have the same name
+    # Raise exception if both symbols have the same name. The only way this
+    # can currently occur is if they are the same symbol (as the normalised
+    # symbol name is used as the key in the symbol table).
     with pytest.raises(ValueError) as excinfo:
         sym_table.swap_symbol_properties(symbol1, symbol1)
     assert("The symbols should have different names, but found 'var1' for "