Merge pull request #2231 from stfc/2213_psyir_public_var_error

add support for visibility symbols and abstract interfaces (closes #2213)

changelog

@@ -534,6 +534,10 @@
 	into is_constant (bool) and initial_value (PSyIR Node). This allows
 	parsing Fortran declarations with initial_value that are not constant.
 
+	181) PR #2231 for #2213. The Fortran frontend and backend now also
+	consider that UnresolvedInterface symbols can come from the body of
+	a Fortran Interface.
+
 release 2.3.1 17th of June 2022
 
 	1) PR #1747 for #1720. Adds support for If blocks to PSyAD.

src/psyclone/psyir/backend/fortran.py

@@ -901,23 +901,23 @@ def gen_decls(self, symbol_table, is_module_scope=False):
 
         :param symbol_table: the SymbolTable instance.
         :type symbol_table: :py:class:`psyclone.psyir.symbols.SymbolTable`
-        :param bool is_module_scope: whether or not the declarations are in \
-                                     a module scoping unit. Default is False.
+        :param bool is_module_scope: whether or not the declarations are in
+            a module scoping unit. Default is False.
 
         :returns: the Fortran declarations for the table.
         :rtype: str
 
-        :raises VisitorError: if one of the symbols is a RoutineSymbol which \
-            does not have an ImportInterface or UnresolvedInterface ( \
-            representing named and unqualified imports respectively) or \
-            ModuleDefaultInterface (representing routines declared in the \
+        :raises VisitorError: if one of the symbols is a RoutineSymbol which
+            does not have an ImportInterface or UnresolvedInterface (
+            representing named and unqualified imports respectively) or
+            ModuleDefaultInterface (representing routines declared in the
             same module) or is not a Fortran intrinsic.
-        :raises VisitorError: if args_allowed is False and one or more \
+        :raises VisitorError: if args_allowed is False and one or more
             argument declarations exist in symbol_table.
-        :raises VisitorError: if there are any symbols (other than \
-            RoutineSymbols) in the supplied table that do not have an \
-            explicit declaration (UnresolvedInterface) and there are no \
-            wildcard imports.
+        :raises VisitorError: if there are any symbols (other than
+            RoutineSymbols) in the supplied table that do not have an
+            explicit declaration (UnresolvedInterface) and there are no
+            wildcard imports or unknown interfaces.
 
         '''
         # pylint: disable=too-many-branches
@@ -944,16 +944,25 @@ def gen_decls(self, symbol_table, is_module_scope=False):
                     isinstance(sym.interface, UnresolvedInterface)):
                 all_symbols.remove(sym)
 
-        # If the symbol table contain any symbols with an UnresolvedInterface
-        # interface (they are not explicitly declared), we need to check that
-        # we have at least one wildcard import which could be bringing them
-        # into this scope.
+        # If the symbol table contains any symbols with an
+        # UnresolvedInterface interface (they are not explicitly
+        # declared), we need to check that we have at least one
+        # wildcard import which could be bringing them into this
+        # scope, or an unknown interface which could be declaring
+        # them.
         unresolved_symbols = []
         for sym in all_symbols[:]:
             if isinstance(sym.interface, UnresolvedInterface):
                 unresolved_symbols.append(sym)
                 all_symbols.remove(sym)
-        if unresolved_symbols and not symbol_table.has_wildcard_imports():
+        try:
+            internal_interface_symbol = symbol_table.lookup(
+                "_psyclone_internal_interface")
+        except KeyError:
+            internal_interface_symbol = None
+        if unresolved_symbols and not (
+                symbol_table.has_wildcard_imports() or
+                internal_interface_symbol):
             symbols_txt = ", ".join(
                 ["'" + sym.name + "'" for sym in unresolved_symbols])
             raise VisitorError(
@@ -1134,7 +1143,6 @@ def routine_node(self, node):
                               node is empty or None.
 
         '''
-        # pylint: disable=too-many-branches
         if not node.name:
             raise VisitorError("Expected node name to have a value.")
 

src/psyclone/psyir/frontend/fparser2.py

@@ -222,57 +222,60 @@ def _first_type_match(nodelist, typekind):
     raise ValueError  # Type not found
 
 
-def _find_or_create_imported_symbol(location, name, scope_limit=None,
-                                    **kargs):
+def _find_or_create_unresolved_symbol(location, name, scope_limit=None,
+                                      **kargs):
     '''Returns the symbol with the name 'name' from a symbol table
     associated with this node or one of its ancestors.  If a symbol is found
     and the `symbol_type` keyword argument is supplied then the type of the
     existing symbol is compared with the specified type. If it is not already
     an instance of this type, then the symbol is specialised (in place).
 
-    If the symbol is not found and there are no ContainerSymbols with wildcard
-    imports then an exception is raised. However, if there are one or more
+    If the symbol is not found and there are no ContainerSymbols with
+    wildcard imports and no interfaces with unknown content then an
+    exception is raised. However, if there are one or more
     ContainerSymbols with wildcard imports (which could therefore be
-    bringing the symbol into scope) then a new Symbol with the
-    specified visibility but of unknown interface is created and
-    inserted in the most local SymbolTable that has such an import.
-    The scope_limit variable further limits the symbol table search so
-    that the search through ancestor nodes stops when the scope_limit node
-    is reached i.e. ancestors of the scope_limit node are not searched.
+    bringing the symbol into scope) or one or more interfaces with
+    unknown content then a new Symbol with the specified visibility
+    but of unknown interface is created and inserted in the most local
+    SymbolTable that has such an import. The scope_limit variable
+    further limits the symbol table search so that the search through
+    ancestor nodes stops when the scope_limit node is reached
+    i.e. ancestors of the scope_limit node are not searched.
 
     :param location: PSyIR node from which to operate.
     :type location: :py:class:`psyclone.psyir.nodes.Node`
     :param str name: the name of the symbol.
-    :param scope_limit: optional Node which limits the symbol \
-        search space to the symbol tables of the nodes within the \
-        given scope. If it is None (the default), the whole \
-        scope (all symbol tables in ancestor nodes) is searched \
-        otherwise ancestors of the scope_limit node are not \
+    :param scope_limit: optional Node which limits the symbol
+        search space to the symbol tables of the nodes within the
+        given scope. If it is None (the default), the whole
+        scope (all symbol tables in ancestor nodes) is searched
+        otherwise ancestors of the scope_limit node are not
         searched.
-    :type scope_limit: :py:class:`psyclone.psyir.nodes.Node` or \
+    :type scope_limit: :py:class:`psyclone.psyir.nodes.Node` or
         `NoneType`
 
     :returns: the matching symbol.
     :rtype: :py:class:`psyclone.psyir.symbols.Symbol`
 
     :raises TypeError: if the supplied scope_limit is not a Node.
-    :raises ValueError: if the supplied scope_limit node is not an \
+    :raises ValueError: if the supplied scope_limit node is not an
         ancestor of the supplied node.
-    :raises SymbolError: if no matching symbol is found and there are \
-        no ContainerSymbols from which it might be brought into scope.
+    :raises SymbolError: if no matching symbol is found and there are
+        no ContainerSymbols from which it might be brought into scope
+        or unknown interfaces which might contain its declaration.
 
     '''
     if not isinstance(location, Node):
         raise TypeError(
             f"The location argument '{location}' provided to "
-            f"_find_or_create_imported_symbol() is not of type `Node`.")
+            f"_find_or_create_unresolved_symbol() is not of type `Node`.")
 
     if scope_limit is not None:
         # Validate the supplied scope_limit
         if not isinstance(scope_limit, Node):
             raise TypeError(
                 f"The scope_limit argument '{scope_limit}' provided to "
-                f"_find_or_create_imported_symbol() is not of type `Node`.")
+                f"_find_or_create_unresolved_symbol() is not of type `Node`.")
 
         # Check that the scope_limit Node is an ancestor of this
         # Reference Node and raise an exception if not.
