Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(Closes #2642, initial step towards #2643) Implement
extends(type)
,procedure
in derived type,class
in declarations. #2644base: master
Are you sure you want to change the base?
(Closes #2642, initial step towards #2643) Implement
extends(type)
,procedure
in derived type,class
in declarations. #2644Changes from 21 commits
a527543
b95abdc
349bd2b
f3bb061
db3d952
6d0bb74
a449c31
16ed2e4
e182c18
ff993ea
8c789b7
4247a00
1cd37e4
0719559
6698427
cc6fb67
c8f74cb
c35d98b
0923123
18679fa
706bffc
b72336b
809ebf2
6f18710
9419a05
421e546
a862a21
cbc5857
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we should be ending up with a Symbol named "*" as there is no such symbol - it's just Fortran syntax? I guess this will become clearer as I do the rest of the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, but that's how the frontend deals with
whatever
in eitherclass(whatever)
ortype(whatever)
. I'm not so sure how to deal with this case unless I modify the frontend even more extensively for this (awful) Fortran syntax? It's used in tests but I certainly hope no one actually usesclass(*)
polymorphism... at least it's not in any.f90
test file.I've moved dealing with this '*' symbol in the backend (by removing it so that it can't be declared anywhere) to line 1003, as this feels a bit cleaner.
Let me know what you think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this means we have to modify the frontend a bit more - conceptually a Symbol refers to a memory location. Here, the "*" is syntax meaning (I've just googled) "unlimited polymorphic type" apparently. I've a feeling this is used in LFRic and I'm sure other codes will as well. So, when we encounter a declaration like:
we know we have a Symbol named "some_arg" but we don't know the type. The simplest thing to do would be to just raise a NotImplementedError in this case so that we will get a Symbol of UnsupportedFortranType. If that's not sufficient for your particular use case, we maybe need a new type. We already have
UnresolvedType
- we could perhaps add anis_class
property to that? What do @sergisiso and @hiker think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like using
UnsupportedFortranType
would be going slightly backward here, as we'd be back to string declarations for these symbols (and I'd have to edit a lot of tests again due to upper/lowercase fparser/PSyclone outputs, probably).I'm open to whatever other option, if having this '*' symbol is too problematic.