[PATCH] D91653: [LLVMFrontend][openacc] Add basic unit tests for functions in LLVMFrontendOpenACC

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 17 14:49:28 PST 2020


Meinersbur added inline comments.


================
Comment at: llvm/unittests/Frontend/OpenACCTest.cpp:19
+
+static const SmallVector<Clause, 45> allClauses = {ACCC_unknown,
+                                                   ACCC_async,
----------------
Since this array is never modified, use a plain old array for this, which doesn't need a static constructor:
```
static const Clause allClauses[] = ...
```


================
Comment at: llvm/unittests/Frontend/OpenACCTest.cpp:66-71
+class OpenACCTest : public testing::Test {
+protected:
+  void SetUp() override {}
+
+  void TearDown() override {}
+};
----------------
This fixup doesn't do anything, you can just used the non-fixup versions ('TEST(..)' instead of `TEST_F`)


================
Comment at: llvm/unittests/Frontend/OpenACCTest.cpp:219
+expectAllowedClauses(Directive dir, unsigned version,
+                     const std::initializer_list<Clause> &allowedClauses) {
+  SmallSet<Clause, 30> allowedClausesSet;
----------------
`std::initializer_list` is unusual for non-constructors. Use `ArrayRef`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91653



More information about the llvm-commits mailing list