Changeset 242778 in webkit

Timestamp:

Mar 11, 2019, 11:04:22 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

WTF::Expected should use std::addressof instead of operator&
​https://bugs.webkit.org/show_bug.cgi?id=195604

Patch by Alex Christensen <​achristensen@webkit.org> on 2019-03-11
Reviewed by Myles Maxfield.

Source/WTF:

The latter was causing problems with types that do tricky things with constructors and operator&,
specifically UniqueRef but I made a reduced test case. When it used operator&, it would get the contained
type and call the constructor that takes a contained type instead of the move constructor.

• wtf/Expected.h:

(std::experimental::fundamentals_v3::expected_detail::base::base):
(std::experimental::fundamentals_v3::expected::swap):

Tools:

• TestWebKitAPI/Tests/WTF/Expected.cpp:

(TestWebKitAPI::Unique::Unique):
(TestWebKitAPI::Unique::operator&):
(TestWebKitAPI::TEST):

Location:

trunk

Files:

• Source/WTF/ChangeLog (modified) (1 diff)
• Source/WTF/wtf/Expected.h (modified) (6 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WTF/Expected.cpp (modified) (1 diff)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2019-03-11  Alex Christensen  <achristensen@webkit.org>
+
+        WTF::Expected should use std::addressof instead of operator&
+        https://bugs.webkit.org/show_bug.cgi?id=195604
+
+        Reviewed by Myles Maxfield.
+
+        The latter was causing problems with types that do tricky things with constructors and operator&,
+        specifically UniqueRef but I made a reduced test case.  When it used operator&, it would get the contained
+        type and call the constructor that takes a contained type instead of the move constructor.
+
+        * wtf/Expected.h:
+        (std::experimental::fundamentals_v3::__expected_detail::base::base):
+        (std::experimental::fundamentals_v3::expected::swap):
+
 2019-03-11  Ross Kirsling  <ross.kirsling@sony.com>
 

trunk/Source/WTF/wtf/Expected.h

@@ -335 +335 @@
     {
         if (has)
-            ::new (&s.val) value_type(o.s.val);
+            ::new (std::addressof(s.val)) value_type(o.s.val);
         else
-            ::new (&s.err) error_type(o.s.err);
+            ::new (std::addressof(s.err)) error_type(o.s.err);
     }
     base(base&& o)
@@ -343 +343 @@
     {
         if (has)
-            ::new (&s.val) value_type(std::move(o.s.val));
+            ::new (std::addressof(s.val)) value_type(std::move(o.s.val));
         else
-            ::new (&s.err) error_type(std::move(o.s.err));
+            ::new (std::addressof(s.err)) error_type(std::move(o.s.err));
     }
     ~base()
@@ -387 +387 @@
     {
         if (!has)
-            ::new (&s.err) error_type(o.s.err);
+            ::new (std::addressof(s.err)) error_type(o.s.err);
     }
     base(base&& o)
@@ -393 +393 @@
     {
         if (!has)
-            ::new (&s.err) error_type(std::move(o.s.err));
+            ::new (std::addressof(s.err)) error_type(std::move(o.s.err));
     }
     ~base()
@@ -462 +462 @@
             error_type e(std::move(o.s.err));
             __expected_detail::destroy(o.s.err);
-            ::new (&o.s.val) value_type(std::move(base::s.val));
+            ::new (std::addressof(o.s.val)) value_type(std::move(base::s.val));
             __expected_detail::destroy(base::s.val);
-            ::new (&base::s.err) error_type(std::move(e));
+            ::new (std::addressof(base::s.err)) error_type(std::move(e));
             swap(base::has, o.has);
         } else if (!base::has && o.has) {
             value_type v(std::move(o.s.val));
             __expected_detail::destroy(o.s.val);
-            ::new (&o.s.err) error_type(std::move(base::s.err));
+            ::new (std::addressof(o.s.err)) error_type(std::move(base::s.err));
             __expected_detail::destroy(base::s.err);
-            ::new (&base::s.val) value_type(std::move(v));
+            ::new (std::addressof(base::s.val)) value_type(std::move(v));
             swap(base::has, o.has);
         } else
@@ -537 +537 @@
         } else if (base::has && !o.has) {
             error_type e(std::move(o.s.err));
-            ::new (&base::s.err) error_type(e);
+            ::new (std::addressof(base::s.err)) error_type(e);
             swap(base::has, o.has);
         } else if (!base::has && o.has) {
-            ::new (&o.s.err) error_type(std::move(base::s.err));
+            ::new (std::addressof(o.s.err)) error_type(std::move(base::s.err));
             swap(base::has, o.has);
         } else

trunk/Tools/ChangeLog

@@ +1 @@
+2019-03-11  Alex Christensen  <achristensen@webkit.org>
+
+        WTF::Expected should use std::addressof instead of operator&
+        https://bugs.webkit.org/show_bug.cgi?id=195604
+
+        Reviewed by Myles Maxfield.
+
+        * TestWebKitAPI/Tests/WTF/Expected.cpp:
+        (TestWebKitAPI::Unique::Unique):
+        (TestWebKitAPI::Unique::operator&):
+        (TestWebKitAPI::TEST):
+
 2019-03-11  Ross Kirsling  <ross.kirsling@sony.com>
 

trunk/Tools/TestWebKitAPI/Tests/WTF/Expected.cpp

@@ -473 +473 @@
 }
 
+class NeedsStdAddress {
+public:
+    NeedsStdAddress(NeedsStdAddress&& other)
+        : m_ptr(WTFMove(other.m_ptr)) { }
+    NeedsStdAddress(int& other)
+        : m_ptr(&other) { }
+    int* operator&() { ASSERT_NOT_REACHED(); return nullptr; }
+private:
+    std::unique_ptr<int> m_ptr;
+};
+
+TEST(WTF_Expected, Address)
+{
+    NeedsStdAddress a(*new int(3));
+    Expected<NeedsStdAddress, float> b(WTFMove(a));
+    Expected<NeedsStdAddress, float> c(WTFMove(b));
+    (void)c;
+}
+
 } // namespace TestWebkitAPI