[webkit-changes] [266713] trunk

Mon, 07 Sep 2020 20:53:09 -0700

Title: [266713] trunk
Revision
266713
Author
[email protected]
Date
2020-09-07 20:50:14 -0700 (Mon, 07 Sep 2020)

Log Message

Make CompactUniquePtrTuple actually work with subclassing and custom deleter
https://bugs.webkit.org/show_bug.cgi?id=216225

Reviewed by Darin Adler.

Source/WTF:

Fixed bugs in CompactUniquePtrTuple which prevented subclassing and custom deleter to work.

* wtf/CompactPointerTuple.h:
(WTF::CompactPointerTuple::CompactPointerTuple): Added move constructor with implicit cast
of a convertible pointer type.
* wtf/CompactUniquePtrTuple.h:
(WTF::makeCompactUniquePtr): Added the missing deleter from the return type.
(WTF::CompactUniquePtrTuple::CompactUniquePtrTuple): Allow Deleter to be different if it's
the default deleter in the move constructor so that CompactUniquePtrTuple<U, Type> could be
moved to CompactUniquePtrTuple<T, Type> if U is convertible to T without having to specify
the same deleter (std::default_delete<U> is not same as std::default_delete<T> but allow it).
(WTF::CompactUniquePtrTuple::operator=): Ditto.
(WTF::CompactUniquePtrTuple::setPointer): Ditto from std::unique_ptr.
(WTF::CompactUniquePtrTuple): Friend declare all other specializations of CompactUniquePtrTuple
so that the above fixes work.

Tools:

* TestWebKitAPI/Tests/WTF/CompactUniquePtrTuple.cpp:
(TestWebKitAPI::A::~A): Make this virtual.
(TestWebKitAPI::B): Added.
(TestWebKitAPI::B::B): Added.
(TestWebKitAPI::B::~B): Added.
(WTF_CompactUniquePtrTuple.Subclassing): Added. Tests subclassing.
(TestWebKitAPI::ADeleter): Added.
(TestWebKitAPI::ADeleter::operator() const):
(WTF_CompactUniquePtrTuple.Deleter): Added. Tests a custom deleter.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (266712 => 266713)


--- trunk/Source/WTF/ChangeLog	2020-09-08 01:35:14 UTC (rev 266712)
+++ trunk/Source/WTF/ChangeLog	2020-09-08 03:50:14 UTC (rev 266713)
@@ -1,3 +1,26 @@
+2020-09-06  Ryosuke Niwa  <[email protected]>
+
+        Make CompactUniquePtrTuple actually work with subclassing and custom deleter
+        https://bugs.webkit.org/show_bug.cgi?id=216225
+
+        Reviewed by Darin Adler.
+
+        Fixed bugs in CompactUniquePtrTuple which prevented subclassing and custom deleter to work.
+
+        * wtf/CompactPointerTuple.h:
+        (WTF::CompactPointerTuple::CompactPointerTuple): Added move constructor with implicit cast
+        of a convertible pointer type.
+        * wtf/CompactUniquePtrTuple.h:
+        (WTF::makeCompactUniquePtr): Added the missing deleter from the return type.
+        (WTF::CompactUniquePtrTuple::CompactUniquePtrTuple): Allow Deleter to be different if it's
+        the default deleter in the move constructor so that CompactUniquePtrTuple<U, Type> could be
+        moved to CompactUniquePtrTuple<T, Type> if U is convertible to T without having to specify
+        the same deleter (std::default_delete<U> is not same as std::default_delete<T> but allow it).
+        (WTF::CompactUniquePtrTuple::operator=): Ditto.
+        (WTF::CompactUniquePtrTuple::setPointer): Ditto from std::unique_ptr.
+        (WTF::CompactUniquePtrTuple): Friend declare all other specializations of CompactUniquePtrTuple
+        so that the above fixes work.
+
 2020-09-06  Myles C. Maxfield  <[email protected]>
 
         [iOS] attachmentActionFont() Needs to use kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized) to get the short emphasized footnote font