@@ -288,8 +291,8 @@ def _find_or_create_imported_symbol(location, name, scope_limit=None,
             # supplied node so raise an exception.
             raise ValueError(
                 f"The scope_limit node '{scope_limit}' provided to "
-                f"_find_or_create_imported_symbol() is not an ancestor of this"
-                f" node '{location}'.")
+                f"_find_or_create_unresolved_symbol() is not an ancestor of "
+                f"this node '{location}'.")
 
     # Keep a reference to the most local SymbolTable with a wildcard
     # import in case we need to create a Symbol.
@@ -341,8 +344,20 @@ def _find_or_create_imported_symbol(location, name, scope_limit=None,
         return first_symbol_table.new_symbol(
                 name, interface=UnresolvedInterface(), **kargs)
 
+    # Are there any interfaces that might be hiding the symbol declaration?
+    symbol_table = location.scope.symbol_table
+    try:
+        _ = symbol_table.lookup(
+            "_psyclone_internal_interface", scope_limit=scope_limit)
+        # There is an unknown interface so add this symbol.
+        return location.scope.symbol_table.new_symbol(
+            name, interface=UnresolvedInterface(), **kargs)
+    except KeyError:
+        pass
+
     # All requested Nodes have been checked but there has been no
-    # match and there are no wildcard imports so raise an exception.
+    # match and there are no wildcard imports or unknown interfaces so
+    # raise an exception.
     raise SymbolError(f"No Symbol found for name '{name}'.")
 
 
@@ -686,7 +701,7 @@ def _kind_find_or_create(name, symbol_table):
     except KeyError:
         # The SymbolTable does not contain an entry for this kind parameter
         # so look to see if it is imported and if not create one.
-        kind_symbol = _find_or_create_imported_symbol(
+        kind_symbol = _find_or_create_unresolved_symbol(
             symbol_table.node, lower_name,
             symbol_type=DataSymbol,
             datatype=default_integer_type(),
@@ -1720,7 +1735,7 @@ def _process_type_spec(self, parent, type_spec):
                     f"other than 'type' are not yet supported.")
             type_name = str(walk(type_spec, Fortran2003.Type_Name)[0])
             # Do we already have a Symbol for this derived type?
-            type_symbol = _find_or_create_imported_symbol(parent, type_name)
+            type_symbol = _find_or_create_unresolved_symbol(parent, type_name)
             # pylint: disable=unidiomatic-typecheck
             if type(type_symbol) == Symbol:
                 # We do but we didn't know what kind of symbol it was. Create
@@ -2409,8 +2424,8 @@ def process_declarations(self, parent, nodes, arg_list,
                         # If a suitable unqualified use statement is found then
                         # this call creates a Symbol and inserts it in the
                         # appropriate symbol table.
-                        _find_or_create_imported_symbol(parent, name,
-                                                        visibility=vis)
+                        _find_or_create_unresolved_symbol(parent, name,
+                                                          visibility=vis)
                     except SymbolError as err:
                         # Improve the error message with context-specific info
                         raise SymbolError(
@@ -2943,7 +2958,7 @@ def _do_construct_handler(self, node, parent):
         loop_var = str(ctrl[0].items[1][0])
         variable_name = str(loop_var)
         try:
-            data_symbol = _find_or_create_imported_symbol(
+            data_symbol = _find_or_create_unresolved_symbol(
                 parent, variable_name, symbol_type=DataSymbol,
                 datatype=DeferredType())
         except SymbolError as err:
@@ -3500,7 +3515,7 @@ def _array_syntax_to_indexed(self, parent, loop_vars):
             range_idx = 0
             for idx, child in enumerate(array.indices):
                 if isinstance(child, Range):
-                    symbol = _find_or_create_imported_symbol(
+                    symbol = _find_or_create_unresolved_symbol(
                         array, loop_vars[range_idx],
                         symbol_type=DataSymbol, datatype=DeferredType())
                     array.children[idx] = Reference(symbol)
@@ -3646,7 +3661,7 @@ def _where_construct_handler(self, node, parent):
                                                    parent=member.parent)
             else:
                 # The array access is to a symbol of ArrayType
-                symbol = _find_or_create_imported_symbol(
+                symbol = _find_or_create_unresolved_symbol(
                     size_node, first_array.name, symbol_type=DataSymbol,
                     datatype=DeferredType())
                 new_ref = Reference(symbol)
@@ -3824,7 +3839,7 @@ def _data_ref_handler(self, node, parent):
         # that first.
         if isinstance(node.children[0], Fortran2003.Name):
             # Base of reference is a scalar entity and must be a DataSymbol.
-            base_sym = _find_or_create_imported_symbol(
+            base_sym = _find_or_create_unresolved_symbol(
                 parent, node.children[0].string.lower(),
                 symbol_type=DataSymbol, datatype=DeferredType())
             base_indices = []
@@ -3834,7 +3849,7 @@ def _data_ref_handler(self, node, parent):
             # Base of reference is an array access. Lookup the corresponding
             # symbol.
             part_ref = node.children[0]
-            base_sym = _find_or_create_imported_symbol(
+            base_sym = _find_or_create_unresolved_symbol(
                 parent, part_ref.children[0].string.lower(),
                 symbol_type=DataSymbol, datatype=DeferredType())
             # Processing the array-index expressions requires access to the
@@ -4140,7 +4155,7 @@ def _name_handler(self, node, parent):
         :rtype: :py:class:`psyclone.psyir.nodes.Reference`
 
         '''
-        symbol = _find_or_create_imported_symbol(parent, node.string)
+        symbol = _find_or_create_unresolved_symbol(parent, node.string)
         return Reference(symbol, parent=parent)
 
     def _parenthesis_handler(self, node, parent):
@@ -4184,7 +4199,7 @@ def _part_ref_handler(self, node, parent):
         # We can't say for sure that the symbol we create here should be a
         # DataSymbol as fparser2 often identifies function calls as
         # part-references instead of function-references.
-        symbol = _find_or_create_imported_symbol(parent, reference_name)
+        symbol = _find_or_create_unresolved_symbol(parent, reference_name)
 
         if isinstance(symbol, RoutineSymbol):
             call_or_array = Call(symbol, parent=parent)

src/psyclone/tests/generator_test.py

@@ -367,8 +367,9 @@ def test_script_file_too_short():
         _, _ = generate(os.path.join(BASE_PATH, "dynamo0p3",
                                      "1_single_invoke.f90"),
                         api="dynamo0.3",
-                        script_name=os.path.join(BASE_PATH,
-                                                 "dynamo0p3", "testkern_xyz_mod.f90"))
+                        script_name=os.path.join(
+                            BASE_PATH,
+                            "dynamo0p3", "testkern_xyz_mod.f90"))
 
 
 def test_no_script_gocean():

src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py

@@ -280,3 +280,34 @@ def test_gen_decls_static_variables(fortran_writer):
     sym.initial_value = 1
     sym.is_constant = True
     assert "parameter :: v1 = 1" in fortran_writer.gen_vardecl(sym)
+
+
+@pytest.mark.parametrize("visibility", ["public", "private"])
+def test_visibility_interface(fortran_reader, fortran_writer, visibility):
+    '''Test that PSyclone's Fortran backend successfully writes out
+    public/private clauses and symbols when the symbol's declaration
+    is hidden in an abstract interface.
+
+    '''
+    code = (
+        f"module test\n"
+        f"  abstract interface\n"
+        f"     subroutine update_interface()\n"
+        f"     end subroutine update_interface\n"
+        f"  end interface\n"
+        f"  {visibility} :: update_interface\n"
+        f"contains\n"
+        f"  subroutine alg()\n"
+        f"  end subroutine alg\n"
+        f"end module test\n")
+    psyir = fortran_reader.psyir_from_source(code)
+    result = fortran_writer(psyir)
+    # The default visibility is PUBLIC so it is always output by
+    # the backend.
+    assert "public\n" in result
+    if visibility == "public":
+        # The generic PUBLIC visibility covers all symbols so we do
+        # not need to output "public :: update_interface".
+        assert "public :: update_interface" not in result
+    if visibility == "private":
+        assert "private :: update_interface" in result

src/psyclone/tests/psyir/conftest.py

@@ -1,7 +1,7 @@
 # -----------------------------------------------------------------------------
 # BSD 3-Clause License
 #
-# Copyright (c) 2019-2020, 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
@@ -32,14 +32,15 @@
 # POSSIBILITY OF SUCH DAMAGE.
 # -----------------------------------------------------------------------------
 # Author A. R. Porter, STFC Daresbury Lab
+# Modified R. W. Ford, STFC Daresbury Lab
 
 
 ''' Module which performs pytest set-up specific to the PSyIR tests. '''
 
-from __future__ import absolute_import
 import pytest
-from psyclone.psyir.symbols import DataSymbol, INTEGER_TYPE
+
 import psyclone.psyir.frontend.fparser2 as fp2
+from psyclone.psyir.symbols import DataSymbol, INTEGER_TYPE
 
 
 @pytest.fixture(scope="function")
@@ -52,6 +53,6 @@ def disable_declaration_check(monkeypatch):
 
     '''
     monkeypatch.setattr(
-        fp2, "_find_or_create_imported_symbol",
+        fp2, "_find_or_create_unresolved_symbol",
         lambda _1, name, _2=None: DataSymbol(name,
                                              INTEGER_TYPE))