[PATCH] D113086: [flang] Add a semantics test for co_reduce

Emil Kieri via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 21 12:44:57 PST 2021


ekieri added a comment.

It would be nice with a comment citing where in the standard the interface of this intrinsic is specified.

Are you (or do you know of someone who is) planning to write the actual semantics checks as well in the near future? I am a little concerned that otherwise the developer who does that one day will be unaware of these tests, and implements additional tests of their own, leaving your tests with XFAIL.

Also, your test contains trailing whitespace. Is this intentional? The coding standards [1] discourages that, and while they primarily concern the c++ code, the argument provided is no less valid for test cases in fortran. On the other hand, @klausler made the (also valid) argument in D113077 <https://reviews.llvm.org/D113077> that variations in style improves test coverage of the parser. I am fine with you doing it either way, but encourage you to make a conscious decision about it.

[1] https://llvm.org/docs/CodingStandards.html#whitespace


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

https://reviews.llvm.org/D113086



More information about the llvm-commits mailing list