Title: [220601] trunk
Revision
220601
Author
wei...@apple.com
Date
2017-08-11 09:56:37 -0700 (Fri, 11 Aug 2017)

Log Message

WTF::Function does not allow for reference / non-default constructible return types
https://bugs.webkit.org/show_bug.cgi?id=175244
Source/_javascript_Core:


Reviewed by Chris Dumez.

* runtime/ArrayBuffer.cpp:
(JSC::ArrayBufferContents::transferTo):
Call reset(), rather than clear() to avoid the call to destroy() in clear(). The
destroy call needed to be a no-op anyway, since the data is being moved.

Source/WebCore:


Reviewed by Chris Dumez.

* bindings/js/JSCustomElementInterface.h:
(WebCore::JSCustomElementInterface::invokeCallback):
Update the default value for the addArguments parameter to be an empty lambda, rather than
default initialization, which leads to a null WTF::Function. This allows us to remove support
for calling null WTF::Function. No change in behavior.

Source/WebKit:


Reviewed by Chris Dumez.

* UIProcess/WebResourceLoadStatisticsStore.h:
Update the default value for the updateCookiePartitioningForDomainsHandler parameter to be an 
empty lambda, rather than default initialization, which leads to a null WTF::Function. This allows
us to remove support for calling null WTF::Function. No change in behavior.

Source/WTF:


Reviewed by Chris Dumez.

When Function, then NoncopyableFunction, was templatized to allow non-void return values
in r201493, it maintained the behavior of being callable even if the Function was null.
To accomplish this, when null, the default construction of the return parameter was used.
This means Function can't be used with return types that are not default constructible,
such as reference types and Ref.

This behavior of returning something when null is surprising, as this is not how normal
functions behave, and not very useful. Instead, we now assert that the function is not
null when being called.

* wtf/Function.h:
(WTF::Function operator(...)):
Instead of allowing a null callable wrapper by returning the default construction of
the return type, assert that the wrapper is there when calling a Function.

Tools:

<rdar://problem/33801582>

Reviewed by Chris Dumez.

* TestWebKitAPI/Tests/WTF/Function.cpp:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (220600 => 220601)


--- trunk/Source/_javascript_Core/ChangeLog	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-08-11 16:56:37 UTC (rev 220601)
@@ -1,3 +1,15 @@
+2017-08-10  Sam Weinig  <s...@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        Reviewed by Chris Dumez.
+
+        * runtime/ArrayBuffer.cpp:
+        (JSC::ArrayBufferContents::transferTo):
+        Call reset(), rather than clear() to avoid the call to destroy() in clear(). The
+        destroy call needed to be a no-op anyway, since the data is being moved.
+
 2017-08-11  Mark Lam  <mark....@apple.com>
 
         Gardening: fix CLoop build.

Modified: trunk/Source/_javascript_Core/runtime/ArrayBuffer.cpp (220600 => 220601)


--- trunk/Source/_javascript_Core/runtime/ArrayBuffer.cpp	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/_javascript_Core/runtime/ArrayBuffer.cpp	2017-08-11 16:56:37 UTC (rev 220601)
@@ -132,7 +132,7 @@
     other.m_sizeInBytes = m_sizeInBytes;
     other.m_destructor = WTFMove(m_destructor);
     other.m_shared = m_shared;
-    clear();
+    reset();
 }
 
 void ArrayBufferContents::copyTo(ArrayBufferContents& other)

Modified: trunk/Source/WTF/ChangeLog (220600 => 220601)