Modified: trunk/Source/WTF/wtf/CompactPointerTuple.h (266712 => 266713)


--- trunk/Source/WTF/wtf/CompactPointerTuple.h	2020-09-08 01:35:14 UTC (rev 266712)
+++ trunk/Source/WTF/wtf/CompactPointerTuple.h	2020-09-08 03:50:14 UTC (rev 266713)
@@ -68,6 +68,12 @@
         ASSERT(this->pointer() == pointer);
     }
 
+    template<typename OtherPointerType, typename = std::enable_if<std::is_pointer<PointerType>::value && std::is_convertible<OtherPointerType, PointerType>::value>>
+    CompactPointerTuple(CompactPointerTuple<OtherPointerType, Type>&& other)
+        : m_data { std::exchange(other.m_data, { }) }
+    {
+    }
+
     PointerType pointer() const { return bitwise_cast<PointerType>(m_data & pointerMask); }
     void setPointer(PointerType pointer)
     {
@@ -108,6 +114,13 @@
     {
     }
 
+    template<typename OtherPointerType, typename = std::enable_if<std::is_pointer<PointerType>::value && std::is_convertible<OtherPointerType, PointerType>::value>>
+    CompactPointerTuple(CompactPointerTuple<OtherPointerType, Type>&& other)
+        : m_pointer { std::exchange(other.m_pointer, { }) }
+        , m_type { std::exchange(other.m_type, { }) }
+    {
+    }
+
     PointerType pointer() const { return m_pointer; }
     void setPointer(PointerType pointer) { m_pointer = pointer; }
     Type type() const { return m_type; }
@@ -117,6 +130,8 @@
     PointerType m_pointer { nullptr };
     Type m_type { 0 };
 #endif
+
+    template<typename, typename> friend class CompactPointerTuple;
 };
 
 } // namespace WTF

Modified: trunk/Source/WTF/wtf/CompactUniquePtrTuple.h (266712 => 266713)


--- trunk/Source/WTF/wtf/CompactUniquePtrTuple.h	2020-09-08 01:35:14 UTC (rev 266712)
+++ trunk/Source/WTF/wtf/CompactUniquePtrTuple.h	2020-09-08 03:50:14 UTC (rev 266713)
@@ -40,7 +40,7 @@
 }
 
 template<typename T, typename Type, typename Deleter, typename... Args>
-ALWAYS_INLINE CompactUniquePtrTuple<T, Type> makeCompactUniquePtr(Args&&... args)
+ALWAYS_INLINE CompactUniquePtrTuple<T, Type, Deleter> makeCompactUniquePtr(Args&&... args)
 {
     return CompactUniquePtrTuple<T, Type, Deleter>(makeUnique<T>(std::forward<Args>(args)...));
 }
@@ -52,8 +52,8 @@
 public:
     CompactUniquePtrTuple() = default;
 
-    template <typename U>
-    CompactUniquePtrTuple(CompactUniquePtrTuple<U, Type, Deleter>&& other)
+    template <typename U, typename UDeleter, typename = std::enable_if_t<std::is_same<UDeleter, Deleter>::value || std::is_same<UDeleter, std::default_delete<U>>::value>>
+    CompactUniquePtrTuple(CompactUniquePtrTuple<U, Type, UDeleter>&& other)
         : m_data { std::exchange(other.m_data, { }) }
     {
     }
@@ -63,10 +63,10 @@
         setPointer(nullptr);
     }
 
-    template <typename U>
-    CompactUniquePtrTuple<T, Type, Deleter>& operator=(CompactUniquePtrTuple<U, Type, Deleter>&& other)
+    template <typename U, typename UDeleter, typename = std::enable_if_t<std::is_same<UDeleter, Deleter>::value || std::is_same<UDeleter, std::default_delete<U>>::value>>
+    CompactUniquePtrTuple<T, Type, Deleter>& operator=(CompactUniquePtrTuple<U, Type, UDeleter>&& other)
     {
-        auto moved = WTFMove(other);
+        CompactUniquePtrTuple moved { WTFMove(other) };
         std::swap(m_data, moved.m_data);
         return *this;
     }
