Implement operator== and operator!= for Optional<>
Created attachment 275726 [details] Patch
Attachment 275726 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:115: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:116: Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:117: Consider using EXPECT_NE instead of EXPECT_FALSE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:118: Consider using EXPECT_NE instead of EXPECT_FALSE(a == b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:140: Consider using EXPECT_EQ instead of EXPECT_FALSE(a != b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:141: Consider using EXPECT_NE instead of EXPECT_TRUE(a != b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:142: Consider using EXPECT_EQ instead of EXPECT_FALSE(a != b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:143: Consider using EXPECT_NE instead of EXPECT_TRUE(a != b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:145: Consider using EXPECT_NE instead of EXPECT_TRUE(a != b) [readability/check] [2] ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:146: Consider using EXPECT_NE instead of EXPECT_TRUE(a != b) [readability/check] [2] Total errors found: 10 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 275726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275726&action=review I think I should wait for Anders to give the r+, but LGTM. > Source/WTF/wtf/Optional.h:202 > + return static_cast<bool>(lhs) == static_cast<bool>(rhs) && (!static_cast<bool>(lhs) || lhs.value() == rhs.value()); Wouldn't an if statement be cleaner? Are these taken directly from C++ docs? > Source/WTF/wtf/Optional.h:256 > + return !(value == opt); Perhaps it would be useful to make one with two template types, and static_cast one to the other, to try to compare things which aren't the same type but are convertible.
(In reply to comment #2) > Attachment 275726 [details] did not pass style-queue: > > > ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:115: Consider using > EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] Using these didn't compile.(In reply to comment #3) > Comment on attachment 275726 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=275726&action=review > > I think I should wait for Anders to give the r+, but LGTM. > > > Source/WTF/wtf/Optional.h:202 > > + return static_cast<bool>(lhs) == static_cast<bool>(rhs) && (!static_cast<bool>(lhs) || lhs.value() == rhs.value()); > > Wouldn't an if statement be cleaner? But this is branchless. > > Are these taken directly from C++ docs? Some are.
(In reply to comment #4) > (In reply to comment #2) > > Attachment 275726 [details] did not pass style-queue: > > > > > > ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:115: Consider using > > EXPECT_EQ instead of EXPECT_TRUE(a == b) [readability/check] [2] > > Using these didn't compile.(In reply to comment #3) > > > Comment on attachment 275726 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=275726&action=review > > > > I think I should wait for Anders to give the r+, but LGTM. > > > > > Source/WTF/wtf/Optional.h:202 > > > + return static_cast<bool>(lhs) == static_cast<bool>(rhs) && (!static_cast<bool>(lhs) || lhs.value() == rhs.value()); > > > > Wouldn't an if statement be cleaner? > > But this is branchless. Pretty sure it isn't branchless. && and || always form branches. Besides, the compiler will be able to get rid of unnecessary branches - even if you use if statements. > > > > Are these taken directly from C++ docs? > > Some are.
http://trac.webkit.org/changeset/199107