Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Check for constrains C741 through C749 #1100

wants to merge 3 commits into from

Conversation

psteinfeld
Copy link
Collaborator

@psteinfeld psteinfeld commented Apr 4, 2020

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 Reviewable

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.
Comment on lines 394 to 395
"Coarray components must be ALLOCATABLE and have a deferred "
"coshape"_err_en_US);
Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

Comment on lines -8 to -9
! C736: If EXTENDS appears and the type being defined has a coarray ultimate
! component, its parent type shall have a coarray ultimate component.
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Comment on lines 14 to 15
!ERROR: 'pointerfield' may not have the POINTER attribute because it is a coarray
!ERROR: Coarray components must be ALLOCATABLE and have a deferred coshape
Copy link
Collaborator

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

Copy link
Collaborator Author

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.

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);
Copy link
Collaborator

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

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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?

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

@klausler klausler left a 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.

Comment on lines 394 to 395
"Coarray components must be ALLOCATABLE and have a deferred "
"coshape"_err_en_US);
Copy link
Collaborator

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.

} else {
if (!details.coshape().IsAssumedSize()) { // C828
if (!details.coshape().IsAssumedSize()) { // C746, C828
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

"ISO_FORTRAN_ENV"_err_en_US);
} else {
if (IsIsoCType(derived)) {
Say("A coarray component may not be of C_PTR or C_FUNPTR from "
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

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);
Copy link
Collaborator

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.
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())};
Copy link
Collaborator

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants