(Closes #2612) add support for length and precision attributes in character type by arporter · Pull Request #3377 · stfc/PSyclone

arporter · 2026-03-16T19:37:29Z

No description provided.

codecov · 2026-03-18T17:40:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.96%. Comparing base (7b6d98a) to head (5508155).

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #3377    +/-   ##
========================================
  Coverage   99.95%   99.96%            
========================================
  Files         387      388     +1     
  Lines       54317    54485   +168     
========================================
+ Hits        54295    54465   +170     
+ Misses         22       20     -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

arporter · 2026-03-23T16:45:55Z

This is a little bigger than I'd have liked because, in quite a few tests, we use character as a "type that we don't support". I've kicked this can down the road a bit by switching to complex but we will also want to add support for that soon. Perhaps at that point there will be no basic type that we don't support?

arporter · 2026-03-23T17:12:22Z

Something goes very wrong in the NEMO ITs:

I don't have a test with len=some_intrinsic(arg)...

arporter · 2026-03-24T10:49:18Z

(Pdb) print(sym)
cd_mpp: DataSymbol<UnsupportedFortranType('CHARACTER(LEN = 3), OPTIONAL, INTENT(IN) :: cd_mpp'), Argument(Access.UNKNOWN)>

arporter · 2026-03-25T17:13:27Z

NEMO v4 IT still fails:

psyclone/psyir/transformations/acc_kernels_trans.py", line 274, in validate
3198
    if sym.datatype.intrinsic != ScalarType.Intrinsic.CHARACTER:
3199
       ^^^^^^^^^^^^^^^^^^^^^^
psyclone/psyir/symbols/datatypes.py", line 325, in intrinsic
    return self.partial_datatype.intrinsic
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'DataTypeSymbol' object has no attribute 'intrinsic'

arporter · 2026-03-31T13:29:30Z

I declare this one ready for review too. I haven't yet had a green run on the NEMOv4 integration tests but I've tried it manually and I think it's OK. We can run the ITs once Aidan's benchmarking is done.

sergisiso

Thanks @arporter it's good to see additional symbols have a proper datatype. See mostly minor comments but the NEMOv4 integration test has also failed.

sergisiso · 2026-04-09T11:36:44Z

doc/developer_guide/psyir_symbols.rst

+|Initialisation        |Explicit            | Implicit loops,    |
+|expressions           |initialisation      | array constructors |


I suppose by supported we mean that it produces an ArrayType datasymbol with the initialisation expression being a CodeBlock?

I am ok with it if that's the case, but to be sure they work, can you add an example of each in src/psyclone/tests/psyir/frontend/fparser2_test.py, in test_process_array_declarations or test_declarations_with_initialisations (unless it is already tested somewhere else, but I couldn't find it)

sergisiso · 2026-04-09T13:37:26Z

src/psyclone/psyir/nodes/array_reference.py

+                    # TODO #3240 - sometimes we have an ArrayReference that is
+                    # actually a character sub-string.
+                    base_type = self.symbol.datatype.copy()


Add a test in src/psyclone/tests/psyir/nodes/array_reference_test.py::test_array_datatype

sergisiso · 2026-04-09T13:42:51Z

src/psyclone/psyir/symbols/datatypes.py

+
+        def debug_string(self) -> str:
+            ''':returns: the name of the Enum item.'''
+            return self.name


pylint says "Instance of 'ScalarTypeAttribute' has no 'name' member"

Can we make ScalarTypeAttribute inherit from Enum, instead of each subclass having multiple-inheritance?

sergisiso · 2026-04-09T13:48:59Z

src/psyclone/psyir/symbols/datatypes.py

+    class CharLengthParameter(ScalarTypeAttribute, Enum):
+        ASTERISK = 1
+        COLON = 2


pylint wants a docstring.

Also, can we give the elements a more semantic name since this is PSyIR. From the fortran standard I see:
So maybe COLON->DEFERRED and ASTERISK->ASSUMED ?

sergisiso · 2026-04-09T13:51:54Z

src/psyclone/psyir/symbols/datatypes.py

+        from psyclone.psyir.nodes.datanode import DataNode
+        if isinstance(value, ScalarType.CharLengthParameter):
+            self._length = value
+        elif isinstance(value, int) and not isinstance(value, bool):


Is the and not isinstance(value, bool) necessary?

sergisiso · 2026-04-09T13:55:13Z

src/psyclone/psyir/symbols/datatypes.py

+                f"The length property of a character ScalarType must be an "
+                f"int, ScalarType.CharLengthParameter "


"a non-negative int ..."

sergisiso · 2026-04-09T14:01:11Z

src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py

+  type(a_type) :: dtype
+  type(d_type), pointer :: dtype_ptr
+  type(a_type), dimension(10) :: type_list


dtype_ptr and type_list are not used in the assignments below, are they needed? or are there assignments missing?

sergisiso · 2026-04-09T14:08:33Z

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

              subroutine test()
                  integer :: a
-                  a = unsupported_type_symbol()
+                  a = INT(REAL(unsupported_type_symbol()))


Why the INT(REAL(?

sergisiso · 2026-04-09T14:17:41Z

src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py

    end subroutine test'''
    psyir = fortran_reader.psyir_from_source(code)

+    trans = ArrayAssignment2LoopsTrans()


There is a TODO to 2612 in this file line 631.

Also in src/psyclone/psyir/nodes/intrinsic_call.py
794: # TODO 2612 This may be more complex if we support character len
813: # TODO 2612 This may be more complex if we support character len

arporter added 3 commits March 9, 2026 10:12

#2612 add length property to ScalarType

ad3bddf

Merge branch 'master' into 2612_add_len_to_scalartype

4ddb6b2

#2612 add routine to handle char-length specification [skip ci]

5bb616c

arporter marked this pull request as draft March 16, 2026 19:37

arporter added 10 commits March 17, 2026 10:08

#2612 WIP fixing tests [skip ci]

4deeb2b

#2612 fix len and kind handling for characters [skip ci]

e11294e

#2612 fix select-type tests [skip ci]

a4e1f40

#2612 fix more psyir tests [skip ci]

1532776

Merge branch 'master' into 2612_add_len_to_scalartype

2fa3db9

#2612 add support for var*len type declarations [skip ci]

37fda47

Merge branch 'master' into 2612_add_len_to_scalartype

de4404c

#2612 allow for colon/asterisk in entity decln [skip ci]

58b9d0b

#2612 fix tests and linting

e7f9cce

#2361 really fix the test

e37e4df

arporter added 6 commits March 19, 2026 18:13

#2612 improve test coverage

3409df4

Merge branch 'master' into 2612_add_len_to_scalartype

bc6dfcf

#2612 tidy implementation and improve cov

60a92f0

#2612 allow for int precision

c76ec8f

#2612 rm unnecessary refinement to copy

43ffeb1

#2612 add comment

866ec47

arporter temporarily deployed to integration March 23, 2026 16:37 — with GitHub Actions Inactive

arporter added enhancement PSyIR Core PSyIR functionality labels Mar 23, 2026

arporter marked this pull request as ready for review March 23, 2026 17:08

arporter added the in progress label Mar 23, 2026

#2612 fix bug in handling of UnsupportedFortranType

a2c4c74

arporter temporarily deployed to integration March 24, 2026 11:25 — with GitHub Actions Inactive

arporter added 4 commits March 24, 2026 14:13

#2612 allow for array of char in acc_kernels_trans

5c368e1

#2612 add test for an argument that is an array of char strings

ef200da

#2612 linting

e7d6ce1

Merge branch 'master' into 2612_add_len_to_scalartype

91996c1

arporter temporarily deployed to integration March 25, 2026 16:01 — with GitHub Actions Inactive

arporter added 3 commits March 31, 2026 13:13

Merge branch 'master' into 2612_add_len_to_scalartype

c0f0221

#2612 attempt to fix bug in check for char length

b05dbb4

#2612 update dev guide on supported PSyIR types

15b5331

arporter requested review from LonelyCat124, hiker and sergisiso March 31, 2026 13:28

arporter self-assigned this Mar 31, 2026

arporter added ready for review and removed in progress labels Mar 31, 2026

arporter changed the title ~~(Closes #2612) add support for length attribute in character type~~ (Closes #2612) add support for length and precision attributes in character type Mar 31, 2026

sergisiso added under review and removed ready for review labels Apr 9, 2026

Merge branch 'master' into 2612_add_len_to_scalartype

5508155

sergisiso deployed to integration April 9, 2026 11:23 — with GitHub Actions Active

sergisiso requested changes Apr 9, 2026

View reviewed changes

sergisiso added reviewed with actions and removed under review labels Apr 9, 2026

		\|Initialisation \|Explicit \| Implicit loops, \|
		\|expressions \|initialisation \| array constructors \|

		f"The length property of a character ScalarType must be an "
		f"int, ScalarType.CharLengthParameter "

Conversation

arporter commented Mar 16, 2026

Uh oh!

codecov bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arporter commented Mar 23, 2026

Uh oh!

arporter commented Mar 23, 2026

Uh oh!

arporter commented Mar 24, 2026

Uh oh!

arporter commented Mar 25, 2026

Uh oh!

arporter commented Mar 31, 2026

Uh oh!

sergisiso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Mar 18, 2026 •

edited

Loading