--- trunk/Source/WTF/ChangeLog	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/WTF/ChangeLog	2017-08-11 16:56:37 UTC (rev 220601)
@@ -1,3 +1,25 @@
+2017-08-10  Sam Weinig  <s...@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        Reviewed by Chris Dumez.
+
+        When Function, then NoncopyableFunction, was templatized to allow non-void return values
+        in r201493, it maintained the behavior of being callable even if the Function was null.
+        To accomplish this, when null, the default construction of the return parameter was used.
+        This means Function can't be used with return types that are not default constructible,
+        such as reference types and Ref.
+
+        This behavior of returning something when null is surprising, as this is not how normal
+        functions behave, and not very useful. Instead, we now assert that the function is not
+        null when being called.
+
+        * wtf/Function.h:
+        (WTF::Function operator(...)):
+        Instead of allowing a null callable wrapper by returning the default construction of
+        the return type, assert that the wrapper is there when calling a Function.
+
 2017-08-10  Mark Lam  <mark....@apple.com>
 
         Make the MASM_PROBE mechanism mandatory for DFG and FTL builds.

Modified: trunk/Source/WTF/wtf/Function.h (220600 => 220601)


--- trunk/Source/WTF/wtf/Function.h	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/WTF/wtf/Function.h	2017-08-11 16:56:37 UTC (rev 220601)
@@ -52,9 +52,8 @@
 
     Out operator()(In... in) const
     {
-        if (m_callableWrapper)
-            return m_callableWrapper->call(std::forward<In>(in)...);
-        return Out();
+        ASSERT(m_callableWrapper);
+        return m_callableWrapper->call(std::forward<In>(in)...);
     }
 
     explicit operator bool() const { return !!m_callableWrapper; }

Modified: trunk/Source/WebCore/ChangeLog (220600 => 220601)


--- trunk/Source/WebCore/ChangeLog	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/WebCore/ChangeLog	2017-08-11 16:56:37 UTC (rev 220601)
@@ -1,3 +1,16 @@
+2017-08-10  Sam Weinig  <s...@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        Reviewed by Chris Dumez.
+
+        * bindings/js/JSCustomElementInterface.h:
+        (WebCore::JSCustomElementInterface::invokeCallback):
+        Update the default value for the addArguments parameter to be an empty lambda, rather than
+        default initialization, which leads to a null WTF::Function. This allows us to remove support
+        for calling null WTF::Function. No change in behavior.
+
 2017-08-11  Antti Koivisto  <an...@apple.com>
 
         Remove RenderQuote collection from RenderView

Modified: trunk/Source/WebCore/bindings/js/JSCustomElementInterface.h (220600 => 220601)


--- trunk/Source/WebCore/bindings/js/JSCustomElementInterface.h	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/WebCore/bindings/js/JSCustomElementInterface.h	2017-08-11 16:56:37 UTC (rev 220601)
@@ -94,7 +94,7 @@
 
     RefPtr<Element> tryToConstructCustomElement(Document&, const AtomicString&);
 
-    void invokeCallback(Element&, JSC::JSObject* callback, const WTF::Function<void(JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&)>& addArguments = { });
+    void invokeCallback(Element&, JSC::JSObject* callback, const WTF::Function<void(JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&)>& addArguments = [](JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&) { });
 
     QualifiedName m_name;
     JSC::Weak<JSC::JSObject> m_constructor;

Modified: trunk/Source/WebKit/ChangeLog (220600 => 220601)


--- trunk/Source/WebKit/ChangeLog	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/WebKit/ChangeLog	2017-08-11 16:56:37 UTC (rev 220601)
@@ -1,3 +1,15 @@
+2017-08-10  Sam Weinig  <s...@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        Reviewed by Chris Dumez.
+
+        * UIProcess/WebResourceLoadStatisticsStore.h:
+        Update the default value for the updateCookiePartitioningForDomainsHandler parameter to be an 
+        empty lambda, rather than default initialization, which leads to a null WTF::Function. This allows
+        us to remove support for calling null WTF::Function. No change in behavior.
+
 2017-08-11  Adrian Perez de Castro  <ape...@igalia.com>
 
         [WPE] Build failure with Clang 4.0.1: no matching conversion for functional-style cast from 'pointer' (aka 'unsigned short *') to 'WTF::String'

Modified: trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h (220600 => 220601)


--- trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h	2017-08-11 16:56:37 UTC (rev 220601)
@@ -60,7 +60,7 @@
 class WebResourceLoadStatisticsStore final : public IPC::Connection::WorkQueueMessageReceiver {
 public:
     using UpdateCookiePartitioningForDomainsHandler = WTF::Function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, ShouldClearFirst)>;
-    static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, UpdateCookiePartitioningForDomainsHandler&& updateCookiePartitioningForDomainsHandler = { })
+    static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, UpdateCookiePartitioningForDomainsHandler&& updateCookiePartitioningForDomainsHandler = [](const Vector<String>&, const Vector<String>&, ShouldClearFirst) { })
     {
         return adoptRef(*new WebResourceLoadStatisticsStore(resourceLoadStatisticsDirectory, WTFMove(testingCallback), WTFMove(updateCookiePartitioningForDomainsHandler)));
     }

Modified: trunk/Tools/ChangeLog (220600 => 220601)


--- trunk/Tools/ChangeLog	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Tools/ChangeLog	2017-08-11 16:56:37 UTC (rev 220601)
@@ -1,3 +1,14 @@
+2017-08-11  Sam Weinig  <s...@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+        <rdar://problem/33801582>
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WTF/Function.cpp:
+        (TestWebKitAPI::TEST):
+
 2017-08-11  Carlos Garcia Campos  <cgar...@igalia.com>
 
         [Soup] Cannot access HTTPS sites using a HTTP proxy that requires authentication

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/Function.cpp (220600 => 220601)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/Function.cpp	2017-08-11 16:54:45 UTC (rev 220600)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/Function.cpp	2017-08-11 16:56:37 UTC (rev 220601)
@@ -141,7 +141,6 @@
 {
     Function<unsigned()> a;
     EXPECT_FALSE(static_cast<bool>(a));
-    EXPECT_EQ(0U, a());
 
     a = [] {
         return 1U;
@@ -151,7 +150,6 @@
 
     a = nullptr;
     EXPECT_FALSE(static_cast<bool>(a));
-    EXPECT_EQ(0U, a());
 
     a = MoveOnly { 2 };
     EXPECT_TRUE(static_cast<bool>(a));
@@ -161,7 +159,6 @@
     EXPECT_TRUE(static_cast<bool>(b));
     EXPECT_EQ(2U, b());
     EXPECT_FALSE(static_cast<bool>(a));
-    EXPECT_EQ(0U, a());
 
     a = MoveOnly { 3 };
     Function<unsigned()> c = WTFMove(a);
@@ -168,17 +165,11 @@
     EXPECT_TRUE(static_cast<bool>(c));
     EXPECT_EQ(3U, c());
     EXPECT_FALSE(static_cast<bool>(a));
-    EXPECT_EQ(0U, a());
 
     b = WTFMove(c);
     EXPECT_TRUE(static_cast<bool>(b));
     EXPECT_EQ(3U, b());
     EXPECT_FALSE(static_cast<bool>(c));
-    EXPECT_EQ(0U, c());
-
-    Function<unsigned()> d = nullptr;
-    EXPECT_FALSE(static_cast<bool>(d));
-    EXPECT_EQ(0U, d());
 }
 
 struct FunctionDestructionChecker {
@@ -222,15 +213,6 @@
     FunctionDestructionChecker::functionResult = std::nullopt;
 
     a = FunctionDestructionChecker(a);
-    a = nullptr;
-    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionAsBool));
-    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionResult));
-    EXPECT_FALSE(FunctionDestructionChecker::functionAsBool.value());
-    EXPECT_EQ(0U, FunctionDestructionChecker::functionResult.value());
-    FunctionDestructionChecker::functionAsBool = std::nullopt;
-    FunctionDestructionChecker::functionResult = std::nullopt;
-
-    a = FunctionDestructionChecker(a);
     a = MoveOnly { 2 };
     EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionAsBool));
     EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionResult));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to