Expected shouldn't assume its contained types are copyable
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Mar 2019 05:53:57 +0000 (05:53 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Mar 2019 05:53:57 +0000 (05:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195986

Patch by Alex Christensen <achristensen@webkit.org> on 2019-03-25
Reviewed by JF Bastien.

Source/WebCore:

* contentextensions/ContentExtensionParser.cpp:
(WebCore::ContentExtensions::loadAction):

Source/WTF:

* wtf/Expected.h:
(std::experimental::fundamentals_v3::__expected_detail::constexpr_base::constexpr_base):
(std::experimental::fundamentals_v3::operator==):
(std::experimental::fundamentals_v3::operator!=):
* wtf/Unexpected.h:
(std::experimental::fundamentals_v3::unexpected::unexpected):

Tools:

* TestWebKitAPI/Tests/WTF/Expected.cpp:
(TestWebKitAPI::NonCopyable::operator== const):
(TestWebKitAPI::NonCopyable::operator!= const):
(TestWebKitAPI::TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243486 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/Expected.h
Source/WTF/wtf/Unexpected.h
Source/WebCore/ChangeLog
Source/WebCore/contentextensions/ContentExtensionParser.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/Expected.cpp

index 6d12ff6..026b9cc 100644 (file)
@@ -1,3 +1,17 @@
+2019-03-25  Alex Christensen  <achristensen@webkit.org>
+
+        Expected shouldn't assume its contained types are copyable
+        https://bugs.webkit.org/show_bug.cgi?id=195986
+
+        Reviewed by JF Bastien.
+
+        * wtf/Expected.h:
+        (std::experimental::fundamentals_v3::__expected_detail::constexpr_base::constexpr_base):
+        (std::experimental::fundamentals_v3::operator==):
+        (std::experimental::fundamentals_v3::operator!=):
+        * wtf/Unexpected.h:
+        (std::experimental::fundamentals_v3::unexpected::unexpected):
+
 2019-03-24  Keith Miller  <keith_miller@apple.com>
 
         Unreviewed, forgot to refactor variable name for windows build in
index 6141773..6f36840 100644 (file)
@@ -249,8 +249,10 @@ union constexpr_storage {
     constexpr constexpr_storage() : dummy() { }
     constexpr constexpr_storage(value_tag_t) : val() { }
     constexpr constexpr_storage(error_tag_t) : err() { }
-    constexpr constexpr_storage(value_tag_t, const value_type& v) : val(v) { }
-    constexpr constexpr_storage(error_tag_t, const error_type& e) : err(e) { }
+    template<typename U = T>
+    constexpr constexpr_storage(value_tag_t, U&& v) : val(std::forward<U>(v)) { }
+    template<typename U = E>
+    constexpr constexpr_storage(error_tag_t, U&& e) : err(std::forward<U>(e)) { }
     ~constexpr_storage() = default;
 };
 
@@ -311,8 +313,10 @@ struct constexpr_base {
     constexpr constexpr_base() : s(), has(true) { }
     constexpr constexpr_base(value_tag_t tag) : s(tag), has(true) { }
     constexpr constexpr_base(error_tag_t tag) : s(tag), has(false) { }
-    constexpr constexpr_base(value_tag_t tag, const value_type& val) : s(tag, val), has(true) { }
-    constexpr constexpr_base(error_tag_t tag, const error_type& err) : s(tag, err), has(false) { }
+    template<typename U = T>
+    constexpr constexpr_base(value_tag_t tag, U&& val) : s(tag, std::forward<U>(val)), has(true) { }
+    template<typename U = E>
+    constexpr constexpr_base(error_tag_t tag, U&& err) : s(tag, std::forward<U>(err)), has(false) { }
     ~constexpr_base() = default;
 };
 
@@ -558,15 +562,15 @@ template<class T, class E> constexpr bool operator!=(const expected<T, E>& x, co
 
 template<class E> constexpr bool operator==(const expected<void, E>& x, const expected<void, E>& y) { return bool(x) == bool(y) && (x ? true : x.error() == y.error()); }
 
-template<class T, class E> constexpr bool operator==(const expected<T, E>& x, const T& y) { return x == expected<T, E>(y); }
-template<class T, class E> constexpr bool operator==(const T& x, const expected<T, E>& y) { return expected<T, E>(x) == y; }
-template<class T, class E> constexpr bool operator!=(const expected<T, E>& x, const T& y) { return x != expected<T, E>(y); }
-template<class T, class E> constexpr bool operator!=(const T& x, const expected<T, E>& y) { return expected<T, E>(x) != y; }
+template<class T, class E> constexpr bool operator==(const expected<T, E>& x, const T& y) { return x ? *x == y : false; }
+template<class T, class E> constexpr bool operator==(const T& x, const expected<T, E>& y) { return y ? x == *y : false; }
+template<class T, class E> constexpr bool operator!=(const expected<T, E>& x, const T& y) { return x ? *x != y : true; }
+template<class T, class E> constexpr bool operator!=(const T& x, const expected<T, E>& y) { return y ? x != *y : true; }
 
-template<class T, class E> constexpr bool operator==(const expected<T, E>& x, const unexpected<E>& y) { return x == expected<T, E>(y); }
-template<class T, class E> constexpr bool operator==(const unexpected<E>& x, const expected<T, E>& y) { return expected<T, E>(x) == y; }
-template<class T, class E> constexpr bool operator!=(const expected<T, E>& x, const unexpected<E>& y) { return x != expected<T, E>(y); }
-template<class T, class E> constexpr bool operator!=(const unexpected<E>& x, const expected<T, E>& y) { return expected<T, E>(x) != y; }
+template<class T, class E> constexpr bool operator==(const expected<T, E>& x, const unexpected<E>& y) { return x ? false : x.error() == y.value(); }
+template<class T, class E> constexpr bool operator==(const unexpected<E>& x, const expected<T, E>& y) { return y ? false : x.value() == y.error(); }
+template<class T, class E> constexpr bool operator!=(const expected<T, E>& x, const unexpected<E>& y) { return x ? true : x.error() != y.value(); }
+template<class T, class E> constexpr bool operator!=(const unexpected<E>& x, const expected<T, E>& y) { return y ? true : x.value() != y.error(); }
 
 template<typename T, typename E> void swap(expected<T, E>& x, expected<T, E>& y) { x.swap(y); }
 
index 193ee4d..382afb3 100644 (file)
@@ -49,8 +49,8 @@ inline namespace fundamentals_v3 {
     class unexpected {
     public:
         unexpected() = delete;
-        constexpr explicit unexpected(const E&);
-        constexpr explicit unexpected(E&&);
+        template <class U = E>
+          constexpr explicit unexpected(E&&);
         constexpr const E& value() const &;
         constexpr E& value() &;
         constexpr E&& value() &&;
@@ -75,8 +75,8 @@ template<class E>
 class unexpected {
 public:
     unexpected() = delete;
-    constexpr explicit unexpected(const E& e) : val(e) { }
-    constexpr explicit unexpected(E&& e) : val(std::forward<E>(e)) { }
+    template <class U = E>
+    constexpr explicit unexpected(U&& u) : val(std::forward<U>(u)) { }
     constexpr const E& value() const & { return val; }
     constexpr E& value() & { return val; }
     constexpr E&& value() && { return WTFMove(val); }
index fad1694..8babd8a 100644 (file)
@@ -1,3 +1,13 @@
+2019-03-25  Alex Christensen  <achristensen@webkit.org>
+
+        Expected shouldn't assume its contained types are copyable
+        https://bugs.webkit.org/show_bug.cgi?id=195986
+
+        Reviewed by JF Bastien.
+
+        * contentextensions/ContentExtensionParser.cpp:
+        (WebCore::ContentExtensions::loadAction):
+
 2019-03-20  Ryosuke Niwa  <rniwa@webkit.org>
 
         Leak of SVGFontFaceElement when RenderStyle holds onto a FontRances which uses it
index 286c80a..7723a2f 100644 (file)
@@ -256,11 +256,11 @@ static Expected<Optional<Action>, std::error_code> loadAction(ExecState& exec, c
     String actionType = asString(typeObject)->value(&exec);
 
     if (actionType == "block")
-        return {{ ActionType::BlockLoad }};
+        return { Action(ActionType::BlockLoad) };
     if (actionType == "ignore-previous-rules")
-        return {{ ActionType::IgnorePreviousRules }};
+        return { Action(ActionType::IgnorePreviousRules) };
     if (actionType == "block-cookies")
-        return {{ ActionType::BlockCookies }};
+        return { Action(ActionType::BlockCookies) };
     if (actionType == "css-display-none") {
         JSValue selector = actionObject.get(&exec, Identifier::fromString(&exec, "selector"));
         if (scope.exception() || !selector.isString())
@@ -274,7 +274,7 @@ static Expected<Optional<Action>, std::error_code> loadAction(ExecState& exec, c
         return { Action(ActionType::CSSDisplayNoneSelector, selectorString) };
     }
     if (actionType == "make-https")
-        return {{ ActionType::MakeHTTPS }};
+        return { Action(ActionType::MakeHTTPS) };
     if (actionType == "notify") {
         JSValue notification = actionObject.get(&exec, Identifier::fromString(&exec, "notification"));
         if (scope.exception() || !notification.isString())
index cbf089d..8035de6 100644 (file)
@@ -1,3 +1,15 @@
+2019-03-25  Alex Christensen  <achristensen@webkit.org>
+
+        Expected shouldn't assume its contained types are copyable
+        https://bugs.webkit.org/show_bug.cgi?id=195986
+
+        Reviewed by JF Bastien.
+
+        * TestWebKitAPI/Tests/WTF/Expected.cpp:
+        (TestWebKitAPI::NonCopyable::operator== const):
+        (TestWebKitAPI::NonCopyable::operator!= const):
+        (TestWebKitAPI::TEST):
+
 2019-03-25  Tim Horton  <timothy_horton@apple.com>
 
         Remove some now-unnecessary dynamic class lookup
index bcbe2e8..4b10c86 100644 (file)
@@ -289,6 +289,17 @@ TEST(WTF_Expected, void)
     }
 }
 
+template<typename T>
+struct NonCopyable {
+    NonCopyable(NonCopyable&&) = default;
+    NonCopyable(const NonCopyable&) = delete;
+    NonCopyable& operator=(const NonCopyable&) = delete;
+    NonCopyable& operator=(NonCopyable&&) = default;
+    bool operator==(const NonCopyable<T>& other) const { return value == other.value; }
+    bool operator!=(const NonCopyable<T>& other) const { return value != other.value; }
+    T value;
+};
+
 TEST(WTF_Expected, comparison)
 {
     typedef Expected<int, const char*> Ex;
@@ -334,6 +345,32 @@ TEST(WTF_Expected, comparison)
 
     EXPECT_FALSE(makeUnexpected(oops) == Ex(42));
     EXPECT_NE(makeUnexpected(oops), Ex(42));
+    
+    NonCopyable<int> a { 5 };
+    NonCopyable<int> b { 6 };
+    Unexpected<NonCopyable<double>> c { makeUnexpected(NonCopyable<double> { 5.0 }) };
+    Expected<NonCopyable<int>, NonCopyable<double>> d { NonCopyable<int> { 5 } };
+    Expected<NonCopyable<int>, NonCopyable<double>> e { makeUnexpected(NonCopyable<double> { 5.0 }) };
+
+    EXPECT_TRUE(a != e);
+    EXPECT_TRUE(e != a);
+    EXPECT_FALSE(a == e);
+    EXPECT_FALSE(e == a);
+
+    EXPECT_TRUE(b != e);
+    EXPECT_TRUE(e != b);
+    EXPECT_FALSE(b == e);
+    EXPECT_FALSE(e == b);
+
+    EXPECT_TRUE(c != d);
+    EXPECT_TRUE(d != c);
+    EXPECT_FALSE(c == d);
+    EXPECT_FALSE(d == c);
+
+    EXPECT_TRUE(c == e);
+    EXPECT_TRUE(e == c);
+    EXPECT_FALSE(c != e);
+    EXPECT_FALSE(e != c);
 }
 
 struct NonTrivialDtor {