Attachment 357360 Details for Bug 192728 – Patch v1

[patch] Patch v1

bug-192728-20181214171446.patch (text/plain), 5.57 KB, created by David Kilzer (:ddkilzer) on 2018-12-14 17:14:46 PST

(hide)

Description:

Filename:

MIME Type:

Creator: David Kilzer (:ddkilzer)

Created: 2018-12-14 17:14:46 PST

Size: 5.57 KB

patch

obsolete

>Subversion Revision: 239112
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index c6721b159da817e1dfe81e0c396c10c9ea0b9224..7b7fa6e75b7c2aed45f5e98deeeb346488c78ffe 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-12-14  David Kilzer  <ddkilzer@apple.com>
>+
>+        wtf/Optional.h: move-constructor and move-assignment operator should disengage the value being moved from
>+        <https://webkit.org/b/192728>
>+        <rdar://problem/46746779>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/Optional.h:
>+        (std::optional::optional(optional&&)):
>+        (std::optional::operator=(optional&&)):
>+        - Clear the right-hand side (rhs) value after the move.
>+
> 2018-12-14  David Kilzer  <ddkilzer@apple.com>
> 
>         clang-tidy: Fix unnecessary copy of AtomicString each time one is logged
>diff --git a/Source/WTF/wtf/Optional.h b/Source/WTF/wtf/Optional.h
>index 1cb3aa8e710021967aeb1cf4f68bc4fda547d832..de23f1532097741a2874c5554b32b466bdda7cc8 100644
>--- a/Source/WTF/wtf/Optional.h
>+++ b/Source/WTF/wtf/Optional.h
>@@ -424,6 +424,7 @@ public:
>     if (rhs.initialized()) {
>         ::new (static_cast<void*>(dataptr())) T(std::move(*rhs));
>         OptionalBase<T>::init_ = true;
>+        rhs.clear();
>     }
>   }
> 
>@@ -461,8 +462,8 @@ public:
>   __NOEXCEPT_(detail_::is_nothrow_move_assignable<T>::value && detail_::is_nothrow_move_constructible<T>::value)
>   {
>     if      (initialized() == true  && rhs.initialized() == false) clear();
>-    else if (initialized() == false && rhs.initialized() == true)  initialize(std::move(*rhs));
>-    else if (initialized() == true  && rhs.initialized() == true)  contained_val() = std::move(*rhs);
>+    else if (initialized() == false && rhs.initialized() == true)  { initialize(std::move(*rhs)); rhs.clear(); }
>+    else if (initialized() == true  && rhs.initialized() == true)  { contained_val() = std::move(*rhs); rhs.clear(); }
>     return *this;
>   }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index b2bfc801513e77db963f766ce53f05929f1ec56a..4dd0d3d07ca9b05e6af79fa6a94c53bb2763b5e4 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-12-14  David Kilzer  <ddkilzer@apple.com>
>+
>+        wtf/Optional.h: move-constructor and move-assignment operator should disengage the value being moved from
>+        <https://webkit.org/b/192728>
>+        <rdar://problem/46746779>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WTF/Optional.cpp:
>+        (TestWebKitAPI::TEST):
>+        - Add tests for wtf/Optional.h move semantics.
>+
> 2018-12-12  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239103.
>diff --git a/Tools/TestWebKitAPI/Tests/WTF/Optional.cpp b/Tools/TestWebKitAPI/Tests/WTF/Optional.cpp
>index a70881f35c982707d8736d691cdaa2f7709e20ed..beac0046740e0f0702fa5dd602329e9507c83313 100644
>--- a/Tools/TestWebKitAPI/Tests/WTF/Optional.cpp
>+++ b/Tools/TestWebKitAPI/Tests/WTF/Optional.cpp
>@@ -146,5 +146,93 @@ TEST(WTF_Optional, Inequality)
>     EXPECT_TRUE(1 != unengaged1);
> }
> 
>+TEST(WTF_Optional, MoveSemanticsConstructor)
>+{
>+    {
>+        std::optional<int> unengaged1 { };
>+        EXPECT_FALSE(!!unengaged1);
>+
>+        std::optional<int> unengaged2(std::move(unengaged1));
>+        EXPECT_FALSE(!!unengaged1);
>+        EXPECT_FALSE(!!unengaged2);
>+    }
>+
>+    {
>+        std::optional<int> engagedToUnengaged { 2 };
>+        EXPECT_TRUE(!!engagedToUnengaged);
>+        EXPECT_EQ(2, engagedToUnengaged.value());
>+
>+        std::optional<int> engaged(std::move(engagedToUnengaged));
>+        EXPECT_FALSE(!!engagedToUnengaged);
>+        EXPECT_TRUE(!!engaged);
>+        EXPECT_EQ(2, engaged.value());
>+    }
>+}
>+
>+TEST(WTF_Optional, MoveSemanticsAssignment)
>+{
>+    {
>+        std::optional<int> unengaged1 { };
>+        EXPECT_FALSE(!!unengaged1);
>+
>+        std::optional<int> unengaged2;
>+        EXPECT_FALSE(!!unengaged2);
>+
>+        unengaged2 = std::move(unengaged1);
>+        EXPECT_FALSE(!!unengaged1);
>+        EXPECT_FALSE(!!unengaged2);
>+    }
>+
>+    {
>+        std::optional<int> engagedToUnengaged { 3 };
>+        EXPECT_TRUE(!!engagedToUnengaged);
>+        EXPECT_EQ(3, engagedToUnengaged.value());
>+
>+        engagedToUnengaged = std::move(std::nullopt);
>+        EXPECT_FALSE(!!engagedToUnengaged);
>+    }
>+
>+    {
>+        std::optional<int> engagedToUnengaged { 4 };
>+        EXPECT_TRUE(!!engagedToUnengaged);
>+        EXPECT_EQ(4, engagedToUnengaged.value());
>+
>+        std::optional<int> unengagedToEngaged;
>+        EXPECT_FALSE(!!unengagedToEngaged);
>+
>+        unengagedToEngaged = std::move(engagedToUnengaged);
>+        EXPECT_FALSE(!!engagedToUnengaged);
>+        EXPECT_TRUE(!!unengagedToEngaged);
>+        EXPECT_EQ(4, unengagedToEngaged.value());
>+    }
>+
>+    {
>+        std::optional<int> engagedToUnengaged { 5 };
>+        EXPECT_TRUE(!!engagedToUnengaged);
>+        EXPECT_EQ(5, engagedToUnengaged.value());
>+
>+        std::optional<int> unengaged;
>+        EXPECT_FALSE(!!unengaged);
>+
>+        engagedToUnengaged = std::move(unengaged);
>+        EXPECT_FALSE(!!engagedToUnengaged);
>+        EXPECT_FALSE(!!unengaged);
>+    }
>+
>+    {
>+        std::optional<int> engagedToUnengaged { 6 };
>+        EXPECT_TRUE(!!engagedToUnengaged);
>+        EXPECT_EQ(6, engagedToUnengaged.value());
>+
>+        std::optional<int> engagedToEngaged { 7 };
>+        EXPECT_TRUE(!!engagedToEngaged);
>+        EXPECT_EQ(7, engagedToEngaged.value());
>+
>+        engagedToEngaged = std::move(engagedToUnengaged);
>+        EXPECT_FALSE(!!engagedToUnengaged);
>+        EXPECT_TRUE(!!engagedToEngaged);
>+        EXPECT_EQ(6, engagedToEngaged.value());
>+    }
>+}
> 
> } // namespace TestWebKitAPI

Actions: View | Formatted Diff | Diff

Attachments on bug 192728: 357360 | 357374 | 357376 | 357377 | 357379 | 357407 | 357409 | 357411 | 357413 | 357414 | 357415 | 357416 | 357417 | 357444 | 357445 | 357492 | 357514 | 357519 | 357521 | 357534 | 357769 | 357772