WTF::Function does not allow for reference / non-default constructible return types
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Aug 2017 16:56:37 +0000 (16:56 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Aug 2017 16:56:37 +0000 (16:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175244
Source/JavaScriptCore:

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@220601 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ArrayBuffer.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/Function.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSCustomElementInterface.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/Function.cpp

index cd5c210900abe15a66b6ef431cf0b1a22a6ad4dc..245fee6076803a3e5d0bbcc47dac3ef24a2667fb 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-10  Sam Weinig  <sam@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.lam@apple.com>
 
         Gardening: fix CLoop build.
index ccf988e2a06061bde4e6bf4adeaae16fdee37465..8d82026a90eedd1404a94693f121fe0cea144fed 100644 (file)
@@ -132,7 +132,7 @@ void ArrayBufferContents::transferTo(ArrayBufferContents& other)
     other.m_sizeInBytes = m_sizeInBytes;
     other.m_destructor = WTFMove(m_destructor);
     other.m_shared = m_shared;
-    clear();
+    reset();
 }
 
 void ArrayBufferContents::copyTo(ArrayBufferContents& other)
index cb685e06391bf337a0da51aab601e03ec74fd906..7fe931de2370460acb785ee99c237e8489fd958d 100644 (file)
@@ -1,3 +1,25 @@
+2017-08-10  Sam Weinig  <sam@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.lam@apple.com>
 
         Make the MASM_PROBE mechanism mandatory for DFG and FTL builds.
index bd5b05c4a7e29b35289facbc57a8801d0b40e9d4..22789afd85bcecb7ecaea4fcf1477a00e2541dd2 100644 (file)
@@ -52,9 +52,8 @@ public:
 
     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; }
index 69c1020f1a7b46d6d16399b3f69a5cc4a2b7daf8..7fe16f5431ee46da340875b95d939a55dde7d700 100644 (file)
@@ -1,3 +1,16 @@
+2017-08-10  Sam Weinig  <sam@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  <antti@apple.com>
 
         Remove RenderQuote collection from RenderView
index ec84590bf1e454d946889ff942dbb5b0abdfea65..f06f322fd95ced0ab118cca30be38fbd94dcce01 100644 (file)
@@ -94,7 +94,7 @@ private:
 
     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;
index b0b98d76dae9db0e4cc72c7477fecd9de9166a31..502132c0808e63e6f13d70ebe0363d8d96add605 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-10  Sam Weinig  <sam@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  <aperez@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'
index afe2949641fd2579c55affbe60973de3f9d12082..4e50fbfda80619066feebf11f286d6f93459f0cd 100644 (file)
@@ -60,7 +60,7 @@ enum class ShouldClearFirst;
 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)));
     }
index dfd4d24eafca779b0b2d9f949d0b496468e25941..df0d01934529b557e67aebfeabf98b6fef02bcbd 100644 (file)
@@ -1,3 +1,14 @@
+2017-08-11  Sam Weinig  <sam@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  <cgarcia@igalia.com>
 
         [Soup] Cannot access HTTPS sites using a HTTP proxy that requires authentication
index 87321390e19d2ee2515b2635492ed15466b2dae3..79357c53b8921dfbb8fdd52d8bf20f2ffe14e40d 100644 (file)
@@ -141,7 +141,6 @@ TEST(WTF_Function, Basics)
 {
     Function<unsigned()> a;
     EXPECT_FALSE(static_cast<bool>(a));
-    EXPECT_EQ(0U, a());
 
     a = [] {
         return 1U;
@@ -151,7 +150,6 @@ TEST(WTF_Function, Basics)
 
     a = nullptr;
     EXPECT_FALSE(static_cast<bool>(a));
-    EXPECT_EQ(0U, a());
 
     a = MoveOnly { 2 };
     EXPECT_TRUE(static_cast<bool>(a));
@@ -161,24 +159,17 @@ TEST(WTF_Function, Basics)
     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);
     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 {
@@ -221,15 +212,6 @@ TEST(WTF_Function, AssignBeforeDestroy)
     FunctionDestructionChecker::functionAsBool = std::nullopt;
     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));