This is an archive of the discontinued LLVM Phabricator instance.

[flang] Fix checking of argument passing for parameterized derived types
ClosedPublic

Authored by PeteSteinfeld on Apr 21 2021, 12:15 PM.

Details

Summary

We were erroneously not taking into account the constant values of LEN type
parameters of parameterized derived types when checking for argument
compatibility. The required checks are identical to those for assignment
compatibility. Since argument compatibility is checked in .../lib/Evaluate and
assignment compatibility is checked in .../lib/Semantics, I moved the common
code into .../lib/Evaluate/tools.cpp and changed the assignment compatibility
checking code to call it.

After implementing these new checks, tests in resolve53.f90 were failing
because the tests were erroneous. I fixed these tests and added new tests
to call03.f90 to test argument passing of parameterized derived types more
completely.

Diff Detail

Event Timeline

PeteSteinfeld created this revision.Apr 21 2021, 12:15 PM
PeteSteinfeld requested review of this revision.Apr 21 2021, 12:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2021, 12:15 PM
PeteSteinfeld added a project: Restricted Project.Apr 21 2021, 12:15 PM
klausler added inline comments.Apr 21 2021, 12:35 PM
flang/lib/Evaluate/tools.cpp
1181

I'm not sure that this is behaviorally identical with AreKindCompatible(), which only compares KIND type parameters and ignores LEN type parameters.

1185

You don't need to check that the expressions are "constant expressions" in the Fortran standard's sense if you're going to be extracting their actual integer constant values later with ToInt64(). I know that you didn't write this (at least not for this patch), but while you're moving this code we might as well simplify it.

const auto param2{type2.FindParameter(name)};
return !param2 || ToInt64(param1.GetExplicit()) == ToInt64(param2->GetExplicit();

The "evaluate::" namespace name isn't needed now that this code is in that namespace.

PeteSteinfeld added inline comments.Apr 21 2021, 1:15 PM
flang/lib/Evaluate/tools.cpp
1181

It's not identical in that it checks both KIND and LEN type parameters. My reading of the standard is that for objects of a parameterized derived type to be compatible, either in assignment statements or in argument passing, the constant values of the type parameters have to be equal. That's true for both KIND and LEN type parameters. For KIND parameters, the constant value is available at compile time. In this case, the checking for the expressions being compile time constants is unnecessary, but it doesn't do any harm. For LEN parameters, it's possible for the parameter to not have a constant expression available at compile time. In this case, checking must be deferred to run time, and we shouldn't emit error messages at compile time.

There was only one call to AreKindCompatible(), which I replaced, so I deleted that function.

1185

Thanks for the suggestion. The code is much cleaner with these changes.

klausler accepted this revision.Apr 21 2021, 1:27 PM
This revision is now accepted and ready to land.Apr 21 2021, 1:27 PM
PeteSteinfeld added inline comments.Apr 22 2021, 8:38 AM
flang/lib/Evaluate/tools.cpp
1185

Actually, except for the elimination of evaluate:: these changes don't work. We only want to do the comparison if we are able to get expressions for both of the parameters, and we only want to do the comparison when both of the parameters are constants. Also, we need to loop through all of the parameters unless we hit one where they are not equal.