[PATCH] D122079: [ADT] add initializer list specialization for is_contained

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

Could you also add some tests that make sure this can be constant-evaluated? (e.g., with `static_assert`)

Also, would be good to have a couple of tests with non-enum types (e.g., some trivial type like `int` and some class with a copy constructor etc.)



================
Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:976
+
+TEST(STLExtrasTest, IsContainedInitializerList) {
+  {
----------------
Can you add an extra test case with an empty set of things to match against?


================
Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:979
+    bool Val = is_contained(Woofer, {Woofer, SubWoofer});
+    ASSERT_TRUE(Val);
+  }
----------------
These should be EXPECT_TRUE.

ASSERT_TRUE are for when you can't continue executing a test after a failure


================
Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:979
+    bool Val = is_contained(Woofer, {Woofer, SubWoofer});
+    ASSERT_TRUE(Val);
+  }
----------------
kuhar wrote:
> These should be EXPECT_TRUE.
> 
> ASSERT_TRUE are for when you can't continue executing a test after a failure
Also, put the call directly inside the check: `EXPECT_TRUE(is_contained(...))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122079