@@ -86,8 +86,8 @@
         m_data.setPointer(nullptr);
     }
 
-    template <typename U>
-    void setPointer(std::unique_ptr<U, Deleter>&& pointer)
+    template <typename U, typename UDeleter, typename = std::enable_if_t<std::is_same<UDeleter, Deleter>::value || std::is_same<UDeleter, std::default_delete<U>>::value>>
+    void setPointer(std::unique_ptr<U, UDeleter>&& pointer)
     {
         deletePointer();
         m_data.setPointer(pointer.release());
@@ -115,6 +115,8 @@
     template<typename U, typename E, typename... Args> friend CompactUniquePtrTuple<U, E> makeCompactUniquePtr(Args&&... args);
     template<typename U, typename E, typename D, typename... Args> friend CompactUniquePtrTuple<U, E, D> makeCompactUniquePtr(Args&&... args);
 
+    template <typename, typename, typename> friend class CompactUniquePtrTuple;
+
     CompactPointerTuple<T*, Type> m_data;
 };

Modified: trunk/Tools/ChangeLog (266712 => 266713)


--- trunk/Tools/ChangeLog	2020-09-08 01:35:14 UTC (rev 266712)
+++ trunk/Tools/ChangeLog	2020-09-08 03:50:14 UTC (rev 266713)
@@ -1,3 +1,20 @@
+2020-09-06  Ryosuke Niwa  <[email protected]>
+
+        Make CompactUniquePtrTuple actually work with subclassing and custom deleter
+        https://bugs.webkit.org/show_bug.cgi?id=216225
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WTF/CompactUniquePtrTuple.cpp:
+        (TestWebKitAPI::A::~A): Make this virtual.
+        (TestWebKitAPI::B): Added.
+        (TestWebKitAPI::B::B): Added.
+        (TestWebKitAPI::B::~B): Added.
+        (WTF_CompactUniquePtrTuple.Subclassing): Added. Tests subclassing.
+        (TestWebKitAPI::ADeleter): Added.
+        (TestWebKitAPI::ADeleter::operator() const):
+        (WTF_CompactUniquePtrTuple.Deleter): Added. Tests a custom deleter.
+
 2020-09-07  Sam Weinig  <[email protected]>
 
         [WebIDL] Fix issues found by preprocess-idls.pl parser validation and enabled parser validation by default for the tests

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/CompactUniquePtrTuple.cpp (266712 => 266713)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/CompactUniquePtrTuple.cpp	2020-09-08 01:35:14 UTC (rev 266712)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/CompactUniquePtrTuple.cpp	2020-09-08 03:50:14 UTC (rev 266713)
@@ -38,8 +38,8 @@
 
 public:
     A() { ++s_constructorCallCount; }
-    ~A() { ++s_destructorCallCount; }
-    
+    virtual ~A() { ++s_destructorCallCount; }
+
     static unsigned s_constructorCallCount;
     static unsigned s_destructorCallCount;
 };
@@ -149,4 +149,177 @@
     EXPECT_EQ(0xf2f2U, b.type());
 }
 
