From 2e917f98ee4ed3743aac67c3b3e94620c56daf30 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Wed, 5 Aug 2026 11:43:51 +0100 Subject: [PATCH 1/5] #3392 Store commonblock name inside its interface --- src/psyclone/psyir/backend/fortran.py | 6 ++ src/psyclone/psyir/frontend/fparser2.py | 23 +++---- src/psyclone/psyir/symbols/interfaces.py | 38 +++++++++++- .../backend/fortran_common_block_test.py | 9 ++- .../frontend/fparser2_common_block_test.py | 60 ++++++------------- .../tests/psyir/symbols/interfaces_test.py | 26 ++++++-- .../tests/psyir/symbols/symbol_table_test.py | 2 +- .../tests/psyir/symbols/symbol_test.py | 2 +- 8 files changed, 98 insertions(+), 68 deletions(-) diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index 59eb7593d2..9b512d471a 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -672,6 +672,12 @@ def gen_vardecl(self, if symbol.inline_comment != "": result += f" {self._COMMENT_PREFIX}{symbol.inline_comment}" + if isinstance(symbol, Symbol) and symbol.is_commonblock: + result += ( + f"\n{self._nindent}common /{symbol.interface.name}/ " + f"{symbol.name}" + ) + return result + "\n" def gen_interfacedecl(self, symbol): diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 011a3f1bd8..0883491283 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2904,10 +2904,9 @@ def _process_data_statements(nodes, psyir_parent): @staticmethod def _process_common_blocks(nodes, psyir_parent): - ''' Process the fparser2 common block declaration statements. This is - done after the other declarations and it will keep the statement - as a UnsupportedFortranType and update the referenced symbols to a - CommonBlockInterface. + ''' Process the fparser2 common block declaration statements. This + is done after the symbols have already been created, it just assigns + the CommonBlockInterface to them. :param nodes: fparser2 AST nodes containing declaration statements. :type nodes: List[:py:class:`fparser.two.utils.Base`] @@ -2925,22 +2924,16 @@ def _process_common_blocks(nodes, psyir_parent): ''' for node in nodes: if isinstance(node, Fortran2003.Common_Stmt): - # Place the declaration statement into a UnsupportedFortranType - # (for now we just want to reproduce it). The name of the - # commonblock is not in the same namespace as the variable - # symbols names (and there may be multiple of them in a - # single statement). So we use an internal symbol name. - psyir_parent.symbol_table.new_symbol( - root_name="_PSYCLONE_INTERNAL_COMMONBLOCK", - symbol_type=DataSymbol, - datatype=UnsupportedFortranType(str(node))) - # Get the names of the symbols accessed with the commonblock, # they are already defined in the symbol table but they must # now have a common-block interface. try: # Loop over every COMMON block defined in this Common_Stmt for cb_object in node.children[0]: + # Get the name of the common block + name = cb_object[0] + name_str = name.string if name is not None else "" + for symbol_name in cb_object[1].items: sym = psyir_parent.symbol_table.lookup( str(symbol_name)) @@ -2951,7 +2944,7 @@ def _process_common_blocks(nodes, psyir_parent): f" ({sym.initial_value.debug_string()}) " f"but appears in a common block. This is " f"not valid Fortran.") - sym.interface = CommonBlockInterface() + sym.interface = CommonBlockInterface(name_str) except KeyError as error: raise NotImplementedError( f"The symbol interface of a common block variable " diff --git a/src/psyclone/psyir/symbols/interfaces.py b/src/psyclone/psyir/symbols/interfaces.py index 833bbdd712..83da2e6c6e 100644 --- a/src/psyclone/psyir/symbols/interfaces.py +++ b/src/psyclone/psyir/symbols/interfaces.py @@ -38,6 +38,7 @@ ''' This module contains the SymbolInterface class and its subclasses. ''' from enum import Enum +from typing import Any # pylint: disable=too-few-public-methods @@ -97,10 +98,43 @@ def __str__(self): class CommonBlockInterface(SymbolInterface): ''' A symbol declared in the local scope but acts as a global that - can be accessed by any scope referencing the same CommonBlock name.''' + can be accessed by any scope referencing the same CommonBlock name. + + :param common_block_name: the name of the common block. + + :raises TypeError: if the common_block_name is not a str + ''' + + def __init__(self, common_block_name: str): + super().__init__() + if not isinstance(common_block_name, str): + raise TypeError( + f"The common block name should be a valid string, but" + f" found '{type(common_block_name).__name__}'") + self._name = common_block_name def __str__(self): - return "CommonBlock" + return f"CommonBlock '{self._name}'" + + def __eq__(self, other: Any) -> bool: + return ( + super().__eq__(other) and + self._name.lower() == other.name.lower() + ) + + @property + def name(self) -> str: + ''' + :returns: the name of the common block. + + ''' + return self._name + + def copy(self) -> 'CommonBlockInterface': + ''' + :returns: a copy of this object. + ''' + return self.__class__(self._name) class UnresolvedInterface(SymbolInterface): diff --git a/src/psyclone/tests/psyir/backend/fortran_common_block_test.py b/src/psyclone/tests/psyir/backend/fortran_common_block_test.py index 9a1b8fd084..00889ebe19 100644 --- a/src/psyclone/tests/psyir/backend/fortran_common_block_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_common_block_test.py @@ -66,13 +66,16 @@ def test_fw_common_blocks(fortran_reader, fortran_writer, tmpdir): assert code == ( "subroutine sub()\n" " integer :: a\n" + " common /name1/ a\n" " integer :: b\n" + " common /name1/ b\n" " integer :: c\n" + " common /name1/ c\n" " real :: d\n" + " common /name2/ d\n" " real :: e\n" + " common // e\n" " real :: f\n" - " COMMON /name1/ a, b\n" - " COMMON /name1/ c /name2/ d\n" - " COMMON // e, f\n\n\n" + " common // f\n\n\n" "end subroutine sub\n") assert Compile(tmpdir).string_compiles(fortran_writer(psyir)) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py b/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py index 227dd6b999..b9a1fe4eca 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py @@ -41,8 +41,7 @@ from fparser.two.Fortran2003 import Specification_Part from psyclone.psyir.frontend.fparser2 import Fparser2Reader from psyclone.psyir.nodes import Routine -from psyclone.psyir.symbols import ( - CommonBlockInterface, ScalarType, UnsupportedFortranType) +from psyclone.psyir.symbols import CommonBlockInterface, ScalarType @pytest.mark.usefixtures("f2008_parser") @@ -62,15 +61,11 @@ def test_named_common_block(): fparser2spec = Specification_Part(reader) processor.process_declarations(routine, fparser2spec.content, []) - # There is a name1 commonblock symbol - commonblock = symtab.lookup("_PSYCLONE_INTERNAL_COMMONBLOCK") - assert isinstance(commonblock.datatype, UnsupportedFortranType) - assert commonblock.datatype.declaration == "COMMON /name1/ a, b, c" - # The variables have been updated to a common block interface - assert isinstance(symtab.lookup("a").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("b").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("c").interface, CommonBlockInterface) + name1_cb = CommonBlockInterface('name1') + assert symtab.lookup("a").interface == name1_cb + assert symtab.lookup("b").interface == name1_cb + assert symtab.lookup("c").interface == name1_cb # The same common block can also bring other variables in a separate # statement @@ -81,12 +76,8 @@ def test_named_common_block(): fparser2spec = Specification_Part(reader) processor.process_declarations(routine, fparser2spec.content, []) - # This is stored in a separate symbol, but the declaration has the right - # text - commonblock_2 = symtab.lookup("_PSYCLONE_INTERNAL_COMMONBLOCK_1") - assert commonblock_2.datatype.declaration == "COMMON /name1/ d, e, f" - assert isinstance(symtab.lookup("d").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("e").interface, CommonBlockInterface) + assert symtab.lookup("d").interface == name1_cb + assert symtab.lookup("e").interface == name1_cb fsym = symtab.lookup("f") assert isinstance(fsym.interface, CommonBlockInterface) assert fsym.datatype.intrinsic is ScalarType.Intrinsic.REAL @@ -108,15 +99,11 @@ def test_unnamed_commonblock(): fparser2spec = Specification_Part(reader) processor.process_declarations(routine, fparser2spec.content, []) - # There is an UnsupportedFortranType symbol containing the commonblock - commonblock = symtab.lookup("_PSYCLONE_INTERNAL_COMMONBLOCK") - assert isinstance(commonblock.datatype, UnsupportedFortranType) - assert commonblock.datatype.declaration == "COMMON // a, b, c" - - # The variables have been updated to a common block interface - assert isinstance(symtab.lookup("a").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("b").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("c").interface, CommonBlockInterface) + # The variables have been updated to the unnamed common block interface + unnamed_cb = CommonBlockInterface("") + assert symtab.lookup("a").interface == unnamed_cb + assert symtab.lookup("b").interface == unnamed_cb + assert symtab.lookup("c").interface == unnamed_cb @pytest.mark.usefixtures("f2008_parser") @@ -137,19 +124,13 @@ def test_multiple_commonblocks_in_statement(): fparser2spec = Specification_Part(reader) processor.process_declarations(routine, fparser2spec.content, []) - # There is a UnsupportedFortranType symbol containing each the commonblock - commonblock = symtab.lookup("_PSYCLONE_INTERNAL_COMMONBLOCK") - assert isinstance(commonblock.datatype, UnsupportedFortranType) - assert commonblock.datatype.declaration == "COMMON /name1/ a, b /name2/ c" - commonblock = symtab.lookup("_PSYCLONE_INTERNAL_COMMONBLOCK_1") - assert isinstance(commonblock.datatype, UnsupportedFortranType) - assert commonblock.datatype.declaration == "COMMON /name2/ d" - # The variables have been updated to a common block interface - assert isinstance(symtab.lookup("a").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("b").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("c").interface, CommonBlockInterface) - assert isinstance(symtab.lookup("d").interface, CommonBlockInterface) + name1_cb = CommonBlockInterface('name1') + name2_cb = CommonBlockInterface('name2') + assert symtab.lookup("a").interface == name1_cb + assert symtab.lookup("b").interface == name1_cb + assert symtab.lookup("c").interface == name2_cb + assert symtab.lookup("d").interface == name2_cb @pytest.mark.usefixtures("f2008_parser") @@ -169,11 +150,6 @@ def test_named_commonblock_with_posterior_declaration(): fparser2spec = Specification_Part(reader) processor.process_declarations(routine, fparser2spec.content, []) - # There is an UnsupportedFortranType symbol containing the commonblock - commonblock = symtab.lookup("_PSYCLONE_INTERNAL_COMMONBLOCK") - assert isinstance(commonblock.datatype, UnsupportedFortranType) - assert commonblock.datatype.declaration == "COMMON /name1/ a, b" - # The variables have been updated to a common block interface assert isinstance(symtab.lookup("a").interface, CommonBlockInterface) assert isinstance(symtab.lookup("b").interface, CommonBlockInterface) diff --git a/src/psyclone/tests/psyir/symbols/interfaces_test.py b/src/psyclone/tests/psyir/symbols/interfaces_test.py index d0a2f316e8..3fb84ff31d 100644 --- a/src/psyclone/tests/psyir/symbols/interfaces_test.py +++ b/src/psyclone/tests/psyir/symbols/interfaces_test.py @@ -91,12 +91,30 @@ def test_static_interface(): def test_commonblockinterface(): - '''Test we can create an CommonBlockInterface instance and check its - __str__ value + '''Test we can create an CommonBlockInterface instance and + __str__, __eq__, copy, and get its name. ''' - interface = CommonBlockInterface() - assert str(interface) == "CommonBlock" + interface = CommonBlockInterface("name") + interface.name == "name" + assert str(interface) == "CommonBlock 'name'" + + # Interfaces can be unnamed + interface2 = CommonBlockInterface("") + interface2.name == "" + assert str(interface2) == "CommonBlock ''" + + # Check that they only accept strings + with pytest.raises(TypeError) as err: + _ = CommonBlockInterface(3) + assert ("The common block name should be a valid string, but found 'int'" + in str(err.value)) + + # Test copy and equality + assert interface != interface2 + copy = interface.copy() + assert interface is not copy + assert interface == copy def test_unresolvedinterface(): diff --git a/src/psyclone/tests/psyir/symbols/symbol_table_test.py b/src/psyclone/tests/psyir/symbols/symbol_table_test.py index 6fc11d2c85..cb6d6c3aea 100644 --- a/src/psyclone/tests/psyir/symbols/symbol_table_test.py +++ b/src/psyclone/tests/psyir/symbols/symbol_table_test.py @@ -2918,7 +2918,7 @@ def test_rename_symbol_errors(): # Cannot rename a common block symbol asym = symbols.DataSymbol("a", symbols.ScalarType.integer_type(), - interface=symbols.CommonBlockInterface()) + interface=symbols.CommonBlockInterface("")) table.add(asym) with pytest.raises(symbols.SymbolError) as err: table.rename_symbol(asym, "b") diff --git a/src/psyclone/tests/psyir/symbols/symbol_test.py b/src/psyclone/tests/psyir/symbols/symbol_test.py index 745722975a..08624c2300 100644 --- a/src/psyclone/tests/psyir/symbols/symbol_test.py +++ b/src/psyclone/tests/psyir/symbols/symbol_test.py @@ -163,7 +163,7 @@ def test_symbol_interface_setter_and_is_properties(): assert not symbol.is_commonblock assert not symbol.is_unknown_interface - symbol.interface = CommonBlockInterface() + symbol.interface = CommonBlockInterface("") assert not symbol.is_automatic assert not symbol.is_import assert not symbol.is_argument From d99323810a1c75f4be43fe4552936c998a46f7ad Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Wed, 12 Aug 2026 09:52:50 +0100 Subject: [PATCH 2/5] #3392 Improve docstrings --- src/psyclone/psyir/frontend/fparser2.py | 4 +--- src/psyclone/tests/psyir/symbols/interfaces_test.py | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 0883491283..0117366fd9 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -2903,16 +2903,14 @@ def _process_data_statements(nodes, psyir_parent): sym.interface = StaticInterface() @staticmethod - def _process_common_blocks(nodes, psyir_parent): + def _process_common_blocks(nodes: list[Base], psyir_parent: ScopingNode): ''' Process the fparser2 common block declaration statements. This is done after the symbols have already been created, it just assigns the CommonBlockInterface to them. :param nodes: fparser2 AST nodes containing declaration statements. - :type nodes: List[:py:class:`fparser.two.utils.Base`] :param psyir_parent: the PSyIR Node with a symbol table in which to add the Common Blocks and update the symbols interfaces. - :type psyir_parent: :py:class:`psyclone.psyir.nodes.ScopingNode` :raises NotImplementedError: if one of the Symbols in a common block has initialisation (including when it is a parameter). This is not diff --git a/src/psyclone/tests/psyir/symbols/interfaces_test.py b/src/psyclone/tests/psyir/symbols/interfaces_test.py index 3fb84ff31d..4d8da06b54 100644 --- a/src/psyclone/tests/psyir/symbols/interfaces_test.py +++ b/src/psyclone/tests/psyir/symbols/interfaces_test.py @@ -91,8 +91,8 @@ def test_static_interface(): def test_commonblockinterface(): - '''Test we can create an CommonBlockInterface instance and - __str__, __eq__, copy, and get its name. + '''Test we can create an CommonBlockInterface instance and tests its + __str__, __eq__, copy, and property methods. ''' interface = CommonBlockInterface("name") From 58196e83ecabfd922224b758927250fe53b59df2 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Wed, 12 Aug 2026 10:29:19 +0100 Subject: [PATCH 3/5] #3392 Add a test mising commonblocks and comments --- .../psyir/frontend/fparser2_common_block_test.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py b/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py index b9a1fe4eca..eafc7c34f9 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py @@ -107,9 +107,9 @@ def test_unnamed_commonblock(): @pytest.mark.usefixtures("f2008_parser") -def test_multiple_commonblocks_in_statement(): +def test_multiple_commonblocks_and_comments(): ''' Test that common block statements with multiple common blocks - are handled correctly.''' + and comments are handled correctly.''' # Create a dummy test routine routine = Routine.create("test_routine") @@ -119,8 +119,12 @@ def test_multiple_commonblocks_in_statement(): # And provide a common block containing two named blocks reader = FortranStringReader(''' integer :: a, b, c, d - common /name1/ a, b /name2/ c - common /name2/ d''') + ! This is the first common block + common /name1/ a, b /name2/ c ! Inline comment + ! This is the second common block + common /name2/ d ! Inline comment + ! Comment after + ''') fparser2spec = Specification_Part(reader) processor.process_declarations(routine, fparser2spec.content, []) From 79d2b18bfd6d27514b60cb215507f9056a1c2b8e Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Wed, 12 Aug 2026 10:59:35 +0100 Subject: [PATCH 4/5] #3392 Test mixing common blocks and comments --- src/psyclone/psyir/frontend/fparser2.py | 2 ++ .../tests/psyir/frontend/fparser2_common_block_test.py | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 9d421a7742..55d23f468f 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -1100,6 +1100,8 @@ def _fparser2_tree_from_fparser2_reader( parse_tree = Fortran2003.Pointer_Assignment_Stmt(source_code) elif partial_code == "statement": parse_tree = Fortran2003.Execution_Part(reader) + elif partial_code == "specs": + parse_tree = Fortran2003.Specification_Part(reader) # When parsing intermediate expressione a None value # is the same as a NoMatch, unrecognised 'partial_code' # values will also be considered a NoMatch diff --git a/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py b/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py index 2de7d306d0..fac599da67 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_common_block_test.py @@ -88,7 +88,7 @@ def test_multiple_commonblocks_and_comments(): processor = Fparser2Reader() # And provide a common block containing two named blocks - reader = FortranStringReader(''' + code = (''' integer :: a, b, c, d ! This is the first common block common /name1/ a, b /name2/ c ! Inline comment @@ -96,7 +96,8 @@ def test_multiple_commonblocks_and_comments(): common /name2/ d ! Inline comment ! Comment after ''') - fparser2spec = Specification_Part(reader) + fparser2spec = processor.generate_parse_tree_from_source( + code, partial_code="specs") processor.process_declarations(routine, fparser2spec.content, []) # The variables have been updated to a common block interface @@ -107,6 +108,9 @@ def test_multiple_commonblocks_and_comments(): assert symtab.lookup("c").interface == name2_cb assert symtab.lookup("d").interface == name2_cb + # The comments are currently discarded + assert symtab.lookup("a").preceding_comment == "" + @pytest.mark.usefixtures("f2008_parser") def test_named_commonblock_with_posterior_declaration(): From f164de23820da0a0daf9e5cb1b23cd899952283c Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 12 Aug 2026 13:27:03 +0100 Subject: [PATCH 5/5] Update changelog --- changelog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelog b/changelog index 7f525778d1..87c09015e0 100644 --- a/changelog +++ b/changelog @@ -1,3 +1,6 @@ + 28) PR #3543 for #3392. Stores commonblock names inside their interfaces + and adds them into the output. + 27) PR #3542 towards #3516. Improve handling of declaration comments. 26) PR #3545 for #3537. Fix issues with WHERE constructs with comments.