[flang-commits] [flang] [flang][MIF] Adding common features related to coarray (PR #215576)

[Dan Bonachea via flang-commits]

================
@@ -402,11 +402,117 @@ class HlfirDesignatorBuilder {
 
   fir::FortranVariableOpInterface
   gen(const Fortran::evaluate::CoarrayRef &coarrayRef) {
-    TODO(getLoc(), "coarray: lowering a reference to a coarray object");
+    PartInfo partInfo;
+    mlir::Type resultType = visit(coarrayRef, partInfo);
+    return genDesignate(resultType, partInfo, coarrayRef);
----------------
bonachea wrote:

Before this PR, an attempt to compile a coindexed access to a coarray of intrinsic type, eg:
```fortran
integer :: x[*]
...
i = x[peer]
```

in `-fcoarray` mode would result in this helpful error:
```
llvm/flang/lib/Lower/ConvertExprToHLFIR.cpp:405: not yet implemented: coarray: lowering a reference to a coarray object
```
However with this change, the code above no longer generates a compile error, it instead compiles to _silently incorrect_ object code that fails to invoke `prif_get` as required and instead just produces incorrect runtime results.

I understand this PR is a precursor to PRs #212777 and #210283 that will actually complete the lowering of coindexed accesses, and I agree with that PR strategy. However I'm somewhat concerned about separately merging this PR that "removes the guardrails" before the followon PRs complete the implementation. We don't want to give users of intermediate commits the mistaken impression that coindexed access is working before it's actually ready.

I suggest we restore the `TODO` for now with a comment, and remove the `TODO` in the follow-on PR that completes the implementation. Would that be acceptable?


```suggestion
    // lowering of coindexed access remains a work-in-progress, see #212777
    TODO(getLoc(), "coarray: lowering a reference to a coarray object");
    
    PartInfo partInfo;
    mlir::Type resultType = visit(coarrayRef, partInfo);
    return genDesignate(resultType, partInfo, coarrayRef);
```

This unfortunately will likely break the (trivial) test added in this PR, but that's by design because we cannot yet correctly lower that code. 

I think it's better to merge untestable (dead) code than to merge changes causing silent miscompilation of coindexed access.



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