Weak ref smart pointer check should be smarter

https://bugs.webkit.org/show_bug.cgi?id=273594
rdar://127395924

Reviewed by Chris Dumez.

Improved the SFINAE usage to account for const/non-const overload.

Moved the static_assert to dereference operations so that WeakPtr<T> can be
more like T* in its support for forward declaration. Now you don't need the
full type definition available in order to destroy a WeakPtr<T>; only to
dereference one.

* Source/WTF/wtf/TypeTraits.h:
* Source/WTF/wtf/WeakPtr.h:
(WTF::WeakPtr::releaseImpl):
(WTF::WeakPtr::get const):
(WTF::WeakPtr::operator-> const):
(WTF::WeakPtr::operator* const):
(WTF::WeakPtr::~WeakPtr): Deleted.
* Source/WTF/wtf/WeakRef.h:
(WTF::WeakRef::ptrAllowingHashTableEmptyValue const):
(WTF::WeakRef::ptr const):
(WTF::WeakRef::get const):
(WTF::WeakRef::~WeakRef): Deleted.

Canonical link: https://commits.webkit.org/278249@main

Author: geoffreygaren

Source/WTF/wtf/TypeTraits.h

@@ -107,7 +107,7 @@ template<typename>
 struct SFINAE1True : std::true_type { };
 
 template<class T>
-static auto HasRefPtrMethodsTest(SFINAE_OVERLOAD_PREFERRED) -> SFINAE1True<decltype(&T::ref, &T::deref)>;
+static auto HasRefPtrMethodsTest(SFINAE_OVERLOAD_PREFERRED) -> SFINAE1True<decltype(static_cast<std::remove_cv_t<T>*>(nullptr)->ref(), static_cast<std::remove_cv_t<T>*>(nullptr)->deref())>;
 template<class>
 static auto HasRefPtrMethodsTest(SFINAE_OVERLOAD_DEFAULT) -> std::false_type;
 
@@ -120,7 +120,7 @@ struct HasRefPtrMethods : decltype(detail::HasRefPtrMethodsTest<T>(SFINAE_OVERLO
 namespace detail {
 
 template<class T>
-static auto HasCheckedPtrMethodsTest(SFINAE_OVERLOAD_PREFERRED) -> SFINAE1True<decltype(&T::incrementPtrCount, &T::decrementPtrCount)>;
+static auto HasCheckedPtrMethodsTest(SFINAE_OVERLOAD_PREFERRED) -> SFINAE1True<decltype(static_cast<std::remove_cv_t<T>*>(nullptr)->incrementPtrCount(), static_cast<std::remove_cv_t<T>*>(nullptr)->decrementPtrCount())>;
 template<class>
 static auto HasCheckedPtrMethodsTest(SFINAE_OVERLOAD_DEFAULT) -> std::false_type;
 

Source/WTF/wtf/WeakPtr.h

@@ -83,17 +83,14 @@ template<typename T, typename WeakPtrImpl, typename PtrTraits> class WeakPtr {
     {
     }
 
-    ~WeakPtr()
-    {
-        static_assert(
-            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_const_t<T>>::value,
-            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr.");
-    }
-
     RefPtr<WeakPtrImpl, PtrTraits> releaseImpl() { return WTFMove(m_impl); }
 
     T* get() const
     {
+        static_assert(
+            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_cv_t<T>>::value,
+            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr. Please do not add new exceptions.");
+
         ASSERT(canSafelyBeUsed());
         return m_impl ? static_cast<T*>(m_impl->template get<T>()) : nullptr;
     }
@@ -114,12 +111,20 @@ template<typename T, typename WeakPtrImpl, typename PtrTraits> class WeakPtr {
 
     T* operator->() const
     {
+        static_assert(
+            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_cv_t<T>>::value,
+            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr. Please do not add new exceptions.");
+
         ASSERT(canSafelyBeUsed());
         return get();
     }
 
     T& operator*() const
     {
+        static_assert(
+            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_cv_t<T>>::value,
+            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr. Please do not add new exceptions.");
+
         ASSERT(canSafelyBeUsed());
         return *get();
     }

Source/WTF/wtf/WeakRef.h

@@ -67,13 +67,6 @@ class WeakRef {
     WeakRef(HashTableDeletedValueType) : m_impl(HashTableDeletedValue) { }
     WeakRef(HashTableEmptyValueType) : m_impl(HashTableEmptyValue) { }
 
-    ~WeakRef()
-    {
-        static_assert(
-            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_const_t<T>>::value,
-            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr. Please do not add new exceptions.");
-    }
-
     bool isHashTableDeletedValue() const { return m_impl.isHashTableDeletedValue(); }
     bool isHashTableEmptyValue() const { return m_impl.isHashTableEmptyValue(); }
 
@@ -82,22 +75,35 @@ class WeakRef {
 
     T* ptrAllowingHashTableEmptyValue() const
     {
+        static_assert(
+            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_cv_t<T>>::value,
+            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr. Please do not add new exceptions.");
+
         return !m_impl.isHashTableEmptyValue() ? static_cast<T*>(m_impl->template get<T>()) : nullptr;
     }
 
     T* ptr() const
     {
+        static_assert(
+            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_cv_t<T>>::value,
+            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr. Please do not add new exceptions.");
+
         auto* ptr = static_cast<T*>(m_impl->template get<T>());
         ASSERT(ptr);
         return ptr;
     }
 
     T& get() const
     {
+        static_assert(
+            HasRefPtrMethods<T>::value || HasCheckedPtrMethods<T>::value || IsDeprecatedWeakRefSmartPointerException<std::remove_cv_t<T>>::value,
+            "Classes that offer weak pointers should also offer RefPtr or CheckedPtr. Please do not add new exceptions.");
+
         auto* ptr = static_cast<T*>(m_impl->template get<T>());
         ASSERT(ptr);
         return *ptr;
     }
+
     operator T&() const { return get(); }
 
     T* operator->() const