Repository: stfc/PSyclone

Issue:
[psyir FE] wildcard import followed by specific import causes failure
The following Fortran code causes the fparser2 frontend to fail:

        module test_mod
          use some_mod
        contains
          subroutine test_sub1()
            ! a_var here must be being brought into scope by the
            ! `use some_mod` in the module.
            a_var = 1.0
          end subroutine test_sub1
          subroutine test_sub2()
            use some_mod, only: a_var
            a_var = 2.0
          end subroutine test_sub2
        end module test_mod

with:

    E           KeyError: "Symbol table already contains a symbol with name 'a_var'."

This prevents the current master of PSyclone from processing (some of) the NEMO source.


Discussion:

Patch:
diff --git a/changelog b/changelog
index a1faa0681..9d3e2351a 100644
--- a/changelog
+++ b/changelog
@@ -100,7 +100,7 @@
 
         34) #823 for #471. Updates the access from INC to READWRITE for
         those kernels that loop over DoFs.
-	
+
 	35) #872 for #736 and #858. Routine Symbols are captured by the Fortran
 	front-end when parsing a Container.
 
@@ -126,6 +126,9 @@
 	provided about a symbol and replace the symbol latter on if more information
 	if found.
 
+	42) #904 for #903. Fixed an error in handling use statements (a
+	symbol clash when there should not be one).
+
 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 e279263a9..9d16e1e54 100644
--- a/src/psyclone/psyir/frontend/fparser2.py
+++ b/src/psyclone/psyir/frontend/fparser2.py
@@ -1188,14 +1188,18 @@ class Fparser2Reader(object):
                     # will replace a previous import with an empty only-list.
                     pass
                 for name in decl.items[4].items:
-                    # The DataSymbol adds itself to the list of symbols
-                    # imported by the Container referenced in the
-                    # GlobalInterface.
                     sym_name = str(name).lower()
                     if sym_name not in parent.symbol_table:
+                        # We're dealing with a symbol named in a use statement
+                        # in the *current* scope therefore we do not check
+                        # any ancestor symbol tables; we just create a
+                        # new symbol. Since we don't yet know anything about
+                        # this symbol apart from its name we create a generic
+                        # Symbol.
                         parent.symbol_table.add(
                             Symbol(sym_name,
-                                   interface=GlobalInterface(container)))
+                                   interface=GlobalInterface(container)),
+                            check_ancestors=False)
                     else:
                         # There's already a symbol with this name
                         existing_symbol = parent.symbol_table.lookup(


Test patch:
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py
index 8e654cf1b..0ed8b08e5 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py
@@ -2678,3 +2678,42 @@ def test_loop_var_exception(parser):
         "Loop-variable name 'i' is not declared and there are no unqualified "
         "use statements. This is currently unsupported."
         in str(excinfo.value))
+
+
+def test_named_and_wildcard_use_var(f2008_parser):
+    ''' Check that we handle the case where a variable is accessed first by
+    a wildcard import and then by a named import. '''
+    reader = FortranStringReader('''
+        module test_mod
+          use some_mod
+        contains
+          subroutine test_sub1()
+            ! a_var here must be being brought into scope by the
+            ! `use some_mod` in the module.
+            a_var = 1.0
+          end subroutine test_sub1
+          subroutine test_sub2()
+            use some_mod, only: a_var
+            a_var = 2.0
+          end subroutine test_sub2
+        end module test_mod
+        ''')
+    prog = f2008_parser(reader)
+    psy = PSyFactory(api="nemo").create(prog)
+    # We should have an entry for "a_var" in the Container symbol table
+    # due to the access in "test_sub1".
+    container = psy.invokes.container
+    avar1 = container.symbol_table.lookup("a_var")
+    # It must be a generic Symbol since we don't know anything about it
+    # pylint: disable=unidiomatic-typecheck
+    assert type(avar1) == Symbol
+    # There should be no entry for "a_var" in the symbol table for the
+    # "test_sub1" routine as it is not declared there.
+    schedule = psy.invokes.invoke_list[0].schedule
+    assert "a_var" not in schedule.symbol_table
+    # There should be another, distinct entry for "a_var" in the symbol table
+    # for "test_sub2" as it has a use statement that imports it.
+    schedule = psy.invokes.invoke_list[1].schedule
+    avar2 = schedule.symbol_table.lookup("a_var")
+    assert type(avar2) == Symbol
+    assert avar2 is not avar1