Diff
Modified: trunk/Source/WTF/ChangeLog (234178 => 234179)
--- trunk/Source/WTF/ChangeLog 2018-07-24 23:14:12 UTC (rev 234178)
+++ trunk/Source/WTF/ChangeLog 2018-07-24 23:29:50 UTC (rev 234179)
@@ -1,3 +1,36 @@
+2018-07-24 Daniel Bates <[email protected]>
+
+ Move-constructing NeverDestroyed should move construct underlying object instead of copy constructing it
+ https://bugs.webkit.org/show_bug.cgi?id=187971
+
+ Reviewed by Saam Barati.
+
+ Fixes an issue where move constructing a NeverDestroyed<T> would always copy construct T into
+ the destination storage buffer regardless of whether T is move constructable. For example:
+
+ {
+ static NeverDestroyed<T> x;
+ static NeverDestroyed<T> y { WTFMove(x) }; // currently makes a copy of x's T even if T is move constructable
+ }
+
+ Currently the NeverDestroyed<T> instantiates T into the destination storage buffer by WTFMove()ing
+ the NeverDestroyed<T> into it, implicitly converting it to const T& by way of const T& conversion
+ operator. As a result the compiler picks the copy constructor for T to invoke instead of evaluating
+ whether T can be move constructed. Instead we should explicitly WTFMove() the underlying T so that
+ the compiler can choose to either move-construct T or copy construct it.
+
+ A side benefit of this change it is now possible to invoke makeNeverDestroyed() with the result
+ of a function that returns a move-only type (e.g. RenderStyle):
+
+ static auto sharedStyle = makeNeverDestroyed([] {
+ auto style = RenderStyle::create();
+ ...
+ return style;
+ }());
+
+ * wtf/NeverDestroyed.h:
+ (WTF::NeverDestroyed::NeverDestroyed):
+
2018-07-24 Ryan Haddad <[email protected]>
Unreviewed, rolling out r234121.
Modified: trunk/Source/WTF/wtf/NeverDestroyed.h (234178 => 234179)
--- trunk/Source/WTF/wtf/NeverDestroyed.h 2018-07-24 23:14:12 UTC (rev 234178)
+++ trunk/Source/WTF/wtf/NeverDestroyed.h 2018-07-24 23:29:50 UTC (rev 234179)
@@ -52,7 +52,7 @@
NeverDestroyed(NeverDestroyed&& other)
{
- MaybeRelax<T>(new (storagePointer()) T(WTFMove(other)));
+ MaybeRelax<T>(new (storagePointer()) T(WTFMove(*other.storagePointer())));
}
operator T&() { return *storagePointer(); }
Modified: trunk/Tools/ChangeLog (234178 => 234179)
--- trunk/Tools/ChangeLog 2018-07-24 23:14:12 UTC (rev 234178)
+++ trunk/Tools/ChangeLog 2018-07-24 23:29:50 UTC (rev 234179)
@@ -1,3 +1,26 @@
+2018-07-24 Daniel Bates <[email protected]>
+
+ Move-constructing NeverDestroyed should move construct underlying object instead of copy constructing it
+ https://bugs.webkit.org/show_bug.cgi?id=187971
+
+ Reviewed by Saam Barati.
+
+ Add unit tests to cover move constructing a NeverDestroyed with a non-const and const data type
+ as well as invoking makeNeverDestroyed() with the result of a function that returns a move-only
+ data type.
+
+ * TestWebKitAPI/CMakeLists.txt:
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.cpp: Added.
+ (TestWebKitAPI::MoveOnlyLifecycleLogger::MoveOnlyLifecycleLogger):
+ (TestWebKitAPI::MoveOnlyLifecycleLogger::~MoveOnlyLifecycleLogger):
+ (TestWebKitAPI::MoveOnlyLifecycleLogger::operator=):
+ (TestWebKitAPI::MoveOnlyLifecycleLogger::setName):
+ (TestWebKitAPI::TEST):
+ * TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.h: Added.
+ * TestWebKitAPI/Tests/WTF/NeverDestroyed.cpp:
+ (TestWebKitAPI::TEST):
+
2018-07-24 Thomas Denney <[email protected]>
Minor changes to the WHLSL interpreter to support the new compiler
https://bugs.webkit.org/show_bug.cgi?id=187728
Modified: trunk/Tools/TestWebKitAPI/CMakeLists.txt (234178 => 234179)
--- trunk/Tools/TestWebKitAPI/CMakeLists.txt 2018-07-24 23:14:12 UTC (rev 234178)
+++ trunk/Tools/TestWebKitAPI/CMakeLists.txt 2018-07-24 23:29:50 UTC (rev 234179)
@@ -117,6 +117,7 @@
${TESTWEBKITAPI_DIR}/Tests/WTF/MathExtras.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/MediaTime.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/MetaAllocator.cpp
+ ${TESTWEBKITAPI_DIR}/Tests/WTF/MoveOnlyLifecycleLogger.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/NakedPtr.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/NeverDestroyed.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/Optional.cpp
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (234178 => 234179)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2018-07-24 23:14:12 UTC (rev 234178)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2018-07-24 23:29:50 UTC (rev 234179)
@@ -748,6 +748,7 @@
CE4D5DE71F6743BA0072CFC6 /* StringWithDirection.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE4D5DE51F6743BA0072CFC6 /* StringWithDirection.cpp */; };
CE6E81A020A6935F00E2C80F /* SetTimeoutFunction.mm in Sources */ = {isa = PBXBuildFile; fileRef = CE6E819F20A6935F00E2C80F /* SetTimeoutFunction.mm */; };
CE6E81A420A933D500E2C80F /* set-timeout-function.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = CE6E81A320A933B800E2C80F /* set-timeout-function.html */; };
+ CE78705F2107AB980053AC67 /* MoveOnlyLifecycleLogger.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE78705D2107AB8C0053AC67 /* MoveOnlyLifecycleLogger.cpp */; };
CEA6CF2819CCF69D0064F5A7 /* open-and-close-window.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = CEA6CF2719CCF69D0064F5A7 /* open-and-close-window.html */; };
CEA7F57D2089624B0078EF6E /* DidResignInputElementStrongPasswordAppearance.mm in Sources */ = {isa = PBXBuildFile; fileRef = CEA7F57B20895F5B0078EF6E /* DidResignInputElementStrongPasswordAppearance.mm */; };
CEBABD491B71687C0051210A /* should-open-external-schemes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = CEBABD481B71687C0051210A /* should-open-external-schemes.html */; };
@@ -1945,6 +1946,8 @@
CE50D8C81C8665CE0072EA5A /* OptionSet.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = OptionSet.cpp; sourceTree = "<group>"; };
CE6E819F20A6935F00E2C80F /* SetTimeoutFunction.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SetTimeoutFunction.mm; sourceTree = "<group>"; };
CE6E81A320A933B800E2C80F /* set-timeout-function.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = "set-timeout-function.html"; path = "ios/set-timeout-function.html"; sourceTree = SOURCE_ROOT; };
+ CE78705C2107AB8C0053AC67 /* MoveOnlyLifecycleLogger.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MoveOnlyLifecycleLogger.h; sourceTree = "<group>"; };
+ CE78705D2107AB8C0053AC67 /* MoveOnlyLifecycleLogger.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MoveOnlyLifecycleLogger.cpp; sourceTree = "<group>"; };
CEA6CF2219CCF5BD0064F5A7 /* OpenAndCloseWindow.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = OpenAndCloseWindow.mm; sourceTree = "<group>"; };
CEA6CF2719CCF69D0064F5A7 /* open-and-close-window.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "open-and-close-window.html"; sourceTree = "<group>"; };
CEA7F57B20895F5B0078EF6E /* DidResignInputElementStrongPasswordAppearance.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = DidResignInputElementStrongPasswordAppearance.mm; sourceTree = "<group>"; };
@@ -2934,6 +2937,8 @@
CD5497B315857F0C00B5BC30 /* MediaTime.cpp */,
0FC6C4CE141034AD005B7F0C /* MetaAllocator.cpp */,
93A427AC180DA60F00CD24D7 /* MoveOnly.h */,
+ CE78705D2107AB8C0053AC67 /* MoveOnlyLifecycleLogger.cpp */,
+ CE78705C2107AB8C0053AC67 /* MoveOnlyLifecycleLogger.h */,
FEB6F74E1B2BA44E009E4922 /* NakedPtr.cpp */,
A57D54F21F338C3600A97AA7 /* NeverDestroyed.cpp */,
1AFDE6541953B2C000C48FFA /* Optional.cpp */,
@@ -3508,6 +3513,7 @@
7C83DEEF1D0A590C00FEBCF3 /* MD5.cpp in Sources */,
7C83DEF11D0A590C00FEBCF3 /* MediaTime.cpp in Sources */,
7C83DEF61D0A590C00FEBCF3 /* MetaAllocator.cpp in Sources */,
+ CE78705F2107AB980053AC67 /* MoveOnlyLifecycleLogger.cpp in Sources */,
7C83DEFE1D0A590C00FEBCF3 /* NakedPtr.cpp in Sources */,
A57D54F31F338C3600A97AA7 /* NeverDestroyed.cpp in Sources */,
7C83DF011D0A590C00FEBCF3 /* Optional.cpp in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.cpp (0 => 234179)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.cpp (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.cpp 2018-07-24 23:29:50 UTC (rev 234179)
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "MoveOnlyLifecycleLogger.h"
+
+namespace TestWebKitAPI {
+
+MoveOnlyLifecycleLogger::MoveOnlyLifecycleLogger()
+{
+ log() << "construct(" << name << ") ";
+}
+
+MoveOnlyLifecycleLogger::MoveOnlyLifecycleLogger(const char* name)
+ : name { name }
+{
+ log() << "construct(" << name << ") ";
+}
+
+MoveOnlyLifecycleLogger::~MoveOnlyLifecycleLogger()
+{
+ log() << "destruct(" << name << ") ";
+}
+
+MoveOnlyLifecycleLogger::MoveOnlyLifecycleLogger(MoveOnlyLifecycleLogger&& other)
+{
+ std::swap(name, other.name);
+ log() << "move-construct(" << name << ") ";
+}
+
+MoveOnlyLifecycleLogger& MoveOnlyLifecycleLogger::operator=(MoveOnlyLifecycleLogger&& other)
+{
+ std::swap(name, other.name);
+ log() << "move-assign(" << name << ") ";
+ return *this;
+}
+
+void MoveOnlyLifecycleLogger::setName(const char* newName)
+{
+ name = newName;
+ log() << "set-name(" << name << ") ";
+}
+
+TEST(MoveOnlyLifecycleLogger, Basic)
+{
+ { MoveOnlyLifecycleLogger l; }
+ ASSERT_STREQ("construct(<default>) destruct(<default>) ", takeLogStr().c_str());
+
+ { MoveOnlyLifecycleLogger l("a"); }
+ ASSERT_STREQ("construct(a) destruct(a) ", takeLogStr().c_str());
+
+ { MoveOnlyLifecycleLogger l("d"); MoveOnlyLifecycleLogger l2(WTFMove(l)); }
+ ASSERT_STREQ("construct(d) move-construct(d) destruct(d) destruct(<default>) ", takeLogStr().c_str());
+
+ { MoveOnlyLifecycleLogger l("e"); MoveOnlyLifecycleLogger l2; l2 = WTFMove(l); }
+ ASSERT_STREQ("construct(e) construct(<default>) move-assign(e) destruct(e) destruct(<default>) ", takeLogStr().c_str());
+
+ { MoveOnlyLifecycleLogger l("f"); l.setName("x"); }
+ ASSERT_STREQ("construct(f) set-name(x) destruct(x) ", takeLogStr().c_str());
+
+ { static MoveOnlyLifecycleLogger l("g"); }
+ ASSERT_STREQ("construct(g) ", takeLogStr().c_str());
+}
+
+}
Added: trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.h (0 => 234179)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.h (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/MoveOnlyLifecycleLogger.h 2018-07-24 23:29:50 UTC (rev 234179)
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include "Logger.h"
+
+namespace TestWebKitAPI {
+
+struct MoveOnlyLifecycleLogger {
+ MoveOnlyLifecycleLogger();
+ MoveOnlyLifecycleLogger(const char*);
+
+ ~MoveOnlyLifecycleLogger();
+
+ MoveOnlyLifecycleLogger(MoveOnlyLifecycleLogger&&);
+ MoveOnlyLifecycleLogger& operator=(MoveOnlyLifecycleLogger&&);
+
+ void setName(const char*);
+
+ const char* name { "<default>" };
+};
+
+}
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/NeverDestroyed.cpp (234178 => 234179)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/NeverDestroyed.cpp 2018-07-24 23:14:12 UTC (rev 234178)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/NeverDestroyed.cpp 2018-07-24 23:29:50 UTC (rev 234179)
@@ -26,6 +26,7 @@
#include "config.h"
#include "LifecycleLogger.h"
+#include "MoveOnlyLifecycleLogger.h"
#include <wtf/NeverDestroyed.h>
#include <wtf/Vector.h>
@@ -49,6 +50,46 @@
ASSERT_STREQ(x.get().name, "name");
}
ASSERT_STREQ("construct(name) copy-construct(name) set-name(x) destruct(x) ", takeLogStr().c_str());
+
+ { static NeverDestroyed<LifecycleLogger> x { [] { return LifecycleLogger { "name" }; }() }; UNUSED_PARAM(x); }
+ ASSERT_STREQ("construct(name) move-construct(name) destruct(<default>) ", takeLogStr().c_str());
+
+ {
+ static auto x = makeNeverDestroyed([] {
+ LifecycleLogger l { "name" };
+ l.setName("x");
+ return l;
+ }());
+ ASSERT_STREQ(x.get().name, "x");
+ }
+ ASSERT_STREQ("construct(name) set-name(x) move-construct(x) destruct(<default>) ", takeLogStr().c_str());
+
+ {
+ static NeverDestroyed<LifecycleLogger> x;
+ static NeverDestroyed<LifecycleLogger> y { WTFMove(x) };
+ UNUSED_PARAM(y);
+ }
+ ASSERT_STREQ("construct(<default>) move-construct(<default>) ", takeLogStr().c_str());
+
+ {
+ static NeverDestroyed<const LifecycleLogger> x;
+ static NeverDestroyed<const LifecycleLogger> y { WTFMove(x) };
+ UNUSED_PARAM(y);
+ }
+ ASSERT_STREQ("construct(<default>) move-construct(<default>) ", takeLogStr().c_str());
+
+ { static NeverDestroyed<MoveOnlyLifecycleLogger> x { [] { return MoveOnlyLifecycleLogger { "name" }; }() }; UNUSED_PARAM(x); }
+ ASSERT_STREQ("construct(name) move-construct(name) destruct(<default>) ", takeLogStr().c_str());
+
+ {
+ static auto x = makeNeverDestroyed([] {
+ MoveOnlyLifecycleLogger l { "name" };
+ l.setName("x");
+ return l;
+ }());
+ UNUSED_PARAM(x);
+ }
+ ASSERT_STREQ("construct(name) set-name(x) move-construct(x) destruct(<default>) ", takeLogStr().c_str());
}
static const Vector<int>& list()
