Repository: stfc/PSyclone

Issue:
PSyclone converts modules containing the `save` attribute to CodeBlock
I'm not sure exactly what this behaviour means in the context of Fortran, but I came across some code which didn't convert a module into a `Container` when using PSyclone to do some source to source translation, due to the presence of the `save` attribute.

Not sure if this is worth fixing or not - if its a small change then perhaps (but I don't know how it might affect the rest of the code, like the `save` attribute in general).

Replicator test.

```
    def test_container_creation(fortran_reader):
        code = '''module test_mod
        implicit none
        private
        public :: var1
        public :: var2, var3

        save

        integer, dimension(123) :: var2, var3

        contains

        subroutine var1()
        var2(:) = 0
        var3(:) = 0
        end subroutine var1

        end module'''

        psyir = fortran_reader.psyir_from_source(code)

        print(psyir.children[0])
       assert isinstance(psyir.children[0], Container)
```

Discussion:

Patch:
diff --git a/changelog b/changelog
index 7f8d73173..9f04b85bd 100644
--- a/changelog
+++ b/changelog
@@ -54,6 +54,8 @@
 
 	20) PR #2377 for #2368. Adds optional 'depth' argument to Node.walk().
 
+	21) PR #2362 for #2243 and #2312. Add support for Fortran save statements.
+
 release 2.4.0 29th of September 2023
 
 	1) PR #1758 for #1741. Splits the PSyData read functionality into a
diff --git a/examples/nemo/scripts/Makefile b/examples/nemo/scripts/Makefile
index e794103f9..c9ca32743 100644
--- a/examples/nemo/scripts/Makefile
+++ b/examples/nemo/scripts/Makefile
@@ -12,6 +12,8 @@
 PSYCLONE_NEMO_DIR ?= ${WORKSPACE}/PSyclone/examples/nemo/scripts
 # - Specify location of preprocessed files - you need to call 'make compile-serial' once first)
 # ROOT_SRC ?=
+# Where to find the MPI header files
+# MPI_INC_DIR ?=
 # - Specify NEMO test case
 TEST_CASE ?= SPITZ12
 # - Specify compilation options
@@ -25,6 +27,7 @@ TEST_CASE ?= SPITZ12
 # Example Config for MetOffice - eORCA1_GO8 - nvidia
 NEMO_DIR ?= ${WORKSPACE}/NEMO
 ROOT_SRC ?= ${NEMO_DIR}/cfgs/SPITZ12_serial/BLD/ppsrc/nemo/
+MPI_INC_DIR ?= ${NVHPC_ROOT}/comm_libs/mpi/include
 COMPILER_ARCH ?= linux_nvidia_omp_gpu
 ADD_KEYS ?= "IEEE_IS_NAN=ieee_is_nan key_nosignedzero"
 DEL_KEYS ?= "key_iomput key_mpp_mpi key_si3"
@@ -34,6 +37,7 @@ NAMELISTS_DIR ?= ${NEMO_DIR}/
 # Example Config for ECMWF - eORCA1_GO8_Z75 (with MPI and SI3) - intel compiler
 # NEMO_DIR ?= ${WORKSPACE}/ecmwf_nemo/nemo/NEMOGCM_V40/
 # ROOT_SRC ?= ${NEMO_DIR}/cfgs/SPITZ12_serial/BLD/ppsrc/nemo/
+# MPI_INC_DIR ?= ${I_MPI_ROOT}/include
 # COMPILER_ARCH ?= linux_intel
 # ADD_KEYS ?= "IEEE_IS_NAN=ieee_is_nan key_nosignedzero"
 # DEL_KEYS ?= "key_iomput"
@@ -62,16 +66,16 @@ $(TARGETS): % : $(addprefix psycloned-%/, $(notdir $(SRC_FILES)))
 # We perform a 'passthrough' test by simply not providing a transformation
 # script.
 psycloned-passthrough/%.f90: ${ROOT_SRC}%.f90 psycloned-passthrough
-	${PSYCLONE_NEMO_DIR}/process_nemo.py -I ${ROOT_SRC} -o psycloned-passthrough $<
+	${PSYCLONE_NEMO_DIR}/process_nemo.py -I ${ROOT_SRC} -I ${MPI_INC_DIR} -o psycloned-passthrough $<
 
 psycloned-openmp_cpu/%.f90: ${ROOT_SRC}%.f90 psycloned-openmp_cpu
