PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

(closes #1495) Add support for type-bound subroutine calls

Open sergisiso opened this issue 1 year ago • 3 comments

In addition to the frontend-node-backend I also ported the LFRic haloex lowering to check that it is equivalent to the gen_code, but I have not continued porting other LFRic infrastructure calls since the PR is already quite large.

sergisiso avatar Mar 22 '24 17:03 sergisiso

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.86%. Comparing base (0a7220e) to head (2712362).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2527   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files         357      357           
  Lines       48170    48259   +89     
=======================================
+ Hits        48104    48193   +89     
  Misses         66       66           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Mar 22 '24 18:03 codecov-commenter

@arporter This is also ready to review. The PR touches many files, but a lot of them are repetitive. I made sure to pass the integration tests because this changes the PSyIR in all the apis/applications.

sergisiso avatar Mar 27 '24 19:03 sergisiso

I fact @hiker can also review this one as the type-bound methods issue is something he found for the extraction and I believe this PR solve it. (because I had to remove the spaces around % of some tests)

Also its not done in this PR because its large enough but I added TODOs explaining that the reference_access needs to be updated when finding my_struct%my_method() by returning my_struct with a new AccessType which is different that READ. Maybe TYPE-SYMBOL-USED? Something similar to the LBOUND/UBOUND/SIZE argument. I believe we talk about this before but it would be good to see if its also what @hiker is thinking.

sergisiso avatar Mar 28 '24 09:03 sergisiso

@arporter This is ready for another review. I also triggered the integration test.

sergisiso avatar Apr 10 '24 08:04 sergisiso

@arporter I added couple more commits because the first integration test failed, now it's all green and ready for review.

sergisiso avatar Apr 10 '24 12:04 sergisiso

@arporter This is ready for next review.

sergisiso avatar Apr 23 '24 09:04 sergisiso

(Link-check failures appear to be because the NVIDIA website is a bit sluggish at the moment.)

arporter avatar Apr 24 '24 20:04 arporter

Integration tests were all green.

arporter avatar Apr 24 '24 21:04 arporter