(towards #3124) Codeblocks with children references by sergisiso · Pull Request #3247 · stfc/PSyclone

sergisiso · 2025-12-02T17:32:32Z

No description provided.

… children references

…tual_references

codecov · 2025-12-02T17:44:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (dfb7589) to head (568bf95).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3247   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files         386      386           
  Lines       54181    54206   +25     
=======================================
+ Hits        54159    54184   +25     
  Misses         22       22

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

…he Codeblock

sergisiso · 2025-12-03T17:15:50Z

@arporter @LonelyCat124 @hiker This is ready for review, it is another step into making the all references expressed in the PSyIR so that tools that expect/operate on References don't need edge cases.

arporter

Nice, thanks Sergi.
The only concern I have is that, with this change, we now won't warn if a Symbol incorrectly gets added to the table of a Container. Can we tighten up on that?
I see you ran the integration tests so I won't do that again this time around.

src/psyclone/psyir/nodes/codeblock.py

arporter · 2025-12-11T08:08:30Z

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

-    # s symbol should not be in my_mod
-    with pytest.raises(KeyError):
-        psyir.children[0].symbol_table.lookup("s")
+    # FIXME: explain


Yes please :-)

Done. Its an important consideration, feel free to discuss.

arporter · 2025-12-11T08:09:45Z

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

    '''
    fake_parent, fparser2spec = process_where(
-        "WHERE (ptsu(myfunc(), :, :) /= 0._wp)\n"
+        "WHERE (ptsu(unres, :, :) /= 0._wp)\n"


What was the reason for this change?

src/psyclone/tests/psyir/nodes/codeblock_test.py

arporter · 2025-12-11T08:14:34Z

src/psyclone/tests/psyir/nodes/omp_directives_test.py

    ''' Tests the infer_sharing_attributes() method when some of the loops have
-    Codeblocks inside it. We check that the infer_sharing attribute analysis
-    succeed by assuming worst case.
+    potentially hidden references (e.g. inside codeblocks or loop varialbes).


"variables"

arporter · 2025-12-11T08:16:53Z

src/psyclone/tests/psyir/nodes/omp_directives_test.py

+    loop = psyir.walk(Loop)[loop_idx]
    # Make sure that the write statements inside the loop are CodeBlocks,
    # otherwise we need a new test example
-    assert loop.has_descendant(nodes.CodeBlock)


We still need something like this check - probably has to be a walk now.

Re-introduced

…tual_references

sergisiso · 2026-03-06T15:29:04Z

@arporter It took me a long time to get back to this, but it is now ready for another look.

arporter

Very nearly there. There's just one line in the fortran backend that isn't covered by the associated tests. I'll fire off the ITs in the meantime.

arporter · 2026-03-09T11:03:47Z

src/psyclone/psyir/backend/fortran.py

+            if type(symbol) is Symbol and symbol.is_unresolved:
+                # However we also accept symbols that we don't know where
+                # they are declared, so we propagated upwards.
+                continue


Unfortunately this line isn't covered by the fortran_*test.py tests.

Since I'm here, I'm also not sure what "we propagated upwards" means as we're already handling a FileContainer at this point. Do you mean they will have been propagated upwards and have ended up in this table?

sergisiso · 2026-03-09T17:33:34Z

Thanks for reviewing @arporter . This PR doesn't bring any benefit by itself, it will only be a benefit when all data symbol usage is captured by a Reference (Loop iterators and LFRic Kernels arguments are still missing). Since a release is imminent, I am wondering if we should not not mess with the CodeBlock structure now and merge this after the release. What do you think?

arporter · 2026-03-10T11:16:56Z

Thanks for reviewing @arporter . This PR doesn't bring any benefit by itself, it will only be a benefit when all data symbol usage is captured by a Reference (Loop iterators and LFRic Kernels arguments are still missing). Since a release is imminent, I am wondering if we should not not mess with the CodeBlock structure now and merge this after the release. What do you think?

Yes, that sounds sensible to me. I'll mark it as 'blocked' pending cutting of a release-candidate.

arporter · 2026-03-10T14:45:55Z

Just stumbled across #831 which I think might already be done and will definitely be done once this is complete?

sergisiso added 9 commits November 25, 2025 12:14

#3124 Improve codeblock get_symbol_names and add the names as virtual…

418f6aa

… children references

#3124 Update code after creating CodeBlocks virtual children references

9130726

Merge remote-tracking branch 'origin/master' into 3124_codeblocks_vir…

7de4e91

…tual_references

#3124 make DefUseChain consider new CodeBlock structure

f5cd2cf

Merge remote-tracking branch 'origin/master' into 3124_codeblocks_vir…

547dad0

…tual_references

#3124 Fix or remove tests

c25c853

#3124 Comment out test with issues

7666bd0

#3124 Fix flake8

ce99fcf

Merge remote-tracking branch 'origin/master' into 3124_codeblocks_vir…

1a6d0c8

…tual_references

sergisiso added 5 commits December 3, 2025 14:39

#3124 Remove unneeded _get_symbol to find invoke codeblock symbol

b00eb12

#3124 Remove unused code

402466e

#3124 Improve test coverage

9aa682c

#3124 Improve test coverage

e18ad2f

#3124 Add missing docstring

4f25b62

sergisiso requested review from LonelyCat124, arporter and hiker December 3, 2025 16:58

sergisiso self-assigned this Dec 3, 2025

sergisiso added the ready for review label Dec 3, 2025

#3124 Revert deletion of existing test and instead adapt it without t…

5faa955

…he Codeblock

sergisiso temporarily deployed to integration December 3, 2025 17:16 — with GitHub Actions Inactive

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

Merge branch 'master' into 3124_codeblocks_virtual_references

990752a

arporter requested changes Dec 11, 2025

View reviewed changes

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

#3124 Improve CodeBlock documentation and errors

9e8ebf4

sergisiso added 6 commits February 25, 2026 11:22

#3124 Bring to master

5d4036b

#3124 Bring to master

c561196

#3124 Temporary bypass a test

f7bfb92

#3124 Improve tests

3de3393

#3124 Improve tests and TODOs

dfb4e38

Merge remote-tracking branch 'origin/master' into 3124_codeblocks_vir…

568bf95

…tual_references

sergisiso added ready for review and removed reviewed with actions labels Mar 6, 2026

arporter added under review and removed ready for review labels Mar 9, 2026

arporter requested changes Mar 9, 2026

View reviewed changes

arporter temporarily deployed to integration March 9, 2026 11:08 — with GitHub Actions Inactive

arporter added Blocked An issue/PR that is blocked by one or more issues/PRs. and removed under review labels Mar 10, 2026

Conversation

sergisiso commented Dec 2, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

sergisiso commented Dec 3, 2025

Uh oh!

arporter left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

sergisiso commented Mar 6, 2026

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!

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

Uh oh!

Uh oh!

arporter commented Mar 10, 2026

Uh oh!

arporter commented Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Dec 2, 2025 •

edited

Loading

sergisiso commented Mar 9, 2026 •

edited

Loading