Title: [207225] trunk
Revision
207225
Author
wei...@apple.com
Date
2016-10-12 10:41:00 -0700 (Wed, 12 Oct 2016)

Log Message

Optional's move-constructor and move-assignment operator don't disengage the value being moved from
https://bugs.webkit.org/show_bug.cgi?id=163309

Reviewed by Anders Carlsson.

Source/WTF:

* wtf/Optional.h:
(WTF::Optional::Optional):
(WTF::Optional::operator=):
Disengage 'other' on move-construction and move-assignment.

Tools:

* TestWebKitAPI/Tests/WTF/Optional.cpp:
(TestWebKitAPI::TEST):
Add tests for Optional's move-constructor and move-assignment operator.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (207224 => 207225)


--- trunk/Source/WTF/ChangeLog	2016-10-12 17:31:03 UTC (rev 207224)
+++ trunk/Source/WTF/ChangeLog	2016-10-12 17:41:00 UTC (rev 207225)
@@ -1,3 +1,15 @@
+2016-10-11  Sam Weinig  <s...@webkit.org>
+
+        Optional's move-constructor and move-assignment operator don't disengage the value being moved from
+        https://bugs.webkit.org/show_bug.cgi?id=163309
+
+        Reviewed by Anders Carlsson.
+
+        * wtf/Optional.h:
+        (WTF::Optional::Optional):
+        (WTF::Optional::operator=):
+        Disengage 'other' on move-construction and move-assignment.
+
 2016-10-08  Filip Pizlo  <fpi...@apple.com>
 
         MarkedBlock should know what objects are live during marking

Modified: trunk/Source/WTF/wtf/Optional.h (207224 => 207225)


--- trunk/Source/WTF/wtf/Optional.h	2016-10-12 17:31:03 UTC (rev 207224)
+++ trunk/Source/WTF/wtf/Optional.h	2016-10-12 17:41:00 UTC (rev 207225)
@@ -71,8 +71,10 @@
     Optional(Optional&& other)
         : m_isEngaged(other.m_isEngaged)
     {
-        if (m_isEngaged)
+        if (m_isEngaged) {
             new (NotNull, &m_value) T(WTFMove(*other.asPtr()));
+            other.m_isEngaged = false;
+        }
     }
 
     Optional(T&& value)
@@ -121,6 +123,7 @@
         if (other.m_isEngaged) {
             new (NotNull, &m_value) T(WTFMove(*other.asPtr()));
             m_isEngaged = true;
+            other.m_isEngaged = false;
         }
         return *this;
     }

Modified: trunk/Tools/ChangeLog (207224 => 207225)


--- trunk/Tools/ChangeLog	2016-10-12 17:31:03 UTC (rev 207224)
+++ trunk/Tools/ChangeLog	2016-10-12 17:41:00 UTC (rev 207225)
@@ -1,3 +1,14 @@
+2016-10-11  Sam Weinig  <s...@webkit.org>
+
+        Optional's move-constructor and move-assignment operator don't disengage the value being moved from
+        https://bugs.webkit.org/show_bug.cgi?id=163309
+
+        Reviewed by Anders Carlsson.
+
+        * TestWebKitAPI/Tests/WTF/Optional.cpp:
+        (TestWebKitAPI::TEST):
+        Add tests for Optional's move-constructor and move-assignment operator.
+
 2016-10-12  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Now playing media sessions are always cleared for the active foreground tab

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/Optional.cpp (207224 => 207225)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/Optional.cpp	2016-10-12 17:31:03 UTC (rev 207224)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/Optional.cpp	2016-10-12 17:41:00 UTC (rev 207225)
@@ -25,6 +25,7 @@
 
 #include "config.h"
 
+#include "Counters.h"
 #include <wtf/Optional.h>
 
 namespace TestWebKitAPI {
@@ -146,5 +147,52 @@
     EXPECT_TRUE(1 != unengaged1);
 }
 
+TEST(WTF_Optional, MoveConstructor)
+{
+    {
+        CopyMoveCounter::TestingScope scope;
 
+        Optional<CopyMoveCounter> optional(InPlace);
+
+        EXPECT_EQ(1U, CopyMoveCounter::constructionCount);
+        EXPECT_EQ(0U, CopyMoveCounter::copyCount);
+        EXPECT_EQ(0U, CopyMoveCounter::moveCount);
+
+        EXPECT_TRUE(static_cast<bool>(optional));
+    
+        Optional<CopyMoveCounter> movedTo(WTFMove(optional));
+
+        EXPECT_EQ(1U, CopyMoveCounter::constructionCount);
+        EXPECT_EQ(0U, CopyMoveCounter::copyCount);
+        EXPECT_EQ(1U, CopyMoveCounter::moveCount);
+
+        EXPECT_FALSE(static_cast<bool>(optional));
+        EXPECT_TRUE(static_cast<bool>(movedTo));
+    }
+}
+
+TEST(WTF_Optional, MoveAssignment)
+{
+    {
+        CopyMoveCounter::TestingScope scope;
+
+        Optional<CopyMoveCounter> optional(InPlace);
+
+        EXPECT_EQ(1U, CopyMoveCounter::constructionCount);
+        EXPECT_EQ(0U, CopyMoveCounter::copyCount);
+        EXPECT_EQ(0U, CopyMoveCounter::moveCount);
+
+        EXPECT_TRUE(static_cast<bool>(optional));
+    
+        Optional<CopyMoveCounter> movedTo = WTFMove(optional);
+
+        EXPECT_EQ(1U, CopyMoveCounter::constructionCount);
+        EXPECT_EQ(0U, CopyMoveCounter::copyCount);
+        EXPECT_EQ(1U, CopyMoveCounter::moveCount);
+
+        EXPECT_FALSE(static_cast<bool>(optional));
+        EXPECT_TRUE(static_cast<bool>(movedTo));
+    }
+}
+
 } // namespace TestWebKitAPI
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to