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

Improve array element errors #1039

Merged
merged 1 commit into from Mar 3, 2020
Merged

Improve array element errors #1039

merged 1 commit into from Mar 3, 2020

Conversation

tskeith
Copy link
Collaborator

@tskeith tskeith commented Feb 28, 2020

When something is parsed as an array element it was sometimes intended
to be a function call or structure constructor. So if the base name is
not found the errors can be confusing. This is an attempt to improve
them.

When the subscript list is empty, it was probably meant to be a function
call, so report that the name is not a function.

If the base is a scalar but there are subscripts, report that it is not
an array.

Copy link
Collaborator

@psteinfeld psteinfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good enough!

} else {
Say("Subscripts may be applied only to an object, component, or array constant"_err_en_US);
}
}
// error was reported: analyze subscripts without reporting more errors
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 understand this comment. Do you mean that an error may been reported?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only hit this line if an error was reported about the base of the array element. In that case we want to analyze the subscript expressions (because every expression should be analyzed) but not report any errors from that analysis because the might be confusing. E.g. in data01.f90, after "'persn' is not an array" you would get an error about 'Abcd Efgh' not being an integer (because array subscripts are required to be integers).

Say("Reference to rank-%d object '%s' has %d subscripts"_err_en_US,
symbolRank, symbol.name(), subscripts);
if (symbolRank != 0) {
Say("Reference to rank-%d object '%s' has %d subscripts"_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.

It would be good to have a test case that produces this message, although it looks like there's never been a test case for this condition.

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 idea. There was "Reference to rank-0 object..." error that I removed, so I've added a couple of tests.

Copy link
Collaborator

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, one remark regarding assumed-rank array, but that may be out-of this PR scope.

symbolRank, symbol.name(), subscripts);
if (symbolRank != 0) {
Say("Reference to rank-%d object '%s' has %d subscripts"_err_en_US,
symbolRank, symbol.name(), subscripts);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit lost with the semantic check status for assumed-rank array, I have them in mind because I just reviewed #1010 . The code above would still emit something weird for them:

function foo(b)
  integer :: foo
  integer :: b(..)
  foo = b(2, 2)
end function

error: Reference to rank-1 object 'b' has 2 subscripts is emitted because symbol.Rank() for assumed-rank returns 1.

If we are not yet enforcing C838 then that may just be a TODO and please ignore my comment.

When something is parsed as an array element it was sometimes intended
to be a function call or structure constructor. So if the base name is
not found the errors can be confusing. This is an attempt to improve
them.

When the subscript list is empty, it was probably meant to be a function
call, so report that the name is not a function.

If the base is a scalar but there are subscripts, report that it is not
an array.
@tskeith tskeith merged commit 899ef78 into master Mar 3, 2020
@tskeith tskeith deleted the tsk-arrays branch March 3, 2020 00:53
swift-ci pushed a commit to apple/llvm-project that referenced this pull request Apr 9, 2020
When something is parsed as an array element it was sometimes intended
to be a function call or structure constructor. So if the base name is
not found the errors can be confusing. This is an attempt to improve
them.

When the subscript list is empty, it was probably meant to be a function
call, so report that the name is not a function.

If the base is a scalar but there are subscripts, report that it is not
an array.

Original-commit: flang-compiler/f18@e2fd533
Reviewed-on: flang-compiler/f18#1039
swift-ci pushed a commit to apple/llvm-project that referenced this pull request Apr 9, 2020
…r/tsk-arrays

Improve array element errors

Original-commit: flang-compiler/f18@899ef78
Reviewed-on: flang-compiler/f18#1039
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
When something is parsed as an array element it was sometimes intended
to be a function call or structure constructor. So if the base name is
not found the errors can be confusing. This is an attempt to improve
them.

When the subscript list is empty, it was probably meant to be a function
call, so report that the name is not a function.

If the base is a scalar but there are subscripts, report that it is not
an array.

Original-commit: flang-compiler/f18@e2fd533
Reviewed-on: flang-compiler/f18#1039
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.

None yet

3 participants