Skip to content

(Closes #2721) Add ArrayConstructor to PSyIR#3458

Merged
LonelyCat124 merged 15 commits into
masterfrom
mn416-array-constructor
Jun 23, 2026
Merged

(Closes #2721) Add ArrayConstructor to PSyIR#3458
LonelyCat124 merged 15 commits into
masterfrom
mn416-array-constructor

Conversation

@mn416

@mn416 mn416 commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

I noticed that reference_accesses() is very conservative on array constructors, e.g. [x, y] leads to both x and y being reported as read/write. This is because array constructors are treated as CodeBlocks and not converted to PSyIR.

This PR adds initial support for array constructors to PSyIR via a new ArrayConstructor node class. Currently, it only supports the most simple form of array constructor

  • [<expr>, <expr>, ..., <expr>]

not

  • [<type-spec> :: <expr>, <expr>, ..., <expr>] (type spec), or
  • [(<expr>, <name> = <expr>, <expr>)] (implied do)

which are still translated to CodeBlock, as before. I'm happy to add support for type specs and/or implied do, but I was wondering if there is any hunger for this PR before spending any more time on it.

I believe this PR addresses #2721.

@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d4d4cbc) to head (fc24bca).

Additional details and impacted files
@@            Coverage Diff             @@
##            master     #3458    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          393       395     +2     
  Lines        55067     55199   +132     
==========================================
+ Hits         55067     55199   +132     

☔ View full report in Codecov by Harness.
📢 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

Copy link
Copy Markdown
Member

Thanks @mn416, I think this will be useful - we hit some limitations to do with this sometimes. Does your implementation support the old (/x, y, z/) syntax too?

@mn416

mn416 commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @arporter, yes the old and new brackets are both supported. They are parsed to almost identical abstract syntax by fparser.

@LonelyCat124

Copy link
Copy Markdown
Collaborator

I know we do hit some implied do in some codes, but I'm not sure we have a reason to need them to not be codeblocks unless we have issues with them so I think that can be skipped for now?

@mn416

mn416 commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

I added support for type specs in the mn416-array-ctr-type-spec branch but I'm slightly less confident in it. I'm therefore inclined to agree with @LonelyCat124 that we may benefit from an incremental approach here, i.e. handle basic array constructors in this PR and add support for type specs and implied do later. An incremental approach should also simplify reviewing (several simple PRs versus one big one).

@arporter

Copy link
Copy Markdown
Member

Absolutely, I'm all in favour of simple PRs 😀

@LonelyCat124 LonelyCat124 self-requested a review June 22, 2026 09:53
@LonelyCat124

Copy link
Copy Markdown
Collaborator

Happy to review this then.

@LonelyCat124 LonelyCat124 changed the title Add ArrayConstructor to PSyIR (Closes #2721) Add ArrayConstructor to PSyIR Jun 22, 2026

@LonelyCat124 LonelyCat124 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mn416 Have left some initial comments, mostly codestyle related - I didn't go through the tests yet, I'll do that on the next pass through.

Comment thread src/psyclone/psyir/frontend/fparser2.py Outdated
Comment thread src/psyclone/psyir/backend/fortran.py Outdated
Comment thread src/psyclone/psyir/nodes/array_constructor.py Outdated
Comment thread src/psyclone/psyir/nodes/array_constructor.py Outdated
Comment thread src/psyclone/psyir/nodes/array_constructor.py Outdated
Comment thread src/psyclone/psyir/nodes/array_constructor.py Outdated
Comment thread src/psyclone/psyir/nodes/array_constructor.py Outdated
Comment thread src/psyclone/psyir/nodes/array_constructor.py Outdated
Comment thread src/psyclone/psyir/transformations/arrayassignment2loops_trans.py Outdated
Comment thread src/psyclone/tests/psyir/frontend/fparser2_array_constructor_handler_test.py Outdated
@mn416

mn416 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @LonelyCat124. I've addressed all your comments for now and I think it's ready for another pass.

@LonelyCat124 LonelyCat124 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mn416 - almost there now - one additional comment I'd liked to be added re: the unknown array size. I'll set any ITs running in the meantime.

Comment thread src/psyclone/psyir/nodes/array_constructor.py
Comment thread src/psyclone/psyir/nodes/array_constructor.py Outdated
@mn416

mn416 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @LonelyCat124. I've added the comment.

@LonelyCat124 LonelyCat124 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ITs all green, I'll proceed to merge.

@LonelyCat124 LonelyCat124 merged commit 2d08da6 into master Jun 23, 2026
15 checks passed
@LonelyCat124 LonelyCat124 deleted the mn416-array-constructor branch June 23, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants