[flang-commits] [flang] [flang] Fixed regression in copy-in/copy-out (PR #161259)

[Peter Klausler via flang-commits]

================
@@ -1493,32 +1493,21 @@ class CopyInOutExplicitInterface {
     return !actualTreatAsContiguous && dummyNeedsContiguity;
   }
 
-  // Returns true, if actual and dummy have polymorphic differences
   bool HavePolymorphicDifferences() const {
-    bool dummyIsAssumedRank{dummyObj_.type.attrs().test(
-        characteristics::TypeAndShape::Attr::AssumedRank)};
-    bool actualIsAssumedRank{semantics::IsAssumedRank(actual_)};
-    bool dummyIsAssumedShape{dummyObj_.type.attrs().test(
-        characteristics::TypeAndShape::Attr::AssumedShape)};
-    bool actualIsAssumedShape{semantics::IsAssumedShape(actual_)};
-    if ((actualIsAssumedRank && dummyIsAssumedRank) ||
-        (actualIsAssumedShape && dummyIsAssumedShape)) {
-      // Assumed-rank and assumed-shape arrays are represented by descriptors,
-      // so don't need to do polymorphic check.
-    } else if (!dummyObj_.ignoreTKR.test(common::IgnoreTKR::Type)) {
-      // flang supports limited cases of passing polymorphic to non-polimorphic.
-      // These cases require temporary of non-polymorphic type. (For example,
-      // the actual argument could be polymorphic array of child type,
-      // while the dummy argument could be non-polymorphic array of parent
-      // type.)
-      bool dummyIsPolymorphic{dummyObj_.type.type().IsPolymorphic()};
-      auto actualType{
-          characteristics::TypeAndShape::Characterize(actual_, fc_)};
-      bool actualIsPolymorphic{
-          actualType && actualType->type().IsPolymorphic()};
-      if (actualIsPolymorphic && !dummyIsPolymorphic) {
-        return true;
-      }
+    // These cases require temporary of non-polymorphic type. (For example,
+    // the actual argument could be polymorphic array of child type,
+    // while the dummy argument could be non-polymorphic array of parent
+    // type.)
+    if (dummyObj_.ignoreTKR.test(common::IgnoreTKR::Type)) {
+      return false;
+    }
+    auto actualType{characteristics::TypeAndShape::Characterize(actual_, fc_)};
+    if (actualType && actualType->type().IsPolymorphic() &&
+        !actualType->type().IsAssumedType() &&
+        !dummyObj_.IsPassedByDescriptor(/*isBindC*/ false)) {
----------------
klausler wrote:

+1 to the use of `std::optional<bool>` here; it's what `IsContiguous()` returns for similar reasons.

I would like us to try to have a single implementation here for both semantics and lowering, if one can be found; it would be hard to have two implementations and be assured that they were identical.

It should be possible to write more tests that can verify that code that is warning-free at compilation time doesn't use copy-in at run-time (use `LOC`), and that code that *does* use copy-in at run-time got warnings at compilation time.

https://github.com/llvm/llvm-project/pull/161259