-	${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/omp_cpu_trans.py -I ${ROOT_SRC} -o psycloned-openmp_cpu $<
+	${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/omp_cpu_trans.py -I ${ROOT_SRC} -I ${MPI_INC_DIR} -o psycloned-openmp_cpu $<
 
 psycloned-openmp_gpu/%.f90: ${ROOT_SRC}%.f90 psycloned-openmp_gpu
-	${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/omp_gpu_trans.py -I ${ROOT_SRC} -o psycloned-openmp_gpu $<
+	${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/omp_gpu_trans.py -I ${ROOT_SRC} -I ${MPI_INC_DIR} -o psycloned-openmp_gpu $<
 
 psycloned-openacc_kernels/%.f90: ${ROOT_SRC}%.f90 psycloned-openacc_kernels
-	${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/acc_kernels_trans.py -I ${ROOT_SRC} -o psycloned-openacc_kernels $<
+	${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/acc_kernels_trans.py -I ${ROOT_SRC} -I ${MPI_INC_DIR} -o psycloned-openacc_kernels $<
 
 psycloned-openacc_loops/%.f90: ${ROOT_SRC}%.f90 psycloned-openacc_loops
 	${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/acc_loops_trans.py -I ${ROOT_SRC} -o psycloned-openacc_loops $<
diff --git a/examples/nemo/scripts/process_nemo.py b/examples/nemo/scripts/process_nemo.py
index 6b831bab5..5e2545fa5 100755
--- a/examples/nemo/scripts/process_nemo.py
+++ b/examples/nemo/scripts/process_nemo.py
@@ -80,9 +80,6 @@ NOT_PERFORMANT = [
 NOT_WORKING = [
     # Array accessed inside WHERE does not use array notation
     "diurnal_bulk.f90",
-    # mpif.h include is lost
-    "mpp_map.f90", "obs_mpp.f90", "icblbc.f90",
-    "timing.f90", "lib_mpp.f90", "nemogcm.f90",
     # TODO #1960: There is a bug on a WHERE fparser2 frontend processing
     "sbccpl.f90",
 ]
diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py
index 50cfea828..8180ad8b5 100644
--- a/src/psyclone/psyir/backend/fortran.py
+++ b/src/psyclone/psyir/backend/fortran.py
@@ -546,11 +546,17 @@ class FortranWriter(LanguageWriter):
 
         if isinstance(symbol.datatype, UnknownType):
             if isinstance(symbol.datatype, UnknownFortranType):
-                if include_visibility and not isinstance(symbol,
-                                                         RoutineSymbol):
+
+                if (include_visibility and
+                        not isinstance(symbol, RoutineSymbol) and
+                        not symbol.name.startswith("_PSYCLONE_INTERNAL")):
+                    # We don't attempt to add accessibility to RoutineSymbols
+                    # or to those created by PSyclone to handle named common
+                    # blocks appearing in SAVE statements.
                     decln = add_accessibility_to_unknown_declaration(symbol)
-                else:
-                    decln = symbol.datatype.declaration
+                    return f"{self._nindent}{decln}\n"
+
+                decln = symbol.datatype.declaration
                 return f"{self._nindent}{decln}\n"
             # The Fortran backend only handles unknown *Fortran* declarations.
             raise VisitorError(
diff --git a/src/psyclone/psyir/frontend/fortran.py b/src/psyclone/psyir/frontend/fortran.py
index 2fd90bbaa..526f29866 100644
--- a/src/psyclone/psyir/frontend/fortran.py
+++ b/src/psyclone/psyir/frontend/fortran.py
@@ -107,7 +107,7 @@ class FortranReader():
         :param str source_code: text of the expression to be parsed.
         :param symbol_table: the SymbolTable in which to search for any
             symbols that are encountered.
-        :type symbol_table: :py.class:`psyclone.psyir.symbols.SymbolTable`
+        :type symbol_table: :py:class:`psyclone.psyir.symbols.SymbolTable`
 
         :returns: PSyIR representing the provided Fortran expression.
         :rtype: :py:class:`psyclone.psyir.nodes.Node`
diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py
index 9f49fcfcc..2294581c6 100644
--- a/src/psyclone/psyir/frontend/fparser2.py
+++ b/src/psyclone/psyir/frontend/fparser2.py
@@ -55,8 +55,8 @@ from psyclone.psyir.nodes import (
     Reference, Return, Routine, Schedule, StructureReference, UnaryOperation,
     WhileLoop)
 from psyclone.psyir.nodes.array_mixin import ArrayMixin
-from psyclone.psyir.nodes.array_of_structures_mixin import \
-    ArrayOfStructuresMixin
+from psyclone.psyir.nodes.array_of_structures_mixin import (
+    ArrayOfStructuresMixin)
 from psyclone.psyir.symbols import (
     ArgumentInterface, ArrayType, ContainerSymbol, DataSymbol, DataTypeSymbol,
     DeferredType, ImportInterface, AutomaticInterface, NoType,
@@ -1523,6 +1523,73 @@ class Fparser2Reader():
 
         return (default_visibility, visibility_map)
 
+    @staticmethod
+    def _process_save_statements(nodes, parent):
+        '''
+        Search the supplied list of fparser2 nodes (which must represent a
+        complete Specification Part) for any SAVE statements (e.g.
+        "SAVE :: my_var") to determine which Symbols are static.
+
+        Any common blocks referred to in a SAVE will result in Symbols of
+        UnknownFortranType being added to the symbol table associated with
+        `parent`.
+
+        :param nodes: nodes in the fparser2 parse tree describing a
+                      Specification Part that will be searched.
+        :type nodes: List[:py:class:`fparser.two.utils.Base`]
+        :param : the parent node in the PSyIR under construction.
+        :type : :py:class:`psyclone.psyir.nodes.Node`
+
+        :returns: names of symbols that are static or just "*" if they all are.
+        :rtype: List[str]
+
+        :raises GenerationError: if the parse tree is found to contain a SAVE
+            without a saved-entity list *and* one or more SAVE attributes or
+            SAVE statements (C580).
+
+        '''
+        symbol_table = parent.scope.symbol_table
+        default_save = False
+        # Set holding the names of those symbols which are marked as static
+        # via an explicit SAVE stmt (e.g. "SAVE :: my_var")
+        explicit_save = set()
+
+        save_stmts = walk(nodes, Fortran2003.Save_Stmt)
+
+        for stmt in save_stmts:
+
+            if not stmt.children[1]:
+                # No saved-entity list means that all entities are static.
+                default_save = True
+            else:
+                symbol_names = [child.string.lower() for child in
+                                stmt.children[1].children]
+                explicit_save.update(symbol_names)
+
+        if default_save:
+            if explicit_save:
+                # This should really be caught by the Fortran parser but
+                # fparser2 is lax.
+                names = sorted(list(explicit_save))
+                raise GenerationError(
+                    f"Supplied nodes contain a SAVE without a saved-entity "
+                    f"list plus one or more SAVES *with* saved-entity lists "
+                    f"(naming {names}). This is not valid Fortran.")
+            explicit_save.add("*")
+
+        # If there are any named Common blocks listed in a SAVE statement then
+        # we create Symbols of UnknownFortranType for them (so that the
+        # backend can recreate the necessary SAVE statement) and remove them
+        # from the list returned by this method.
+        for name in explicit_save.copy():
+            if name.startswith("/"):
+                uftype = UnknownFortranType(f"SAVE :: {name}")
+                symbol_table.new_symbol(root_name="_PSYCLONE_INTERNAL_SAVE",
+                                        symbol_type=DataSymbol,
+                                        datatype=uftype)
+                explicit_save.remove(name)
+        return list(explicit_save)
+
     @staticmethod
     def _process_use_stmts(parent, nodes, visibility_map=None):
         '''
@@ -1743,7 +1810,8 @@ class Fparser2Reader():
 
         return base_type, precision
 
-    def _process_decln(self, scope, symbol_table, decl, visibility_map=None):
+    def _process_decln(self, scope, symbol_table, decl, visibility_map=None,
+                       statics_list=()):
         '''
         Process the supplied fparser2 parse tree for a declaration. For each
         entity that is declared, a symbol is added to the supplied symbol
@@ -1755,27 +1823,31 @@ class Fparser2Reader():
         :type symbol_table: py:class:`psyclone.psyir.symbols.SymbolTable`
         :param decl: fparser2 parse tree of declaration to process.
         :type decl: :py:class:`fparser.two.Fortran2003.Type_Declaration_Stmt`
-        :param visibility_map: mapping of symbol name to visibility (for \
+        :param visibility_map: mapping of symbol name to visibility (for
             those symbols listed in an accessibility statement).
-        :type visibility_map: dict with str keys and \
+        :type visibility_map: dict with str keys and
             :py:class:`psyclone.psyir.symbols.Symbol.Visibility` values
+        :param statics_list: the names of symbols which are static (due to
+            appearing in a SAVE statement). If all symbols are static then
+            this contains the single entry "*".
+        :type statics_list: Iterable[str]
 
         :raises NotImplementedError: if an unsupported attribute is found.
-        :raises NotImplementedError: if an unsupported intent attribute is \
+        :raises NotImplementedError: if an unsupported intent attribute is
             found.
-        :raises NotImplementedError: if an unsupported access-spec attribute \
+        :raises NotImplementedError: if an unsupported access-spec attribute
             is found.
-        :raises NotImplementedError: if the allocatable attribute is found on \
+        :raises NotImplementedError: if the allocatable attribute is found on
             a non-array declaration.
-        :raises InternalError: if an array with defined extent has the \
+        :raises InternalError: if an array with defined extent has the
             allocatable attribute.
-        :raises NotImplementedError: if an unsupported initialisation \
+        :raises NotImplementedError: if an unsupported initialisation
             expression is found for a parameter declaration.
-        :raises NotImplementedError: if a character-length specification is \
+        :raises NotImplementedError: if a character-length specification is
             found.
-        :raises SymbolError: if a declaration is found for a symbol that is \
+        :raises SymbolError: if a declaration is found for a symbol that is
             already present in the symbol table with a defined interface.
-        :raises GenerationError: if a set of incompatible Fortran \
+        :raises GenerationError: if a set of incompatible Fortran
             attributes are found in a symbol declaration.
 
         '''
@@ -1797,6 +1869,8 @@ class Fparser2Reader():
         # 5) Access-specification - this var is only set if the declaration
         # has an explicit access-spec (e.g. INTEGER, PRIVATE :: xxx)
         decln_access_spec = None
+        # 6) Whether this declaration has the SAVE attribute.
+        has_save_attr = False
         if attr_specs:
             for attr in attr_specs.items:
                 if isinstance(attr, Fortran2003.Attr_Spec):
@@ -1804,7 +1878,7 @@ class Fparser2Reader():
                     if normalized_string == "save":
                         if interface is not None:
                             multiple_interfaces = True
-                        interface = StaticInterface()
+                        has_save_attr = True
                     elif normalized_string == "parameter":
                         # Flag the existence of a constant value in the RHS
                         has_constant_value = True
@@ -1849,7 +1923,7 @@ class Fparser2Reader():
             # Fortran but fparser does not check, so we need to check for them
             # here.
             # TODO fparser/#413 could also fix these issues.
-            if isinstance(interface, StaticInterface) and has_constant_value:
+            if has_save_attr and has_constant_value:
                 raise GenerationError(
                     f"SAVE and PARAMETER attributes are not compatible but "
                     f"found:\n {decl}")
@@ -1874,16 +1948,6 @@ class Fparser2Reader():
                     f"Multiple or duplicated incompatible attributes "
                     f"found in declaration:\n {decl}")
 
-        # If interface is not explicitly specified, provide a default value
-        if interface is None:
-            if isinstance(scope, Container):
-                interface = DefaultModuleInterface()
-            else:
-                interface = AutomaticInterface()
-                # This might still be redefined as Argument later if it
-                # appears in the argument list, but we don't know at this
-                # point.
-
         # Parse declarations RHS and declare new symbol into the
         # parent symbol table for each entity found.
         for entity in entities.items:
@@ -1944,6 +2008,27 @@ class Fparser2Reader():
                 else:
                     visibility = symbol_table.default_visibility
 
+            listed_in_save = "*" in statics_list or sym_name in statics_list
+            if has_save_attr or listed_in_save:
+                if has_save_attr and listed_in_save:
+                    raise GenerationError(
+                        f"Invalid Fortran: '{decl}'. Symbol 'sym_name' is "
+                        f"the subject of a SAVE statement but also has a SAVE "
+                        f"attribute on its declaration.")
+                this_interface = StaticInterface()
+            elif not interface:
+                # Interface not explicitly specified, provide a default value.
+                # This might still be redefined as Argument later if it appears
+                # in the argument list, but we don't know at this point.
+                this_interface = (DefaultModuleInterface() if
+                                  isinstance(scope, Container) else
+                                  AutomaticInterface())
+            else:
+                # We use copies of the interface object because we will reuse
+                # the interface for each entity if there are multiple in the
+                # same declaration statement.
+                this_interface = interface.copy()
+
             if entity_shape:
                 # array
                 datatype = ArrayType(base_type, entity_shape)
@@ -1960,7 +2045,7 @@ class Fparser2Reader():
                     # This was a generic symbol. We now know what it is
                     sym.specialise(DataSymbol, datatype=datatype,
                                    visibility=visibility,
-                                   interface=interface,
+                                   interface=this_interface,
                                    is_constant=has_constant_value,
                                    initial_value=init_expr)
                 else:
@@ -2002,9 +2087,6 @@ class Fparser2Reader():
 
                 symbol_table.add(sym, tag=tag)
 
-            # We use copies of the interface object because we will reuse the
-            # interface for each entity if there are multiple in the same
-            # declaration statement.
             if init_expr:
                 # In Fortran, an initialisation expression on a declaration of
                 # a symbol (whether in a routine or a module) implies that the
@@ -2012,7 +2094,7 @@ class Fparser2Reader():
                 # unless it is a pointer initialisation.
                 sym.interface = StaticInterface()
             else:
-                sym.interface = interface.copy()
+                sym.interface = this_interface
 
     def _process_derived_type_decln(self, parent, decl, visibility_map):
         '''
@@ -2265,13 +2347,13 @@ class Fparser2Reader():
         :param parent: PSyIR node in which to insert the symbols found.
         :type parent: :py:class:`psyclone.psyir.nodes.KernelSchedule`
         :param nodes: fparser2 AST nodes containing declaration statements.
-        :type nodes: list of :py:class:`fparser.two.utils.Base`
+        :type nodes: List[:py:class:`fparser.two.utils.Base`]
         :param arg_list: fparser2 AST node containing the argument list.
         :type arg_list: :py:class:`fparser.Fortran2003.Dummy_Arg_List`
         :param visibility_map: mapping of symbol names to explicit
             visibilities.
-        :type visibility_map: dict with str keys and values of type
-            :py:class:`psyclone.psyir.symbols.Symbol.Visibility`
+        :type visibility_map: dict[
+            str, :py:class:`psyclone.psyir.symbols.Symbol.Visibility`]
 
         :raises GenerationError: if an INCLUDE statement is encountered.
         :raises NotImplementedError: the provided declarations contain
@@ -2290,6 +2372,9 @@ class Fparser2Reader():
         # Look at any USE statements
         self._process_use_stmts(parent, nodes, visibility_map)
 
+        # Look at any SAVE statements to determine any static symbols.
+        statics_list = self._process_save_statements(nodes, parent)
+
         # Handle any derived-type declarations/definitions before we look
         # at general variable declarations in case any of the latter use
         # the former.
@@ -2364,7 +2449,7 @@ class Fparser2Reader():
             elif isinstance(node, Fortran2003.Type_Declaration_Stmt):
                 try:
                     self._process_decln(parent, parent.symbol_table, node,
-                                        visibility_map)
+                                        visibility_map, statics_list)
                 except NotImplementedError:
                     # Found an unsupported variable declaration. Create a
                     # DataSymbol with UnknownType for each entity being
@@ -2430,6 +2515,7 @@ class Fparser2Reader():
                     node.children[2].items = tuple(orig_children)
 
             elif isinstance(node, (Fortran2003.Access_Stmt,
+                                   Fortran2003.Save_Stmt,
                                    Fortran2003.Derived_Type_Def,
                                    Fortran2003.Stmt_Function_Stmt,
                                    Fortran2003.Common_Stmt,
@@ -4398,7 +4484,7 @@ class Fparser2Reader():
         statement into a PSyIR Routine node.
 
         :param node: node in fparser2 parse tree.
-        :type node: :py:class:`fparser.two.Fortran2003.Subroutine_Subprogram` \
+        :type node: :py:class:`fparser.two.Fortran2003.Subroutine_Subprogram`
             or :py:class:`fparser.two.Fortran2003.Function_Subprogram`
         :param parent: parent node of the PSyIR node being constructed.
         :type parent: subclass of :py:class:`psyclone.psyir.nodes.Node`
@@ -4409,19 +4495,18 @@ class Fparser2Reader():
 
         :raises NotImplementedError: if the node contains a Contains clause.
         :raises NotImplementedError: if the node contains an ENTRY statement.
-        :raises NotImplementedError: if an unsupported prefix is found or no
-            explicit type information is available for a Function.
+        :raises NotImplementedError: if an unsupported prefix is found.
+        :raises SymbolError: if no explicit type information is available for
+                             the return value of a Function.
 
         '''
         try:
             _first_type_match(node.children,
                               Fortran2003.Internal_Subprogram_Part)
-            has_contains = True
-        except ValueError:
-            has_contains = False
-        if has_contains:
             raise NotImplementedError("PSyclone doesn't yet support 'Contains'"
                                       " inside a Subroutine or Function")
+        except ValueError:
+            pass
 
         entry_stmts = walk(node, Fortran2003.Entry_Stmt)
         if entry_stmts:
@@ -4445,10 +4530,10 @@ class Fparser2Reader():
         # TODO this if test can be removed once fparser/#211 is fixed
         # such that routine arguments are always contained in a
         # Dummy_Arg_List, even if there's only one of them.
-        if isinstance(node, (Fortran2003.Subroutine_Subprogram,
-                             Fortran2003.Function_Subprogram)) and \
-           isinstance(node.children[0].children[2],
-                      Fortran2003.Dummy_Arg_List):
+        if (isinstance(node, (Fortran2003.Subroutine_Subprogram,
+                              Fortran2003.Function_Subprogram)) and
+                isinstance(node.children[0].children[2],
+                           Fortran2003.Dummy_Arg_List)):
             arg_list = node.children[0].children[2].children
         else:
             # Routine has no arguments
@@ -4496,13 +4581,6 @@ class Fparser2Reader():
                 # function of the variable used to hold the return
                 # value.
                 if isinstance(symbol, RoutineSymbol):
-                    if not base_type:
-                        # The type of the return value was not specified in the
-                        # function prefix either therefore we have no explicit
-                        # type information for it.
-                        raise NotImplementedError(
-                            f"No explicit type information found for "
-                            f"function '{name}'")
                     # Remove the RoutineSymbol ready to replace it with a
                     # DataSymbol.
                     routine.symbol_table.remove(symbol)
@@ -4515,6 +4593,17 @@ class Fparser2Reader():
                 # attempt to recreate the prefix. We have to set shadowing to
                 # True as there is likely to be a RoutineSymbol for this
                 # function in any enclosing Container.
+                if not base_type:
+                    # The type of the return value was not specified in the
+                    # function prefix or in a local declaration and therefore
+                    # we have no explicit type information for it. Since we
+                    # default to adding `implicit none` when generating Fortran
+                    # we can't simply put this function into a CodeBlock as the
+                    # generated code won't compile.
+                    raise SymbolError(
+                        f"No explicit return-type information found for "
+                        f"function '{name}'. PSyclone requires that all "
+                        f"symbols be explicitly typed.")
 
                 # First, update the existing RoutineSymbol with the
                 # return datatype specified in the function
diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py
index c11b72536..a617e5bb2 100644
--- a/src/psyclone/psyir/symbols/datatypes.py
+++ b/src/psyclone/psyir/symbols/datatypes.py
@@ -161,7 +161,7 @@ class UnknownFortranType(UnknownType):
     @property
     def partial_datatype(self):
         '''
-        :returns: partial datatype information if it can be determined, \
+        :returns: partial datatype information if it can be determined,
             else None.
         :rtype: Optional[:py:class:`psyclone.symbols.DataType`]
         '''
@@ -179,8 +179,16 @@ class UnknownFortranType(UnknownType):
         of the variable being declared. Once that is done this method
         won't be required.
 
+        Note that UnknownFortranType is also used to hold things like
+        'SAVE :: /my_common/' and thus it is not always possible/appropriate
+        to extract a type expression.
+
         :returns: the Fortran code specifying the type.
         :rtype: str
+
+        :raises NotImplementedError: if declaration text cannot be
+            extracted from the original Fortran declaration.
+
         '''
         if self._type_text:
             return self._type_text
@@ -195,10 +203,28 @@ class UnknownFortranType(UnknownType):
         # Set reader to free format.
         string_reader.set_format(FortranFormat(True, False))
         ParserFactory().create(std="f2008")
-        ptree = Fortran2003.Declaration_Construct(
-            string_reader)
-        self._type_text = str(ptree.children[0])
-
+        try:
+            ptree = Fortran2003.Specification_Part(
+                string_reader)
+        except Exception as err:
+            raise NotImplementedError(
+                f"Cannot extract the declaration part from UnknownFortranType "
+                f"'{self._declaration}' because parsing (attempting to match "
+                f"a Fortran2003.Specification_Part) failed.") from err
+        node = ptree.children[0]
+        if isinstance(node, (Fortran2003.Declaration_Construct,
+                             Fortran2003.Type_Declaration_Stmt)):
+            self._type_text = str(node.children[0])
+        elif isinstance(node, Fortran2003.Save_Stmt):
+            self._type_text = "SAVE"
+        elif isinstance(node, Fortran2003.Common_Stmt):
+            self._type_text = "COMMON"
+        else:
+            raise NotImplementedError(
+                f"Cannot extract the declaration part from UnknownFortranType "
+                f"'{self._declaration}'. Only Declaration_Construct, "
+                f"Type_Declaration_Stmt, Save_Stmt and Common_Stmt are "
+                f"supported but got '{type(node).__name__}' from the parser.")
         return self._type_text
 
     def __eq__(self, other):


Test patch:
diff --git a/.github/workflows/nemo_tests.yml b/.github/workflows/nemo_tests.yml
index d4f506f2c..7357fa317 100644
--- a/.github/workflows/nemo_tests.yml
+++ b/.github/workflows/nemo_tests.yml
@@ -177,10 +177,15 @@ jobs:
         gist_id: a4049a0fc0a0a11651a5ce6a04d76160
         file_path: ../../../../store_results/performance_history_openacc_loops_gpu
         
-    # PSyclone, compile and run ECMWF NEMO with OpenMP for CPUs
+    # PSyclone, compile and run ECMWF NEMO with OpenMP for CPUs. This uses
+    # the Intel compiler and does not disable MPI. Therefore we have to ensure
+    # that we provide the path to the header files for Intel MPI.
     - name: NEMO ECMWF OpenMP for CPU
       run: |
         . .runner_venv/bin/activate
+        module rm nvidia-hpcsdk
+        module load intel/oneapi compiler mpi
+        module load perl/${PERL_VERSION}
         export PSYCLONE_NEMO_DIR=${GITHUB_WORKSPACE}/examples/nemo/scripts
         export NEMO_DIR=${HOME}/NEMOGCM_V40
         export COMPILER_ARCH=linux_intel
@@ -188,10 +193,9 @@ jobs:
         export DEL_KEYS="key_iomput"
         export MODEL_DIR=/archive/ssiso/ecmwf_eORCA1_GO8/
         export NAMELISTS_DIR=${NEMO_DIR}/testscripts_V40/output/openmp_outer_V40_eORCA1_GO8_Z75_20170906_cray_dp_1x1/
+        export MPI_INC_DIR=${I_MPI_ROOT}/include
         cd $PSYCLONE_NEMO_DIR
         make -j 4 openmp_cpu
-        module load intel/oneapi compiler mpi
-        module load perl/${PERL_VERSION}
         export LD_LIBRARY_PATH=/home/ssiso/ecmwf_nemo/ESiWACE2/scripts/dev/netcdf-c-4.9.0/lib/:$LD_LIBRARY_PATH
         export LD_LIBRARY_PATH=/home/ssiso/ecmwf_nemo/ESiWACE2/scripts/dev/netcdf-fortran-4.5.4/lib:$LD_LIBRARY_PATH
         export LD_LIBRARY_PATH=/home/ssiso/ecmwf_nemo/ESiWACE2/scripts/dev/hdf5-1.12.2/lib/:$LD_LIBRARY_PATH
diff --git a/src/psyclone/tests/psyir/backend/fortran_unknown_declns_test.py b/src/psyclone/tests/psyir/backend/fortran_unknown_declns_test.py
index 6fdddee89..8d2be4b2c 100644
--- a/src/psyclone/tests/psyir/backend/fortran_unknown_declns_test.py
+++ b/src/psyclone/tests/psyir/backend/fortran_unknown_declns_test.py
@@ -266,3 +266,22 @@ def test_generating_unknowntype_routine_imports(
     code = fortran_writer(psyir)
     assert "use a_mod, only : unknown_type_symbol" in code
     assert "interface" not in code.lower()
+
+
+def test_fw_save_common(fortran_reader, fortran_writer):
+    '''
+    Check that a SAVE statement involving a named Common Block is correctly
+    re-generated.
+    '''
+    code = '''
+      module my_mod
+        save :: var3, /my_common/
+        common /my_common/ a, b
+        integer :: var1
+        integer :: a, b
+        integer :: var3
+      end module my_mod'''
+    psyir = fortran_reader.psyir_from_source(code)
+    output = fortran_writer(psyir)
+    assert "SAVE :: /my_common/\n" in output
+    assert "integer, save, public :: var3\n" in output
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_interface_block_test.py b/src/psyclone/tests/psyir/frontend/fparser2_interface_block_test.py
index ca73ae75b..ceb2cd5fb 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_interface_block_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_interface_block_test.py
@@ -93,7 +93,7 @@ def test_named_interface_declared(fortran_reader):
         module procedure test_code
       end interface test
     contains
-      function test_code()
+      real function test_code()
       end function test_code
     end module test_mod
     '''
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_save_stmts_test.py b/src/psyclone/tests/psyir/frontend/fparser2_save_stmts_test.py
new file mode 100644
index 000000000..3b16be727
--- /dev/null
+++ b/src/psyclone/tests/psyir/frontend/fparser2_save_stmts_test.py
@@ -0,0 +1,232 @@
+# -----------------------------------------------------------------------------
+# BSD 3-Clause License
+#
+# Copyright (c) 2023, Science and Technology Facilities Council.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# * Redistributions of source code must retain the above copyright notice, this
+#   list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright notice,
+#   this list of conditions and the following disclaimer in the documentation
+#   and/or other materials provided with the distribution.
+#
+# * Neither the name of the copyright holder nor the names of its
+#   contributors may be used to endorse or promote products derived from
+#   this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+# -----------------------------------------------------------------------------
+# Author: A. R. Porter, STFC Daresbury Lab
+# -----------------------------------------------------------------------------
+
+''' Tests Fortran save statements in the fparser2 PSyIR front-end '''
+
+import pytest
+
+from psyclone.errors import GenerationError
+from psyclone.psyir.nodes import Routine, Literal
+from psyclone.psyir.symbols import (StaticInterface, DefaultModuleInterface,
+                                    AutomaticInterface, UnknownFortranType,
+                                    UnknownInterface, INTEGER_TYPE)
+
+
+def test_save_statement_module(fortran_reader):
+    '''Tests that save statement and attribute are correctly captured when
+    they appear in a module, irrespective of ordering.'''
+    code = '''
+      module my_mod
+        save :: var3
+        integer :: var1
+        integer, save :: var2
+        save :: var1
+        integer :: var3, var4
+      end module my_mod'''
+    psyir = fortran_reader.psyir_from_source(code)
+    symtab = psyir.children[0].symbol_table
+    var1 = symtab.lookup("var1")
+    assert var1.is_constant is False
+    assert isinstance(var1.interface, StaticInterface)
+    var2 = symtab.lookup("var2")
+    assert isinstance(var2.interface, StaticInterface)
+    var3 = symtab.lookup("var3")
+    assert isinstance(var3.interface, StaticInterface)
+    var4 = symtab.lookup("var4")
+    assert isinstance(var4.interface, DefaultModuleInterface)
+
+
+def test_default_save_module(fortran_reader):
+    '''
+    Test that all Symbols declared in a module are given a StaticInterface if
+    there is a SAVE statement.
+    '''
+    code = '''
+      module my_mod
+        integer :: var1, var4
+        integer :: var2
+        save
+        integer :: var3
+      end module my_mod'''
+    psyir = fortran_reader.psyir_from_source(code)
+    symtab = psyir.children[0].symbol_table
+    for var in ["var1", "var2", "var3", "var4"]:
+        varsym = symtab.lookup(var)
+        assert isinstance(varsym.interface, StaticInterface)
+
+
+def test_save_invalid_fortran(fortran_reader):
+    '''
+    The Fortran standard (2008) says:
+
+    C580: If a SAVE statement with an omitted saved entity list appears in a
+    scoping unit, no other appearance of the SAVE attr-spec or SAVE statement
+    is permitted in that scoping unit.
+
+    This should be picked up by fparser2 but isn't so we catch it in PSyclone.
+    '''
+    code = '''
+      module my_mod
+        integer :: var2
+        save
+        integer, save :: var3
+      end module my_mod'''
+    with pytest.raises(GenerationError) as err:
+        _ = fortran_reader.psyir_from_source(code)
+    assert ("'INTEGER, SAVE :: var3'. Symbol 'sym_name' is the subject of a "
+            "SAVE statement but also has a SAVE attribute on its declaration"
+            in str(err.value))
+    code = '''
+      module my_mod
+        integer :: var2, var3
+        save
+        save :: var3
+      end module my_mod'''
+    with pytest.raises(GenerationError) as err:
+        _ = fortran_reader.psyir_from_source(code)
+    assert ("Supplied nodes contain a SAVE without a saved-entity list plus "
+            "one or more SAVES *with* saved-entity lists (naming ['var3']). "
+            "This is not valid Fortran" in str(err.value))
+
+
+def test_save_statement_subroutine(fortran_reader):
+    '''Tests that save statements and attributes are correctly captured when
+    they appear in a subroutine.'''
+    code = '''
+      subroutine my_sub()
+        save :: var3
+        integer :: var1, var4
+        integer, save :: var2
+        save :: var1
+        integer :: var3
+        write (*,*) var1, var2, var3
+      end subroutine my_sub
+'''
+    psyir = fortran_reader.psyir_from_source(code)
+    symtab = psyir.children[0].symbol_table
+    for var in ["var1", "var2", "var3"]:
+        varsym = symtab.lookup(var)
+        assert isinstance(varsym.interface, StaticInterface)
+    sym = symtab.lookup("var4")
+    assert isinstance(sym.interface, AutomaticInterface)
+
+
+@pytest.mark.parametrize("declns",
+                         ["        integer, parameter :: var1 = 1\n"
+                          "        save\n",
+                          "        parameter(var1=1)\n"
+                          "        integer, save :: var1\n"])
+def test_save_and_parameter(fortran_reader, declns):
+    '''
+    Check that when a variable is effectively declared with both the parameter
+    and save attributes the resulting Symbol has the correct properties.
+    (Although Fortran forbids 'parameter' and 'save' from appearing in the
+    same declaration statement, it doesn't say anything about situations
+    where they are separate and gfortran [at least] permits this.)
+
+    '''
+    code = f'''
+      subroutine my_sub()
+{declns}
+        write(*,*) var1
+      end subroutine my_sub
+'''
+    psyir = fortran_reader.psyir_from_source(code)
+    symtab = psyir.walk(Routine)[0].symbol_table
+    var1 = symtab.lookup("var1")
+    assert var1.is_constant
+    assert var1.is_static
+    assert var1.initial_value == Literal("1", INTEGER_TYPE)
+
+
+def test_save_and_unsupported_attr(fortran_reader):
+    '''
+    Check that a SAVE statement does not have any effect on a variable
+    declared with an unsupported attribute.
+    '''
+    code = '''
+      subroutine my_sub()
+        integer, volatile :: var1
+        save
+        write(*,*) var1
+      end subroutine my_sub
+'''
+    psyir = fortran_reader.psyir_from_source(code)
+    symtab = psyir.walk(Routine)[0].symbol_table
+    var1 = symtab.lookup("var1")
+    assert isinstance(var1.datatype, UnknownFortranType)
+    assert isinstance(var1.interface, UnknownInterface)
+
+
+def test_save_common_module(fortran_reader):
+    '''
+    Check that SAVE statements involving a named Common Block in a module
+    and a subroutine are correctly captured.
+    '''
+    code = '''
+      module my_mod
+        save :: var3, /my_common/
+        common /my_common/ a, b
+        integer :: var1
+        integer :: a, b
+        integer :: var3
+      contains
+        subroutine my_sub()
+          save :: /some_other_common/
+          common /some_other_common/ igorina
+          real :: igorina
+        end subroutine my_sub
+      end module my_mod'''
+    psyir = fortran_reader.psyir_from_source(code)
+    symtab = psyir.children[0].symbol_table
+    var3 = symtab.lookup("var3")
+    assert isinstance(var3.interface, StaticInterface)
+    for sym in symtab.symbols:
+        if isinstance(sym.datatype, UnknownFortranType):
+            assert sym.name.lower().startswith("_psyclone_internal_save")
+            assert sym.datatype._declaration == "SAVE :: /my_common/"
+            break
+    else:  # pragma: no cover
+        assert False, "No Symbol of UnknownFortranType found"
+
+    sub = psyir.walk(Routine)[0]
+    for sym in sub.symbol_table.symbols:
+        if isinstance(sym.datatype, UnknownFortranType):
+            assert sym.name.lower().startswith("_psyclone_internal_save")
+            assert sym.datatype._declaration == "SAVE :: /some_other_common/"
+            break
+    else:  # pragma: no cover
+        assert False, "No Symbol of UnknownFortranType found in nested table"
diff --git a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py
index 081d68c40..a2943357e 100644
--- a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py
+++ b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py
@@ -49,7 +49,7 @@ from psyclone.psyir.frontend.fparser2 import (Fparser2Reader,
 from psyclone.psyir.nodes import Container, Routine, CodeBlock, FileContainer
 from psyclone.psyir.symbols import (DataSymbol, DeferredType, NoType,
                                     RoutineSymbol, ScalarType,
-                                    UnknownFortranType)
+                                    SymbolError, UnknownFortranType)
 
 IN_OUTS = []
 # subroutine no declarations
@@ -279,9 +279,10 @@ def test_function_result_suffix(fortran_reader, fortran_writer,
 
 def test_function_missing_return_type(fortran_reader):
     '''
-    Test that we generate a CodeBlock for a Fortran function without an
-    explicit declaration of its return type (i.e. if it's relying on Fortran's
-    implicit typing).
+    Test that we reject a Fortran function without an explicit declaration of
+    its return type (i.e. if it's relying on Fortran's implicit typing). We
+    can't put such a function in a CodeBlock because we generate code with
+    `implicit none` specified.
 
     '''
     code = (
@@ -291,8 +292,25 @@ def test_function_missing_return_type(fortran_reader):
         "    my_func = 1.0\n"
         "  end function my_func\n"
         "end module\n")
-    psyir = fortran_reader.psyir_from_source(code)
-    assert isinstance(psyir.children[0].children[0], CodeBlock)
+    with pytest.raises(SymbolError) as err:
+        _ = fortran_reader.psyir_from_source(code)
+    assert ("No explicit return-type information found for function "
+            "'my_func'. PSyclone requires that all symbols be explicitly "
+            "typed." in str(err.value))
+    # Test where the result is specified in a suffix but there is no actual
+    # declaration of the symbol.
+    code = (
+        "module a\n"
+        "contains\n"
+        "  function my_func() result(some_var)\n"
+        "    some_var = 1.0\n"
+        "  end function my_func\n"
+        "end module\n")
+    with pytest.raises(SymbolError) as err:
+        _ = fortran_reader.psyir_from_source(code)
+    assert ("No explicit return-type information found for function "
+            "'my_func'. PSyclone requires that all symbols be explicitly "
+            "typed." in str(err.value))
 
 
 def test_function_unsupported_type(fortran_reader):
diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py
index 80aa71ca4..07a595756 100644
--- a/src/psyclone/tests/psyir/symbols/datatype_test.py
+++ b/src/psyclone/tests/psyir/symbols/datatype_test.py
@@ -626,6 +626,43 @@ def test_unknown_fortran_type_text():
     # Calling it a second time should just return the previously cached
     # result.
     assert utype.type_text is text
+    # Test for a SAVE for a common block.
+    decl2 = "save :: /a_common_fault/"
+    utype2 = UnknownFortranType(decl2)
+    assert utype2.type_text == "SAVE"
+    # Test for a Common block.
+    decl3 = "common /name1/ a, b, c"
+    utype3 = UnknownFortranType(decl3)
+    assert utype3.type_text == "COMMON"
+
+
+def test_unknown_fortran_type_text_error():
+    '''
+    Check that the expected error is raised if the 'type_text' is requested
+    for something that is not a straightforward declaration.
+    '''
+    decl = "10 format('4I4')"
+    utype = UnknownFortranType(decl)
+    with pytest.raises(NotImplementedError) as err:
+        utype.type_text
+    assert ("Cannot extract the declaration part from UnknownFortranType "
+            "'10 format('4I4')'. Only Declaration_Construct, "
+            "Type_Declaration_Stmt, Save_Stmt and Common_Stmt are supported "
+            "but got 'Implicit_Part' from the parser." in str(err.value))
+    # Valid Fortran but not a declaration.
+    utype = UnknownFortranType("call fn(a)")
+    with pytest.raises(NotImplementedError) as err:
+        utype.type_text
+    assert ("Cannot extract the declaration part from UnknownFortranType "
+            "'call fn(a)' because parsing (attempting to match a "
+            "Fortran2003.Specification_Part) failed." in str(err.value))
+    # Invalid Fortran.
+    utype = UnknownFortranType("not valid fortran")
+    with pytest.raises(NotImplementedError) as err:
+        utype.type_text
+    assert ("Cannot extract the declaration part from UnknownFortranType "
+            "'not valid fortran' because parsing (attempting to match a "
+            "Fortran2003.Specification_Part) failed." in str(err.value))
 
 
 def test_unknown_fortran_type_eq():
@@ -637,6 +674,15 @@ def test_unknown_fortran_type_eq():
     # Type is the same even if the variable name is different.
     assert utype == UnknownFortranType("type(some_type) :: var1")
     assert utype != UnknownFortranType("type(other_type) :: var")
+    # A common block is the same type as another common block.
+    assert (UnknownFortranType("common /how_common/ a, b, cc") ==
+            UnknownFortranType("common /common_land/ a, b, cc"))
+    # A SAVE statement is the same type as another SAVE statement.
+    assert (UnknownFortranType("save :: /how_common/") ==
+            UnknownFortranType("save :: blue_blood"))
+    # Just sanity check that the type of a SAVE != that of a common.
+    assert (UnknownFortranType("common /how_common/ a, b, cc") !=
+            UnknownFortranType("save :: blue_blood"))
 
 
 # StructureType tests