[flang-commits] [PATCH] D123331: Expand coverage of this_image semantics testing

[Katherine Rasmussen via Phabricator via flang-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:
> > 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?
> Probably the first option, if the bug can't be fixed.  But fixing the bug would be best.
@klausler Thanks for your feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123331