-
Notifications
You must be signed in to change notification settings - Fork 48
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
Check for constrains C741 through C749 #1100
base: master
Are you sure you want to change the base?
Conversation
Most of these checks were already implemented and I just added references to them to the code and tests. I implemented the check for C747 to not allow coarray components in derived types that are of type C_PTR, C_FUNPTR, or type TEAM_TYPE. I implemented the check for C748 that requires a data component whose type has a coarray ultimate component to be a nonpointer, nonallocatable scalar and not be a coarray. I also fixed some unrelated tests that got new errors when I implemented these checks.
lib/Semantics/check-declarations.cpp
Outdated
"Coarray components must be ALLOCATABLE and have a deferred " | ||
"coshape"_err_en_US); |
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 don't think the message should say "and have a deferred coshape" unless it doesn't have a deferred coshape.
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.
Or just combine the tests and use the second message, it's got all the information.
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.
Thanks, Tim. I'll change the code to remove that phrase when it doesn't apply.
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.
@klausler, I think it's a little confusing to have the message tell the user that the declaration must have a deferred coshape when it already has one. I plan to stick with Tim's suggestion.
! C736: If EXTENDS appears and the type being defined has a coarray ultimate | ||
! component, its parent type shall have a coarray ultimate component. |
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.
Where is the test for C736 now?
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.
C736 is specific to the definition of a component of a derived type, and all of the tests in allocate11.f90 were allocate statements. I added the test resolve86.f90 in my previous pull request to cover the case of defining a component that contains a coarray ultimate component without having a parent type that contains a coarray ultimate component. This may have happened after you reviewed and approved that pull request.
test/Semantics/resolve88.f90
Outdated
!ERROR: 'pointerfield' may not have the POINTER attribute because it is a coarray | ||
!ERROR: Coarray components must be ALLOCATABLE and have a deferred coshape |
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.
It would be nice if the wording of these two messages were similar. For example, the second could be:
Component 'pointerfield' must have the ALLOCATABLE attribute because it is a coarray
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.
Sounds good. I'll change it.
lib/Semantics/resolve-names.cpp
Outdated
if (FindCoarrayUltimateComponent(*derived)) { // C748 | ||
if (attrs.HasAny({Attr::POINTER, Attr::ALLOCATABLE})) { | ||
Say("A component whose type has a coarray ultimate component " | ||
"cannot be a POINTER or ALLOCATABLE"_err_en_US); |
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.
We have mostly been using "may not" rather than "cannot".
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 also use "must not", but I shouldn't -- "may" is the correct modal verb here, since we're talking about what's allowed, not what might be possible.
You might as well capture the result of FindCoarrayUltimateComponent
and use its name in the message and link to its declaration.
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.
Thanks. I'll fix it.
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 notice that there are many uses of the word "cannot" in resolve-names.cpp. @tskeith, should I fix them all in this pull request?
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 it makes more sense to change them in a separate pull request, but if you want to do it in this one that's okay too.
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 I'll wait.
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.
All of these constraints on coarrays really could have been expressed more simply in the standard, and maybe it would be easier on the user in some cases if we explicitly stated in messages what the standard only implies -- a coarray can't be part of any array, and can't be a local variable other than a dummy argument or (I think) something that's SAVE. In implementation terms, every coarray that isn't a dummy argument has -- or can have -- a fixed static address set at link time. But I can't think of a good way to use that simple statement in a way that would improve the messages; maybe you can.
lib/Semantics/check-declarations.cpp
Outdated
"Coarray components must be ALLOCATABLE and have a deferred " | ||
"coshape"_err_en_US); |
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.
Or just combine the tests and use the second message, it's got all the information.
lib/Semantics/check-declarations.cpp
Outdated
} else { | ||
if (!details.coshape().IsAssumedSize()) { // C828 | ||
if (!details.coshape().IsAssumedSize()) { // C746, C828 |
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.
C746 applies to components, but components are all handled earlier.
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.
Good catch! This also applies to the comment above. I'll remove the spurious references.
lib/Semantics/resolve-names.cpp
Outdated
"ISO_FORTRAN_ENV"_err_en_US); | ||
} else { | ||
if (IsIsoCType(derived)) { | ||
Say("A coarray component may not be of C_PTR or C_FUNPTR from " |
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.
Missing the word "type", or needs rewording.
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.
Thanks for catching this. I'll fix it.
lib/Semantics/resolve-names.cpp
Outdated
if (FindCoarrayUltimateComponent(*derived)) { // C748 | ||
if (attrs.HasAny({Attr::POINTER, Attr::ALLOCATABLE})) { | ||
Say("A component whose type has a coarray ultimate component " | ||
"cannot be a POINTER or ALLOCATABLE"_err_en_US); |
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 also use "must not", but I shouldn't -- "may" is the correct modal verb here, since we're talking about what's allowed, not what might be possible.
You might as well capture the result of FindCoarrayUltimateComponent
and use its name in the message and link to its declaration.
I redid the error messages for violations of C746 to be more consistent and customized them so that we don't complain unnecessarily about deferred coshapes. I removed some spurious references to C746. I fixed the error message for coarray components of type C_PTR or C_FUNPTR. I added an attachment to the messages relating to ultimate components to point to the relevant declarations. I fixed the tests to match the message changes in the code and added another test for components that are allocatable coarrays that do not have a deferred coshape.
lib/Semantics/resolve-names.cpp
Outdated
if (auto it{FindCoarrayUltimateComponent(*derived)}) { // C748 | ||
Message declMsg{Say(it->name(), | ||
"Type '%s' has coarray ultimate component '%s' declared here"_en_US, | ||
declType->AsFortran(), it.BuildResultDesignatorName())}; |
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.
Does this work correctly if the coarray component is declared in a module that is read in from a .mod file?
coarray ultimate components to use the function AttachDeclaration() to output the declaration of the derived type that contains the associated coarray ultimate component.
Most of these checks were already implemented and I just added references to
them to the code and tests.
I implemented the check for C747 to not allow coarray components in derived
types that are of type C_PTR, C_FUNPTR, or type TEAM_TYPE.
I implemented the check for C748 that requires a data component whose type has
a coarray ultimate component to be a nonpointer, nonallocatable scalar and not
be a coarray.
I also fixed some unrelated tests that got new errors when I implemented these
new checks.
This change is