Add ArrayConstructor to PSyIR#3458
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3458 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 393 395 +2
Lines 55065 55204 +139
==========================================
+ Hits 55065 55204 +139 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks @mn416, I think this will be useful - we hit some limitations to do with this sometimes. Does your implementation support the old |
|
Hi @arporter, yes the old and new brackets are both supported. They are parsed to almost identical abstract syntax by |
|
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? |
|
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). |
|
Absolutely, I'm all in favour of simple PRs 😀 |
I noticed that
reference_accesses()is very conservative on array constructors, e.g.[x, y]leads to bothxandybeing reported as read/write. This is because array constructors are treated asCodeBlocks and not converted to PSyIR.This PR adds initial support for array constructors to PSyIR via a new
ArrayConstructornode 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.