Title: [271358] trunk/Source
Revision
271358
Author
[email protected]
Date
2021-01-11 05:04:27 -0800 (Mon, 11 Jan 2021)

Log Message

WebKit::IPC::Encoder needs definitions of all custom enum values at the Encoder definition time
https://bugs.webkit.org/show_bug.cgi?id=220410

Patch by Kimmo Kinnunen <[email protected]> on 2021-01-11
Reviewed by Darin Adler.

Change WTF::EnumTraits to have either EnumTraits::values or EnumTraits::isValidEnum().
Source/WebCore:

Change here needed to conform to the new protocol. Fixes an issue with the template usage
in WebKit::IPC::Encoder, see WebKit and WTF ChangeLogs for discussion.

* platform/ContextMenuItem.h:
(WTF::EnumTraits<WebCore::ContextMenuAction>::isValidEnum):

Source/WebKit:

Fixes the upcoming problem where Encoder::encode(E) function is moved from Encoder
to other class, such as EncoderBase, and where we do not want to add redundant includes.
See the WTF/ ChangeLog for discussiong wrt why they were needed before.

* Scripts/webkit/messages.py:
* Scripts/webkit/tests/MessageNames.h:
(WTF::EnumTraits<IPC::MessageName>::isValidEnum):

Source/WTF:

Previously, if clients wanted to check isValidEnum for custom enum, they had to:
 1) Define HasCustomIsValidEnum<TheirEnum> : true_type {};
 2) Define new function template isValidEnum() for TheirEnum
 3) Ensure that their isValidEnum() was defined before the definition of the call to
    the isValidEnum().

This has the problem that isValidEnum() cannot be called in generic code, because
at the definition time generic code typically does not have all the types in scope
that will be used at instantiation time.
Each isValidEnum() is their own function template, C++ does not have function template
specialization.

After the change, clients need to:
 1) Define EnumTraits<TheirEnum> { bool isValidEnum(...) }

Fix by using the fact that WTF::EnumTraits is a class and that can have template
specializations.

Consistent with the EnumTraits::values case:
  a) Automatic case?     --> Define EnumTraits::values
  b) Manual/custom case? --> Define EnumTraits::isValidEnum()

* wtf/EnumTraits.h:
(WTF::isValidEnum):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (271357 => 271358)


--- trunk/Source/WTF/ChangeLog	2021-01-11 11:44:41 UTC (rev 271357)
+++ trunk/Source/WTF/ChangeLog	2021-01-11 13:04:27 UTC (rev 271358)
@@ -1,3 +1,36 @@
+2021-01-11  Kimmo Kinnunen  <[email protected]>
+
+        WebKit::IPC::Encoder needs definitions of all custom enum values at the Encoder definition time
+        https://bugs.webkit.org/show_bug.cgi?id=220410
+
+        Reviewed by Darin Adler.
+
+        Change WTF::EnumTraits to have either EnumTraits::values or EnumTraits::isValidEnum().
+        Previously, if clients wanted to check isValidEnum for custom enum, they had to:
+         1) Define HasCustomIsValidEnum<TheirEnum> : true_type {};
+         2) Define new function template isValidEnum() for TheirEnum
+         3) Ensure that their isValidEnum() was defined before the definition of the call to
+            the isValidEnum().
+
+        This has the problem that isValidEnum() cannot be called in generic code, because
+        at the definition time generic code typically does not have all the types in scope
+        that will be used at instantiation time.
+        Each isValidEnum() is their own function template, C++ does not have function template
+        specialization.
+
+        After the change, clients need to:
+         1) Define EnumTraits<TheirEnum> { bool isValidEnum(...) }
+
+        Fix by using the fact that WTF::EnumTraits is a class and that can have template
+        specializations.
+
+        Consistent with the EnumTraits::values case:
+          a) Automatic case?     --> Define EnumTraits::values
+          b) Manual/custom case? --> Define EnumTraits::isValidEnum()
+
+        * wtf/EnumTraits.h:
+        (WTF::isValidEnum):
+
 2021-01-08  Yusuke Suzuki  <[email protected]>
 
         [JSC] Disable JITCage compile time in old iOS

Modified: trunk/Source/WTF/wtf/EnumTraits.h (271357 => 271358)


--- trunk/Source/WTF/wtf/EnumTraits.h	2021-01-11 11:44:41 UTC (rev 271357)
+++ trunk/Source/WTF/wtf/EnumTraits.h	2021-01-11 13:04:27 UTC (rev 271358)
@@ -51,10 +51,8 @@
     }
 };
 
