[PATCH] D144220: New SetOperations and unittesting for all SetOperations

Snehasish Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 16 14:59:29 PST 2023


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

lgtm



================
Comment at: llvm/include/llvm/ADT/SetOperations.h:91
+/// from A (B - A).
+///
+template <class S1Ty, class S2Ty>
----------------
extra empty comment line?


================
Comment at: llvm/unittests/ADT/SetOperationsTest.cpp:88
+  // is empty as they are non-overlapping.
+  ExpectedResult.clear();
+  // Set1 and Set2 should not be touched.
----------------
For cases where the expected result is empty, if you use `EXPECT_THAT(Result, IsEmpty());` you will get an appropriate error message on failure, wont have to call clear on ExpectedResult variables and the expectation is easily spelled out for the reader.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144220



More information about the llvm-commits mailing list