[PATCH] D47940: [SmallSet] Add some simple unit tests.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 8 11:14:59 PDT 2018


dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Thanks for adding some test coverage! Few things that might be able to be simplified before committing.



================
Comment at: unittests/ADT/SmallSetTest.cpp:34-35
+
+  EXPECT_EQ(0u, s1.count(99));
+  EXPECT_EQ(0u, s1.count(4));
+}
----------------
not sure these two cases are worth testing separately - even though 4 is technically a boundary value, the set was never passed the value 4 so it seems just as likely to be buggily present as 99 does? (similar feedback on similar testing in the other cases, I think)


https://reviews.llvm.org/D47940





More information about the llvm-commits mailing list