-template<typename U> class HasCustomIsValidEnum : public std::false_type { };
-
-template<typename E, typename T, std::enable_if_t<std::is_enum<E>::value && !std::is_same<std::underlying_type_t<E>, bool>::value && !HasCustomIsValidEnum<E>::value>* = nullptr>
-constexpr bool isValidEnum(T t)
+template<typename E, typename T, typename = std::enable_if_t<!std::is_same_v<std::underlying_type_t<E>, bool>>>
+constexpr auto isValidEnum(T t) -> decltype(EnumValueChecker<T, typename EnumTraits<E>::values>::isValidEnum(t), bool())
 {
     static_assert(sizeof(T) >= sizeof(std::underlying_type_t<E>), "Integral type must be at least the size of the underlying enum type");
 
@@ -61,7 +59,13 @@
     return EnumValueChecker<T, typename EnumTraits<E>::values>::isValidEnum(t);
 }
 
-template<typename E, typename T, std::enable_if_t<std::is_same<std::underlying_type_t<E>, bool>::value>* = nullptr>
+template<typename E, typename T, typename = std::enable_if_t<!std::is_same_v<std::underlying_type_t<E>, bool>>>
+auto isValidEnum(T t) -> decltype(EnumTraits<E>::isValidEnum(t), bool())
+{
+    return EnumTraits<E>::isValidEnum(t);
+}
+
+template<typename E, typename T, typename = std::enable_if_t<std::is_same_v<std::underlying_type_t<E>, bool>>>
 constexpr bool isValidEnum(T t)
 {
     return !t || t == 1;

Modified: trunk/Source/WebCore/ChangeLog (271357 => 271358)


--- trunk/Source/WebCore/ChangeLog	2021-01-11 11:44:41 UTC (rev 271357)
+++ trunk/Source/WebCore/ChangeLog	2021-01-11 13:04:27 UTC (rev 271358)
@@ -1,3 +1,18 @@
+2021-01-11  Kimmo Kinnunen  <[email protected]>
+
+        WebKit::IPC::Encoder needs definitions of all custom enum values at the Encoder definition time
+        https://bugs.webkit.org/show_bug.cgi?id=220410
+
+        Reviewed by Darin Adler.
+
+        Change WTF::EnumTraits to have either EnumTraits::values or EnumTraits::isValidEnum().
+
+        Change here needed to conform to the new protocol. Fixes an issue with the template usage
+        in WebKit::IPC::Encoder, see WebKit and WTF ChangeLogs for discussion.
+
+        * platform/ContextMenuItem.h:
+        (WTF::EnumTraits<WebCore::ContextMenuAction>::isValidEnum):
+
 2021-01-07  Sergio Villar Senin  <[email protected]>
 
         [css-multicol] Restore placeholders on multicolumn children position changes.

Modified: trunk/Source/WebCore/platform/ContextMenuItem.h (271357 => 271358)


--- trunk/Source/WebCore/platform/ContextMenuItem.h	2021-01-11 11:44:41 UTC (rev 271357)
+++ trunk/Source/WebCore/platform/ContextMenuItem.h	2021-01-11 13:04:27 UTC (rev 271358)
@@ -208,13 +208,12 @@
 namespace WTF {
 
 template<>
-class HasCustomIsValidEnum<WebCore::ContextMenuAction> : public std::true_type { };
-
-template<typename E, typename T, std::enable_if_t<std::is_same_v<E, WebCore::ContextMenuAction>>* = nullptr>
-bool isValidEnum(T action)
-{
-    static_assert(sizeof(T) == sizeof(E), "isValidEnum<WebCore::ContextMenuAction> should only be called with 32-bit types");
-    return WebCore::isValidContextMenuAction(static_cast<E>(action));
+struct EnumTraits<WebCore::ContextMenuAction> {
+    template<typename T>
+    static std::enable_if_t<sizeof(T) == sizeof(WebCore::ContextMenuAction), bool> isValidEnum(T action)
+    {
+        return WebCore::isValidContextMenuAction(static_cast<WebCore::ContextMenuAction>(action));
+    };
 };
 
 template<> struct EnumTraits<WebCore::ContextMenuItemType> {

Modified: trunk/Source/WebKit/ChangeLog (271357 => 271358)


--- trunk/Source/WebKit/ChangeLog	2021-01-11 11:44:41 UTC (rev 271357)
+++ trunk/Source/WebKit/ChangeLog	2021-01-11 13:04:27 UTC (rev 271358)
@@ -1,3 +1,20 @@
+2021-01-11  Kimmo Kinnunen  <[email protected]>
+
+        WebKit::IPC::Encoder needs definitions of all custom enum values at the Encoder definition time
+        https://bugs.webkit.org/show_bug.cgi?id=220410
+
+        Reviewed by Darin Adler.
+
+        Change WTF::EnumTraits to have either EnumTraits::values or EnumTraits::isValidEnum().
+
+        Fixes the upcoming problem where Encoder::encode(E) function is moved from Encoder
+        to other class, such as EncoderBase, and where we do not want to add redundant includes.
+        See the WTF/ ChangeLog for discussiong wrt why they were needed before.
+
+        * Scripts/webkit/messages.py:
+        * Scripts/webkit/tests/MessageNames.h:
+        (WTF::EnumTraits<IPC::MessageName>::isValidEnum):
+
 2021-01-11  Carlos Garcia Campos  <[email protected]>
 
         [GTK] System font scaling not applied to 'font-size: XXXpt'

Modified: trunk/Source/WebKit/Scripts/webkit/messages.py (271357 => 271358)


--- trunk/Source/WebKit/Scripts/webkit/messages.py	2021-01-11 11:44:41 UTC (rev 271357)
+++ trunk/Source/WebKit/Scripts/webkit/messages.py	2021-01-11 13:04:27 UTC (rev 271358)
@@ -988,15 +988,14 @@
     result.append('namespace WTF {\n')
     result.append('\n')
     result.append('template<>\n')
-    result.append('class HasCustomIsValidEnum<IPC::MessageName> : public std::true_type { };\n')
-    result.append('template<typename E, typename T, std::enable_if_t<std::is_same_v<E, IPC::MessageName>>* = nullptr>\n')
-    result.append('bool isValidEnum(T messageName)\n')
-    result.append('{\n')
-    result.append('    static_assert(sizeof(T) == sizeof(E), "isValidEnum<IPC::MessageName> should only be called with 16-bit types");\n')
-    result.append('    static_assert(std::is_unsigned<T>::value, "isValidEnum<IPC::MessageName> should only be called with unsigned types");\n')
-    result.append('    if (messageName > static_cast<std::underlying_type<IPC::MessageName>::type>(IPC::MessageName::Last))\n')
-    result.append('        return false;\n')
-    result.append('    return IPC::isValidMessageName(static_cast<E>(messageName));\n')
+    result.append('struct EnumTraits<IPC::MessageName> {\n')
+    result.append('    template<typename T>\n')
+    result.append('    static std::enable_if_t<sizeof(T) == sizeof(IPC::MessageName) && std::is_unsigned_v<T>, bool> isValidEnum(T messageName)\n')
+    result.append('    {\n')
+    result.append('        if (messageName > WTF::enumToUnderlyingType(IPC::MessageName::Last))\n')
+    result.append('            return false;\n')
+    result.append('        return IPC::isValidMessageName(static_cast<IPC::MessageName>(messageName));\n')
+    result.append('    }\n')
     result.append('};\n')
     result.append('\n')
     result.append('} // namespace WTF\n')

Modified: trunk/Source/WebKit/Scripts/webkit/tests/MessageNames.h (271357 => 271358)


--- trunk/Source/WebKit/Scripts/webkit/tests/MessageNames.h	2021-01-11 11:44:41 UTC (rev 271357)
+++ trunk/Source/WebKit/Scripts/webkit/tests/MessageNames.h	2021-01-11 13:04:27 UTC (rev 271358)
@@ -115,15 +115,14 @@
 namespace WTF {
 
 template<>
-class HasCustomIsValidEnum<IPC::MessageName> : public std::true_type { };
-template<typename E, typename T, std::enable_if_t<std::is_same_v<E, IPC::MessageName>>* = nullptr>
-bool isValidEnum(T messageName)
-{
-    static_assert(sizeof(T) == sizeof(E), "isValidEnum<IPC::MessageName> should only be called with 16-bit types");
-    static_assert(std::is_unsigned<T>::value, "isValidEnum<IPC::MessageName> should only be called with unsigned types");
-    if (messageName > static_cast<std::underlying_type<IPC::MessageName>::type>(IPC::MessageName::Last))
-        return false;
-    return IPC::isValidMessageName(static_cast<E>(messageName));
+struct EnumTraits<IPC::MessageName> {
+    template<typename T>
+    static std::enable_if_t<sizeof(T) == sizeof(IPC::MessageName) && std::is_unsigned_v<T>, bool> isValidEnum(T messageName)
+    {
+        if (messageName > WTF::enumToUnderlyingType(IPC::MessageName::Last))
+            return false;
+        return IPC::isValidMessageName(static_cast<IPC::MessageName>(messageName));
+    }
 };
 
 } // namespace WTF
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to