Changeset 243486 in webkit

Timestamp:

Mar 25, 2019, 10:53:57 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

Expected shouldn't assume its contained types are copyable
​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):

Location:

trunk

Files:

• Source/WTF/ChangeLog (modified) (1 diff)
• Source/WTF/wtf/Expected.h (modified) (3 diffs)
• Source/WTF/wtf/Unexpected.h (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/contentextensions/ContentExtensionParser.cpp (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WTF/Expected.cpp (modified) (2 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WTF/wtf/Expected.h

@@ -250 +250 @@
     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;
 };
@@ -312 +314 @@
     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;
 };
@@ -559 +563 @@
 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 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 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 ? 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); }

trunk/Source/WTF/wtf/Unexpected.h

@@ -50 +50 @@
     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() &;
@@ -76 +76 @@
 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; }

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/contentextensions/ContentExtensionParser.cpp

@@ -257 +257 @@
 
     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"));
@@ -275 +275 @@
     }
     if (actionType == "make-https")
-        return {{ ActionType::MakeHTTPS }};
+        return { Action(ActionType::MakeHTTPS) };
     if (actionType == "notify") {
         JSValue notification = actionObject.get(&exec, Identifier::fromString(&exec, "notification"));

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/TestWebKitAPI/Tests/WTF/Expected.cpp

@@ -290 +290 @@
 }
 
+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)
 {
@@ -335 +346 @@
     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);
 }