+class B : public A {
+    WTF_MAKE_FAST_ALLOCATED;
+
+public:
+    B() { ++s_constructorCallCount; }
+    virtual ~B() { ++s_destructorCallCount; }
+
+    static unsigned s_constructorCallCount;
+    static unsigned s_destructorCallCount;
+};
+
+unsigned B::s_constructorCallCount = 0;
+unsigned B::s_destructorCallCount = 0;
+
+TEST(WTF_CompactUniquePtrTuple, Subclassing)
+{
+    A::s_constructorCallCount = 0;
+    A::s_destructorCallCount = 0;
+    B::s_constructorCallCount = 0;
+    B::s_destructorCallCount = 0;
+
+    CompactUniquePtrTuple<B, uint16_t> empty;
+    EXPECT_EQ(0U, A::s_constructorCallCount);
+    EXPECT_EQ(0U, A::s_destructorCallCount);
+    EXPECT_EQ(0U, B::s_constructorCallCount);
+    EXPECT_EQ(0U, B::s_destructorCallCount);
+    EXPECT_EQ(nullptr, empty.pointer());
+    EXPECT_EQ(0U, empty.type());
+
+    CompactUniquePtrTuple<B, uint16_t> b = makeCompactUniquePtr<B, uint16_t>();
+    EXPECT_EQ(1U, A::s_constructorCallCount);
+    EXPECT_EQ(0U, A::s_destructorCallCount);
+    EXPECT_EQ(1U, B::s_constructorCallCount);
+    EXPECT_EQ(0U, B::s_destructorCallCount);
+    EXPECT_NE(nullptr, b.pointer());
+    EXPECT_EQ(0U, b.type());
+
+    b.setType(0xff);
+    EXPECT_EQ(1U, A::s_constructorCallCount);
+    EXPECT_EQ(0U, A::s_destructorCallCount);
+    EXPECT_EQ(1U, B::s_constructorCallCount);
+    EXPECT_EQ(0U, B::s_destructorCallCount);
+    EXPECT_NE(nullptr, b.pointer());
+    EXPECT_EQ(0xffU, b.type());
+
+    b.setPointer(nullptr);
+    EXPECT_EQ(1U, A::s_constructorCallCount);
+    EXPECT_EQ(1U, A::s_destructorCallCount);
+    EXPECT_EQ(1U, B::s_constructorCallCount);
+    EXPECT_EQ(1U, B::s_destructorCallCount);
+    EXPECT_EQ(nullptr, b.pointer());
+    EXPECT_EQ(0xffU, b.type());
+
+    CompactUniquePtrTuple<A, uint16_t> a = makeCompactUniquePtr<B, uint16_t>();
+    EXPECT_EQ(2U, A::s_constructorCallCount);
+    EXPECT_EQ(1U, A::s_destructorCallCount);
+    EXPECT_EQ(2U, B::s_constructorCallCount);
+    EXPECT_EQ(1U, B::s_destructorCallCount);
+    EXPECT_NE(nullptr, a.pointer());
+    EXPECT_EQ(0U, a.type());
+
+    a.setType(0x1c);
+    EXPECT_EQ(2U, A::s_constructorCallCount);
+    EXPECT_EQ(1U, A::s_destructorCallCount);
+    EXPECT_EQ(2U, B::s_constructorCallCount);
+    EXPECT_EQ(1U, B::s_destructorCallCount);
+    EXPECT_NE(nullptr, a.pointer());
+    EXPECT_EQ(0x1cU, a.type());
+
+    auto* oldPointer = a.pointer();
+    a.setPointer(makeUnique<B>());
+    EXPECT_EQ(3U, A::s_constructorCallCount);
+    EXPECT_EQ(2U, A::s_destructorCallCount);
+    EXPECT_EQ(3U, B::s_constructorCallCount);
+    EXPECT_EQ(2U, B::s_destructorCallCount);
+    EXPECT_NE(oldPointer, a.pointer());
+    EXPECT_EQ(0x1cU, a.type());
+
+    a = makeCompactUniquePtr<A, uint16_t>();
+    EXPECT_EQ(4U, A::s_constructorCallCount);
+    EXPECT_EQ(3U, A::s_destructorCallCount);
+    EXPECT_EQ(3U, B::s_constructorCallCount);
+    EXPECT_EQ(3U, B::s_destructorCallCount);
+    EXPECT_NE(oldPointer, a.pointer());
+    EXPECT_EQ(0U, a.type());
+
+    a.setType(0xfed);
+    EXPECT_EQ(4U, A::s_constructorCallCount);
+    EXPECT_EQ(3U, A::s_destructorCallCount);
+    EXPECT_EQ(3U, B::s_constructorCallCount);
+    EXPECT_EQ(3U, B::s_destructorCallCount);
+    EXPECT_NE(nullptr, a.pointer());
+    EXPECT_EQ(0xfedU, a.type());
+
+    oldPointer = a.pointer();
+    auto uniqueA = a.moveToUniquePtr();
+    EXPECT_EQ(4U, A::s_constructorCallCount);
+    EXPECT_EQ(3U, A::s_destructorCallCount);
+    EXPECT_EQ(3U, B::s_constructorCallCount);
+    EXPECT_EQ(3U, B::s_destructorCallCount);
+    EXPECT_EQ(nullptr, a.pointer());
+    EXPECT_EQ(oldPointer, uniqueA.get());
+    EXPECT_EQ(0xfedU, a.type());
+    
+    uniqueA = nullptr;
+    EXPECT_EQ(4U, A::s_constructorCallCount);
+    EXPECT_EQ(4U, A::s_destructorCallCount);
+    EXPECT_EQ(3U, B::s_constructorCallCount);
+    EXPECT_EQ(3U, B::s_destructorCallCount);
+    EXPECT_EQ(nullptr, a.pointer());
+    EXPECT_EQ(nullptr, uniqueA.get());
+    EXPECT_EQ(0xfedU, a.type());
+}
+
+struct ADeleter {
+    static unsigned s_callCount;
+
+    void operator()(A* a) const
+    {
+        ++s_callCount;
+        delete a;
+    }
+};
+
+unsigned ADeleter::s_callCount = 0;
+
+TEST(WTF_CompactUniquePtrTuple, Deleter)
+{
+    A::s_constructorCallCount = 0;
+    A::s_destructorCallCount = 0;
+    B::s_constructorCallCount = 0;
+    B::s_destructorCallCount = 0;
+    ADeleter::s_callCount = 0;
+
+    CompactUniquePtrTuple<B, uint16_t, ADeleter> empty;
+    EXPECT_EQ(0U, A::s_constructorCallCount);
+    EXPECT_EQ(0U, A::s_destructorCallCount);
+    EXPECT_EQ(0U, B::s_constructorCallCount);
+    EXPECT_EQ(0U, B::s_destructorCallCount);
+    EXPECT_EQ(0U, ADeleter::s_callCount);
+    EXPECT_EQ(nullptr, empty.pointer());
+    EXPECT_EQ(0U, empty.type());
+
+    CompactUniquePtrTuple<A, uint16_t, ADeleter> a = makeCompactUniquePtr<A, uint16_t, ADeleter>();
+    EXPECT_EQ(1U, A::s_constructorCallCount);
+    EXPECT_EQ(0U, A::s_destructorCallCount);
+    EXPECT_EQ(0U, B::s_constructorCallCount);
+    EXPECT_EQ(0U, B::s_destructorCallCount);
+    EXPECT_EQ(0U, ADeleter::s_callCount);
+    EXPECT_NE(nullptr, a.pointer());
+    EXPECT_EQ(0U, a.type());
+
+    a.setType(0x7e89U);
+    EXPECT_EQ(1U, A::s_constructorCallCount);
+    EXPECT_EQ(0U, A::s_destructorCallCount);
+    EXPECT_EQ(0U, B::s_constructorCallCount);
+    EXPECT_EQ(0U, B::s_destructorCallCount);
+    EXPECT_EQ(0U, ADeleter::s_callCount);
+    EXPECT_NE(nullptr, a.pointer());
+    EXPECT_EQ(0x7e89U, a.type());
+
+    auto* oldPointer = a.pointer();
+    a.setPointer(std::unique_ptr<B, ADeleter>(new B));
+    EXPECT_EQ(2U, A::s_constructorCallCount);
+    EXPECT_EQ(1U, A::s_destructorCallCount);
+    EXPECT_EQ(1U, B::s_constructorCallCount);
+    EXPECT_EQ(0U, B::s_destructorCallCount);
+    EXPECT_EQ(1U, ADeleter::s_callCount);
+    EXPECT_NE(oldPointer, a.pointer());
+    EXPECT_EQ(0x7e89U, a.type());
+
+}
+
 } // namespace TestWebKitAPI

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to