Merge pull request #5595 from cloudflare/jasnell/eliminate-duplicate-v8-value-templates

Author: jasnell

src/workerd/jsg/jsg.h

@@ -42,7 +42,7 @@ kj::String stringifyHandle(v8::Local<v8::Value> value);
 
 namespace v8 {
 // Allows v8 handles to be passed to kj::str() as well as KJ_LOG and related macros.
-template <typename T, typename = kj::EnableIf<kj::canConvert<T*, v8::Value*>()>>
+template <workerd::jsg::V8Value T>
 kj::String KJ_STRINGIFY(v8::Local<T> value) {
   return workerd::jsg::stringifyHandle(value);
 }

src/workerd/jsg/jsvalue.h

@@ -713,18 +713,21 @@ inline kj::String KJ_STRINGIFY(const JsValue& value) {
   return value.toString(jsg::Lock::current());
 }
 
+template <typename T>
+concept JsValueType = std::is_assignable_v<JsValue, T>;
+
 template <typename TypeWrapper>
 struct JsValueWrapper {
 #define TYPES_TO_WRAP(V)                                                                           \
   V(Value)                                                                                         \
   JS_TYPE_CLASSES(V)
 
-  template <typename T, typename = kj::EnableIf<std::is_assignable_v<JsValue, T>>>
+  template <JsValueType T>
   static constexpr const std::type_info& getName(T*) {
     return typeid(T);
   }
 
-  template <typename T, typename = kj::EnableIf<std::is_assignable_v<JsValue, T>>>
+  template <JsValueType T>
   static constexpr const std::type_info& getName(JsRef<T>*) {
     return typeid(T);
   }
@@ -742,7 +745,7 @@ struct JsValueWrapper {
   TYPES_TO_WRAP(V)
 #undef V
 
-  template <typename T, typename = kj::EnableIf<std::is_assignable_v<JsValue, T>>>
+  template <JsValueType T>
   kj::Maybe<T> tryUnwrap(Lock& js,
       v8::Local<v8::Context> context,
       v8::Local<v8::Value> handle,
@@ -763,7 +766,7 @@ struct JsValueWrapper {
     }
   }
 
-  template <typename T, typename = kj::EnableIf<std::is_assignable_v<JsValue, T>>>
+  template <JsValueType T>
   kj::Maybe<JsRef<T>> tryUnwrap(Lock& js,
       v8::Local<v8::Context> context,
       v8::Local<v8::Value> handle,

src/workerd/jsg/type-wrapper.h

@@ -41,12 +41,12 @@ concept ValueLessParameter =
 // This is just a trivial pass-through.
 class V8HandleWrapper {
  public:
-  template <typename T, typename = kj::EnableIf<kj::canConvert<T, v8::Value>()>>
+  template <V8Value T>
   static constexpr const std::type_info& getName(v8::Local<T>*) {
     return typeid(T);
   }
 
-  template <typename T, typename = kj::EnableIf<kj::canConvert<T, v8::Value>()>>
+  template <V8Value T>
   v8::Local<T> wrap(jsg::Lock& js,
       v8::Local<v8::Context> context,
       kj::Maybe<v8::Local<v8::Object>> creator,
@@ -111,12 +111,12 @@ class V8HandleWrapper {
 #undef JSG_DEFINE_TRY_UNWRAP
 #undef JSG_FOR_EACH_V8_VALUE_SUBCLASS
 
-  template <typename T, typename = kj::EnableIf<kj::canConvert<T, v8::Value>()>>
+  template <V8Value T>
   static constexpr const std::type_info& getName(v8::Global<T>*) {
     return typeid(T);
   }
 
-  template <typename T, typename = kj::EnableIf<kj::canConvert<T, v8::Value>()>>
+  template <V8Value T>
   v8::Local<T> wrap(jsg::Lock& js,
       v8::Local<v8::Context> context,
       kj::Maybe<v8::Local<v8::Object>> creator,
@@ -132,12 +132,12 @@ class V8HandleWrapper {
     return v8::Global<v8::Value>(js.v8Isolate, handle);
   }
 
-  template <typename T, typename = kj::EnableIf<kj::canConvert<T, v8::Value>()>>
+  template <V8Value T>
   static constexpr const std::type_info& getName(V8Ref<T>*) {
     return typeid(T);
   }
 
-  template <typename T, typename = kj::EnableIf<kj::canConvert<T, v8::Value>()>>
+  template <V8Value T>
   v8::Local<T> wrap(jsg::Lock& js,
       v8::Local<v8::Context> context,
       kj::Maybe<v8::Local<v8::Object>> creator,

src/workerd/jsg/util.h

@@ -482,6 +482,9 @@ inline bool isFinite(double value) {
   return !(kj::isNaN(value) || value == kj::inf() || value == -kj::inf());
 }
 
+template <typename T>
+concept StrictlyBool = kj::isSameType<T, bool>();
+
 // ======================================================================================
 
 class Lock;

src/workerd/jsg/value.h

@@ -24,7 +24,6 @@ namespace workerd::jsg {
 
 // =======================================================================================
 // Primitives (numbers, booleans)
-
 // TypeWrapper mixin for numbers and booleans.
 //
 // This wrapper has extra wrap() overloads that take an isolate instead of a
@@ -375,15 +374,15 @@ class PrimitiveWrapper {
     return "boolean";
   }
 
-  template <typename T, typename = kj::EnableIf<kj::isSameType<T, bool>()>>
+  template <StrictlyBool T>
   v8::Local<v8::Boolean> wrap(
       Lock& js, v8::Local<v8::Context> context, kj::Maybe<v8::Local<v8::Object>> creator, T value) {
     // The template is needed to prevent this overload from being chosen for arbitrary types that
     // can convert to bool, such as pointers.
     return wrap(js.v8Isolate, creator, value);
   }
 
-  template <typename T, typename = kj::EnableIf<kj::isSameType<T, bool>()>>
+  template <StrictlyBool T>
   v8::Local<v8::Boolean> wrap(
       v8::Isolate* isolate, kj::Maybe<v8::Local<v8::Object>> creator, T value) {
     // The template is needed to prevent this overload from being chosen for arbitrary types that

src/workerd/jsg/web-idl.h

@@ -100,7 +100,7 @@ template <typename K, typename V>
 constexpr bool isRecordType<Dict<V, K>> = true;
 
 template <typename T>
-constexpr bool isBooleanType = kj::isSameType<T, bool>() || kj::isSameType<T, NonCoercible<bool>>();
+constexpr bool isBooleanType = StrictlyBool<T> || kj::isSameType<T, NonCoercible<bool>>();
 
 template <typename T>
 constexpr bool isIntegerType = kj::isSameType<T, int8_t>() || kj::isSameType<T, int16_t>() ||