[PATCH] D123331: Expand coverage of this_image semantics testing

[Katherine Rasmussen via Phabricator via llvm-commits]

ktras added inline comments.


================
Comment at: flang/test/Semantics/this_image02.f90:30
+  n = this_image(team_coarray, team=home)
+  n = this_image(team_coarray[1], team=home)
+  n = this_image(coarray=co_array, team=home)
----------------
klausler wrote:
> ktras wrote:
> > The implementation of `this_image` does not currently support this call, so please add the following error message before it.
> > `!ERROR: missing mandatory 'dim=' argument`
> > 
> Don't change tests to match buggy behavior.
I believe that @rouson is working on contributing tests for parallel features but is not currently looking at editing source. It seems to me in this situation there are three options:

  # Do not add an error for this line of code and add an XFAIL directive
  # Do not add an error and do not add XFAIL directive and push a failing test
  # Add the error and when the support is added, the error is removed in the same patch

Based on looking at other tests in the past that had error directives that would go away once certain features were supported, I thought this case was similar and thought the last option was best.

@klausler Since your comment seems to say this is not the best approach, which is the best option for this test and what matches the workflow of the developers?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123331/new/

https://reviews.llvm.org/D123331