[PATCH] D23178: [ADT] Add relation operators for Optional

[David Blaikie via llvm-commits]

dblaikie added inline comments.

================
Comment at: include/llvm/ADT/Optional.h:155
@@ +154,3 @@
+bool operator==(const Optional<T> &X, const Optional<U> &Y) {
+  if (X.hasValue() && Y.hasValue())
+    return *X == *Y;
----------------
I'd probably just use if (X && Y) ? (similarly elsewhere)

================
Comment at: unittests/ADT/OptionalTest.cpp:380
@@ -379,12 +379,3 @@
 
-TEST_F(OptionalTest, NoneComparison) {
-  Optional<int> o;
-  EXPECT_EQ(o, None);
-  EXPECT_EQ(None, o);
-  EXPECT_FALSE(o != None);
-  EXPECT_FALSE(None != o);
-  o = 3;
-  EXPECT_FALSE(o == None);
-  EXPECT_FALSE(None == o);
-  EXPECT_TRUE(o != None);
-  EXPECT_TRUE(None != o);
+TEST_F(OptionalTest, Comparisonn) {
+  Optional<int> Opts[] = {None, 3, 4};
----------------
This seems to have lost the tests for comparison with NoneType (as distinct from the tests with Optional<T>s that are in the non-value state)

But in general - while this is obviously more compact than writing out all the tests explicitly, I wonder if there's a better pivot or other way we could write these tests to be more legible and/or more compact. Did you have any other ideas/things you attempted for testing? as a very rough sketch I'd probably start by wanting to have a test case/function per operator, then figure out how to factor the common functionality of those tests into utilities/helpers. Maybe templates.. 




https://reviews.llvm.org/D23178