Page MenuHomePhabricator

[flang] Fix bogus message on interface procedure argument names
ClosedPublic

Authored by PeteSteinfeld on Dec 2 2020, 12:41 PM.

Details

Summary

We were keeping the state of parsed equivalence sets in the class
DeclarationVisitor. A problem happened when analyzing the the specification
part of a declaration that contained an EQUIVALENCE statement followed by an
interface block. The same DeclarationVisitor object that was created for the
outer declaration was being used to analyze the specification part
of a procedure body in the interface block. When analyzing the specification
part of the procedure in the interface block, the names in the outer
declaration's EQUIVALENCE statement were erroneously compared with the names in
the arguments of the interface procedure. This resulted in a bogus error
message.

I fixed this by checking to see if we were in an interface block before emitting an error message for EQUIVALENCE statement processing. I also added a test that will produce an erroneous error message without this change.

Diff Detail

Event Timeline

PeteSteinfeld created this revision.Dec 2 2020, 12:41 PM
Herald added a project: Restricted Project. · View Herald Transcript
PeteSteinfeld requested review of this revision.Dec 2 2020, 12:41 PM
PeteSteinfeld added a project: Restricted Project.Dec 2 2020, 12:41 PM
klausler requested changes to this revision.Dec 2 2020, 1:02 PM

I don't think that moving a temporary data structure from declaration processing into a more permanent object like Scope to accomplish scoping is the right way to fix this problem, unfortunately.

The specification-part of an interface block exists only to define the dummy arguments and (if there is one) the function result, so declarations of other local variables are moot in an interface outside of specification inquiries like SIZE(). Since local variables are the only objects that could meaningfully appear in an EQUIVALENCE statement in the specification part of an interface -- dummies can't be EQUIVALENCEd and there's nothing to be learned from an EQUIVALENCE of a function result -- there's no good reason to check such EQUIVALENCE statements for anything. I think that we should warn about their uselessness and otherwise ignore them.

This revision now requires changes to proceed.Dec 2 2020, 1:02 PM

I don't think that moving a temporary data structure from declaration processing into a more permanent object like Scope to accomplish scoping is the right way to fix this problem, unfortunately.

The specification-part of an interface block exists only to define the dummy arguments and (if there is one) the function result, so declarations of other local variables are moot in an interface outside of specification inquiries like SIZE(). Since local variables are the only objects that could meaningfully appear in an EQUIVALENCE statement in the specification part of an interface -- dummies can't be EQUIVALENCEd and there's nothing to be learned from an EQUIVALENCE of a function result -- there's no good reason to check such EQUIVALENCE statements for anything. I think that we should warn about their uselessness and otherwise ignore them.

Thanks, Peter. I thought about doing it that way, but I rejected it since that makes it impossible to detect errors in an EQUIVALENCE statement that appears in an interface block, moot or not. But I'm happy to follow your reasoning. My plan is now to avoid semantic checking on EQUIVALENCE statements when analyzing the specification part of a procedure in an interface block. This means that errors in such EQUIVALENCE statements will go undetected. But, as you say, such errors are irrelevant to the program.

Stop me if you think this is the wrong strategy for fixing this problem.

I changed my implementation to just not do semantic checking for EQUIVALENCE
statements when in an interface block.

klausler accepted this revision.Dec 3 2020, 9:33 AM

A warning message should be considered, too.

This revision is now accepted and ready to land.Dec 3 2020, 9:33 AM
tskeith accepted this revision.Dec 3 2020, 10:24 AM
PeteSteinfeld edited the summary of this revision. (Show Details)Dec 8 2020, 8:15 AM