This is an archive of the discontinued LLVM Phabricator instance.

[flang] Duplicate names for ac-implied-do variables erroneously cause errors
ClosedPublic

Authored by PeteSteinfeld on Nov 16 2020, 12:08 PM.

Details

Summary

According to section 19.4, paragraph 5, the scope of an ac-implied-do variable
is the enclosing ac-implied-do. But we were not creating new scopes upon
entry to an ac-implied-do. This was causing error messages to be erroneously
emitted.

I fixed, the code, added a test to array-constr-values.f90, and corrected the
test symbol05.f90.

Diff Detail

Event Timeline

PeteSteinfeld created this revision.Nov 16 2020, 12:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 16 2020, 12:08 PM
PeteSteinfeld requested review of this revision.Nov 16 2020, 12:08 PM
PeteSteinfeld added a project: Restricted Project.Nov 16 2020, 12:13 PM
klausler added inline comments.Nov 16 2020, 12:24 PM
flang/lib/Semantics/resolve-names.cpp
4999

enclosing

ac-implied-do

flang/test/Semantics/array-constr-values.f90
65

You should probably use the duplicated ac-implied-do names in the body and loop control expressions of the nested implied DO loop, and verify that a correct constant array is created.

tskeith accepted this revision.Nov 16 2020, 12:25 PM
tskeith added inline comments.
flang/lib/Semantics/resolve-names.cpp
4999

enclosing

This revision is now accepted and ready to land.Nov 16 2020, 12:25 PM
PeteSteinfeld added inline comments.Nov 16 2020, 1:27 PM
flang/lib/Semantics/resolve-names.cpp
4999

Ay-yi-yi. I'll fix these.

flang/test/Semantics/array-constr-values.f90
65

Good idea. I'll create a constant folding test.

I fixed some spelling errors and added a test to verify that the duplicate
ac-implied-do variable names get evaluated correctly in the presence of
constant folding.

klausler accepted this revision.Nov 16 2020, 1:53 PM
klausler added inline comments.
flang/test/Evaluate/folding15.f90
8

Can you test using the "outer" iDuplicate as part of an implied DO loop bound expression for the "inner" iDuplicate, please?

PeteSteinfeld added inline comments.Nov 16 2020, 2:14 PM
flang/test/Evaluate/folding15.f90
8

I'm not sure what you mean. In the two tests I added, the "iDuplicate" variables are defined in parallel scopes. It's not allowed to have an ac-implied-do variable with the same name as one in an outer scope.

Can you give me an example?

klausler requested changes to this revision.Nov 16 2020, 2:42 PM
klausler added inline comments.
flang/test/Evaluate/folding15.f90
8

Actually, I can't, because I think that C7115 implies that the kind of scoping that you're implementing isn't necessary in the first place.

This revision now requires changes to proceed.Nov 16 2020, 2:42 PM
PeteSteinfeld added inline comments.Nov 16 2020, 3:02 PM
flang/test/Evaluate/folding15.f90
8

Right. My reading of the standard is that C7115 precludes nested scopes of ac-implied-do variables with the same name. But section 19.4, paragraph 5 implies that parallel regions with the same name are allowed. C7115 was already tested in tests/Semantics/array-constr-values.f90, and it still passes with these changes.

klausler accepted this revision.Nov 16 2020, 3:55 PM
This revision is now accepted and ready to land.Nov 16 2020, 3:55 PM
This revision was landed with ongoing or failed builds.Nov 16 2020, 7:14 PM
This revision was automatically updated to reflect the committed changes.