(Closes #2642, initial step towards #2643) Implement `extends(type)`, `procedure` in derived type, `class` in declarations.
This adds the following object- and inheritance-oriented features:
- an
extendsattribute inStructureTypefor inheritance of derived types, which avoids relying onUnsupportedFortranTypeand itsdeclaration: strattribute, - a
procedure_componentsattribute inStructureTypefor procedures in acontainsblock of a derived type, including visibility and initial value, - support for
class(versustype) in routine declarations, by introducing a boolean attribute inDataTypeSymbol.
Note that as this makes some derived types supported there is for now a workaround using FortranWriter in both GOceanKernelMetadata and LFRicKernelMetadata, which rely on UnsupportedFortranType().declaration and fparser, see #2643.
This will require me to edit the documentation, including (but maybe not limited to) https://psyclone-dev.readthedocs.io/en/latest/psyir_symbols.html#datatypes
@arporter @sergisiso This works fine in practice on our project. I’ll need to add some more specific tests, see what’s up with the master branch conflicts (Aidan’s routines things?) and refactor this thing for sure so it’s going to stay a draft until next week at least. But if you have time for a broad overview at some point I’m definitely interested in it :) Don’t bother with the details, but if something doesn’t agree with what you’d want in there, feel very free to point it out and I’ll rework it. Most especially: I’m not so sure the way I’m getting the fparser attributes in the frontend is very clean at times.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 99.88%. Comparing base (8736ae3) to head (cbc5857).
:warning: Report is 3167 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #2644 +/- ##
==========================================
- Coverage 99.88% 99.88% -0.01%
==========================================
Files 357 357
Lines 49823 50214 +391
==========================================
+ Hits 49767 50156 +389
- Misses 56 58 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@sergisiso @arporter Two remaining lines still to be tested for codecov (I’ll do this on Monday) but this is otherwise ready for review. It might be nice to fire the integration tests to be sure it breaks nothing in lfric due to the metadata extraction workaround. I’m arguably going a bit blind on that front. Also interested on your opinion on my having to xfail one test in due to added support: src/psyclone/tests/psyad/domain/lfric/test_lfric_adjoint_harness.py There’s probably some better way to deal with the PSyclone error on these two routines not being imported in the TL source code.
Thanks @JulienRemy. I've set the integration tests going...
Unfortunately both the LFRic integration tests and the compilation tests failed. You should be able to reproduce the latter by running pytest with the --compile flag. The error is:
One of the tests that failed is:
tests/psyir/frontend/fparser2_select_type_test.py
The LFRic integration tests failed when linking the LFRic executable:
Thanks @arporter
I had missed a few things related to psyGen (kernels get renamed, so procedure pointers must as well). All --compile tests now pass locally.
Hopefully the integration tests will as well... or they'll reveal new bugs :)
Thanks for your review @arporter :) I've pushed my edits and detailed some answers above as well.
@arporter All edits except for those about class(*) have been done, pending a decision about these.
@hiker and @sergisiso - we could do with your opinion on how to handle declarations of type class(*) - these are "unlimited polymorphic type". My thought is to have these as UnresolvedType and extend that type with an is_class property. That's perhaps going to be non intuitive though as it will mean we then need to generate a declaration for such a Symbol but only if is_class is True (and also, the meaning of UnresolvedType is currently that we can't resolve it but could in principle). We may therefore be better off adding an entirely new type, possibly DeferredType? What do you think?