(closes #2884) Allow WHERE with imported symbols by changing Ref2ArrayRange behaviour to do the validations #3260

sergisiso · 2025-12-15T08:31:34Z

Change Ref2ArrayRange behaviour by letting it pass if the provided node is already in the expanded form, but making it fail if for a expression we cannot guarantee if it could be expanded or not. This will bring validations that now need to be done every time its used to inside the transformation.

codecov · 2025-12-15T12:15:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.91%. Comparing base (97dd02d) to head (c350586).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3260      +/-   ##
==========================================
- Coverage   99.91%   99.91%   -0.01%     
==========================================
  Files         376      376              
  Lines       53529    53522       -7     
==========================================
- Hits        53484    53477       -7     
  Misses         45       45

☔ 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.

…ayrange_behaviour

sergisiso · 2025-12-17T15:56:27Z

@arporter @LonelyCat124 This is ready for review. It changes the Reference2ArrayReference to fail if the outcome cannot be guaranteed, so thing like WHERE can rely on its validation.

This will be followed by #1858 (structures) and #2722 (dependencies), which can be done in a single location after this PR, but it was to big to do here.

arporter

Really nice Sergi, it's good to see the simplification that this has achieved.
I'm worried that we are no longer raising an exception when we see a StructureReference - given that we can't handle them this needs to be flagged?
Apart from that, it's just minor tidying. I'll run the integration tests next time (or you could fire them off perhaps once you're done).

src/psyclone/psyir/transformations/reference2arrayrange_trans.py

arporter · 2025-12-18T14:20:03Z

src/psyclone/psyir/transformations/reference2arrayrange_trans.py

-        # TODO issue #1858. Add support for structures containing arrays.
+        if node and node.parent and isinstance(node.parent, Call):
+            if node.position == 0:
+                return


Comment please - I didn't readily understand at first but presumably this is checking that the node isn't the Reference associated with the routine target of a Call?

Exactly. Added comment

You probably don't need the and node.parent part of L120 as, if it's None, it won't pass the isinstance(node.parent, Call)?

src/psyclone/psyir/transformations/reference2arrayrange_trans.py

src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py

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

arporter · 2025-12-18T14:45:37Z

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

-        trans.validate(reference)
-    assert ("node is passed as an argument to a Call to a non-elemental "
-            "routine (DEALLOCATE(a)" in str(info.value))
+        trans.validate(assign.lhs)


The docstring says we test that a StructureReference raises an exception but clearly it doesn't any longer. Given that we don't support them yet, I would argue we should raise an exception as we don't know what to do with them.

It felt bad to have the comment that are not supported and then just return the validation.

The reason I did it is because both the WHERE and the ArrayAssignment2Loop already implement and have several tests for structure references (which they happily take at face-value, without checking if they need to be added ranges).
Adding this restriction here, without implementing proper support for letting valid cases pass, and array types extended, makes all of those fail.

Also this is not worse than master, in master it had an exception but it was "except:pass" everywhere it was used.

I suggest I tackle this in a follow up (I attempted starting it but it quickly grows very large), or we can block this for now. Let me know what you prefer.

I implemented it in #3266

Thanks Sergi. As you're in the process of doing #3266, please could you just update this test with TODO to that PR (or would you like to get that PR on first?).

or would you like to get that PR on first?

I based that PR after this one's HEAD, so I can't change the order. The other PR is ready to go once this one is merge.

I added the TODO just before the test to avoid conflicts as I already modified this test significantly in the follow up.

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

sergisiso · 2025-12-19T15:16:39Z

@arporter This is ready for another look, I just fired the integration test.

arporter

Thanks Sergi. Just a bit more tidying to do and we have some indirect coverage changes:

I think you can probably just remove the try...except from that bit of code?

arporter · 2026-01-05T09:10:07Z

src/psyclone/psyir/transformations/reference2arrayrange_trans.py

+    to do so (e.g. it won't convert call arguments because it would change the
+    bounds values).
+
+    Note that if the provided node does not need to be modified is provided (


s/is provided// but I can do this if there's nothing else.

arporter · 2026-01-05T09:13:01Z

src/psyclone/psyir/transformations/reference2arrayrange_trans.py

-        # TODO issue #1858. Add support for structures containing arrays.
+        if node and node.parent and isinstance(node.parent, Call):
+            if node.position == 0:
+                return


You probably don't need the and node.parent part of L120 as, if it's None, it won't pass the isinstance(node.parent, Call)?

src/psyclone/psyir/transformations/reference2arrayrange_trans.py

arporter · 2026-01-05T09:22:51Z

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

-    '''Test that a reference in a PointerAssignment raises an exception. '''
+def test_pointer_assignment(fortran_reader, fortran_writer):
+    ''' Test that a reference in a PointerAssignment raises an exception
+    Pointer and target attributes (currently represented by partial_datatype


Pls close the parentheses after "partial_datatype"

arporter · 2026-01-05T09:26:54Z

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

-        trans.validate(reference)
-    assert ("node is passed as an argument to a Call to a non-elemental "
-            "routine (DEALLOCATE(a)" in str(info.value))
+        trans.validate(assign.lhs)


Thanks Sergi. As you're in the process of doing #3266, please could you just update this test with TODO to that PR (or would you like to get that PR on first?).

sergisiso · 2026-01-05T17:26:35Z

I think you can probably just remove the try...except from that bit of code?

I already tried this but it didn't work. Unfortunately psyad is unsafe in master as it considers any imported symbol is scalar or a non-elemental call. This happens to be true but it is unsafe. Ultimately we need to remove the except as the transformation guarantees this but if I do it now there is a big regression because most of the psyad real code has imports that are unchecked. We need capabilities to follow imports in psyad and its build system. I modified a test to show this and created an issue and TODOs.

sergisiso · 2026-01-05T20:04:30Z

@arporter This is ready for another look

sergisiso added 2 commits December 15, 2025 08:10

#2884 Change Ref2ArrayRange behaviour

a82e8e5

#2884 Improve comments

db67564

sergisiso temporarily deployed to integration December 15, 2025 08:32 — with GitHub Actions Inactive

#2884 Add missing line

0a3d225

sergisiso temporarily deployed to integration December 15, 2025 09:24 — with GitHub Actions Inactive

#2884 Fix examples

c80f055

sergisiso temporarily deployed to integration December 15, 2025 16:14 — with GitHub Actions Inactive

sergisiso and others added 2 commits December 16, 2025 13:38

Merge remote-tracking branch 'origin/master' into 2884_change_ref2arr…

9311edc

…ayrange_behaviour

#2884 Simplify fparser2 where handle with new implementation

2bf6e5e

sergisiso temporarily deployed to integration December 16, 2025 15:32 — with GitHub Actions Inactive

#2884 Change order of conditions in ref2arrayref

38e2b65

sergisiso requested review from LonelyCat124 and arporter December 17, 2025 15:47

sergisiso self-assigned this Dec 17, 2025

sergisiso added the ready for review label Dec 17, 2025

sergisiso changed the title ~~(closes #2884) Change Ref2ArrayRange behaviour~~ (closes #2884) Allow WHERE with imported symbols by changing Ref2ArrayRange behaviour to do the validations Dec 17, 2025

sergisiso mentioned this pull request Dec 17, 2025

Reference2ArrayReference should support structures #1858

Open

arporter added under review and removed ready for review labels Dec 18, 2025

arporter requested changes Dec 18, 2025

View reviewed changes

arporter added reviewed with actions and removed under review labels Dec 18, 2025

sergisiso added 2 commits December 19, 2025 09:34

#2884 Improve ref2arrayrange implementation

526dedb

#2884 Improve typehints and error messages

3e1c6b8

sergisiso temporarily deployed to integration December 19, 2025 11:20 — with GitHub Actions Inactive

sergisiso force-pushed the 2884_change_ref2arrayrange_behaviour branch from 99ab3e0 to 3e1c6b8 Compare December 19, 2025 13:33

sergisiso temporarily deployed to integration December 19, 2025 13:34 — with GitHub Actions Inactive

sergisiso added ready for review and removed reviewed with actions labels Dec 19, 2025

#1858 Improve ref2arrayrange pointer assingment tests

3197baf

sergisiso temporarily deployed to integration December 28, 2025 18:33 — with GitHub Actions Inactive

arporter added under review and removed ready for review labels Jan 5, 2026

arporter requested changes Jan 5, 2026

View reviewed changes

arporter added reviewed with actions and removed under review labels Jan 5, 2026

#2884 Clean up code

c350586

sergisiso temporarily deployed to integration January 5, 2026 17:21 — with GitHub Actions Inactive

sergisiso added ready for review and removed reviewed with actions labels Jan 5, 2026

(closes #2884) Allow WHERE with imported symbols by changing Ref2ArrayRange behaviour to do the validations #3260

Are you sure you want to change the base?

(closes #2884) Allow WHERE with imported symbols by changing Ref2ArrayRange behaviour to do the validations #3260

Conversation

sergisiso commented Dec 15, 2025

Uh oh!

codecov bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sergisiso commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arporter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergisiso Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

sergisiso commented Dec 19, 2025

Uh oh!

arporter 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!

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!

sergisiso commented Jan 5, 2026

Uh oh!

sergisiso commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Dec 15, 2025 •

edited

Loading

sergisiso commented Dec 17, 2025 •

edited

Loading

sergisiso Dec 19, 2025 •

edited

Loading