Skip to content

Commit e872176

Browse files
authored
[chore] number of minor updates/modernizations to jsg code (#5683)
1 parent 769e53f commit e872176

File tree

9 files changed

+239
-228
lines changed

9 files changed

+239
-228
lines changed

src/workerd/jsg/function.h

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -49,36 +49,24 @@ class WrappableFunction<Ret(Args...)>: public WrappableFunctionBase {
4949
}
5050
};
5151

52-
template <typename Signature, typename Impl, bool = hasPublicVisitForGc<Impl>()>
52+
template <typename Signature, typename Impl>
5353
class WrappableFunctionImpl;
5454

5555
template <typename Ret, typename... Args, typename Impl>
56-
class WrappableFunctionImpl<Ret(Args...), Impl, false>: public WrappableFunction<Ret(Args...)> {
56+
class WrappableFunctionImpl<Ret(Args...), Impl>: public WrappableFunction<Ret(Args...)> {
5757
public:
5858
WrappableFunctionImpl(Impl&& func)
59-
: WrappableFunction<Ret(Args...)>(false),
59+
: WrappableFunction<Ret(Args...)>(hasPublicVisitForGc<Impl>()),
6060
func(kj::fwd<Impl>(func)) {}
6161

6262
Ret operator()(Lock& js, Args&&... args) override {
6363
return func(js, kj::fwd<Args>(args)...);
6464
}
6565

66-
private:
67-
Impl func;
68-
};
69-
70-
template <typename Ret, typename... Args, typename Impl>
71-
class WrappableFunctionImpl<Ret(Args...), Impl, true>: public WrappableFunction<Ret(Args...)> {
72-
public:
73-
WrappableFunctionImpl(Impl&& func)
74-
: WrappableFunction<Ret(Args...)>(true),
75-
func(kj::fwd<Impl>(func)) {}
76-
77-
Ret operator()(Lock& js, Args&&... args) override {
78-
return func(js, kj::fwd<Args>(args)...);
79-
}
8066
void jsgVisitForGc(GcVisitor& visitor) override {
81-
visitor.visit(func);
67+
if constexpr (hasPublicVisitForGc<Impl>()) {
68+
visitor.visit(func);
69+
}
8270
}
8371

8472
private:

src/workerd/jsg/jsg.h

Lines changed: 13 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -643,34 +643,17 @@ using HasGetTemplateOverload = decltype(kj::instance<T&>().getTemplate(
643643
registry.registerJsBundle(bundle); \
644644
} while (false)
645645

646-
namespace {
647-
// true when the T has _JSG_STRUCT_TS_ROOT_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_ROOT
648-
template <typename T, typename = int>
649-
struct HasStructTypeScriptRoot: std::false_type {};
650-
651-
// true when the T has _JSG_STRUCT_TS_ROOT_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_ROOT
646+
// true when T has _JSG_STRUCT_TS_ROOT_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_ROOT
652647
template <typename T>
653-
struct HasStructTypeScriptRoot<T, decltype(T::_JSG_STRUCT_TS_ROOT_DO_NOT_USE_DIRECTLY, 0)>
654-
: std::true_type {};
655-
656-
// true when the T has _JSG_STRUCT_TS_OVERRIDE_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_OVERRIDE
657-
template <typename T, typename = int>
658-
struct HasStructTypeScriptOverride: std::false_type {};
648+
concept HasStructTypeScriptRoot = requires { T::_JSG_STRUCT_TS_ROOT_DO_NOT_USE_DIRECTLY; };
659649

660-
// true when the T has _JSG_STRUCT_TS_OVERRIDE_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_OVERRIDE
650+
// true when T has _JSG_STRUCT_TS_OVERRIDE_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_OVERRIDE
661651
template <typename T>
662-
struct HasStructTypeScriptOverride<T, decltype(T::_JSG_STRUCT_TS_OVERRIDE_DO_NOT_USE_DIRECTLY, 0)>
663-
: std::true_type {};
664-
665-
// true when the T has _JSG_STRUCT_TS_DEFINE_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_DEFINE
666-
template <typename T, typename = int>
667-
struct HasStructTypeScriptDefine: std::false_type {};
652+
concept HasStructTypeScriptOverride = requires { T::_JSG_STRUCT_TS_OVERRIDE_DO_NOT_USE_DIRECTLY; };
668653

669-
// true when the T has _JSG_STRUCT_TS_DEFINE_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_DEFINE
654+
// true when T has _JSG_STRUCT_TS_DEFINE_DO_NOT_USE_DIRECTLY field generated by JSG_STRUCT_TS_DEFINE
670655
template <typename T>
671-
struct HasStructTypeScriptDefine<T, decltype(T::_JSG_STRUCT_TS_DEFINE_DO_NOT_USE_DIRECTLY, 0)>
672-
: std::true_type {};
673-
} // namespace
656+
concept HasStructTypeScriptDefine = requires { T::_JSG_STRUCT_TS_DEFINE_DO_NOT_USE_DIRECTLY; };
674657

675658
// Nest this inside a simple struct declaration in order to support translating it to/from a
676659
// JavaScript object / Web IDL dictionary.
@@ -728,18 +711,18 @@ struct HasStructTypeScriptDefine<T, decltype(T::_JSG_STRUCT_TS_DEFINE_DO_NOT_USE
728711
template <typename Registry, typename Self, typename Config> \
729712
static void registerMembersInternal(Registry& registry, Config arg) { \
730713
JSG_FOR_EACH(JSG_STRUCT_REGISTER_MEMBER, , __VA_ARGS__); \
731-
if constexpr (::workerd::jsg::HasStructTypeScriptRoot<Self>::value) { \
714+
if constexpr (::workerd::jsg::HasStructTypeScriptRoot<Self>) { \
732715
registry.registerTypeScriptRoot(); \
733716
} \
734717
if constexpr (requires(jsg::GetConfiguration<Self> arg) { \
735718
registerTypeScriptDynamicOverride<Registry>(registry, arg); \
736719
}) { \
737720
registerTypeScriptDynamicOverride<Registry>(registry, arg); \
738-
} else if constexpr (::workerd::jsg::HasStructTypeScriptOverride<Self>::value) { \
721+
} else if constexpr (::workerd::jsg::HasStructTypeScriptOverride<Self>) { \
739722
registry.template registerTypeScriptOverride< \
740723
Self::_JSG_STRUCT_TS_OVERRIDE_DO_NOT_USE_DIRECTLY>(); \
741724
} \
742-
if constexpr (::workerd::jsg::HasStructTypeScriptDefine<Self>::value) { \
725+
if constexpr (::workerd::jsg::HasStructTypeScriptDefine<Self>) { \
743726
registry \
744727
.template registerTypeScriptDefine<Self::_JSG_STRUCT_TS_DEFINE_DO_NOT_USE_DIRECTLY>(); \
745728
} \
@@ -757,12 +740,10 @@ struct HasStructTypeScriptDefine<T, decltype(T::_JSG_STRUCT_TS_DEFINE_DO_NOT_USE
757740
registerMembersInternal<Registry, Self, jsg::GetConfiguration<Self>>(registry, arg); \
758741
}
759742

760-
namespace {
761743
template <size_t N>
762-
consteval size_t prefixLengthToStrip(const char (&s)[N]) {
744+
inline consteval size_t prefixLengthToStrip(const char (&s)[N]) {
763745
return s[0] == '$' ? 1 : 0;
764746
}
765-
} // namespace
766747

767748
// This string may not be what's actually exported to v8. For example, if it starts with a `$`, then
768749
// this value will still contain the `$` even though the `FieldWrapper` template argument will have
@@ -1160,14 +1141,10 @@ template <typename T>
11601141
struct IsArguments_ {
11611142
static constexpr bool value = false;
11621143
};
1163-
1164-
// Is `T` some specialization of `Arguments<U>`?
11651144
template <typename T>
11661145
struct IsArguments_<Arguments<T>> {
11671146
static constexpr bool value = true;
11681147
};
1169-
1170-
// Is `T` some specialization of `Arguments<U>`?
11711148
template <typename T>
11721149
constexpr bool isArguments() {
11731150
return IsArguments_<T>::value;
@@ -1636,14 +1613,10 @@ template <typename T>
16361613
struct IsPromise_ {
16371614
static constexpr bool value = false;
16381615
};
1639-
1640-
// Convenience template to detect a `jsg::Promise` type.
16411616
template <typename T>
16421617
struct IsPromise_<Promise<T>> {
16431618
static constexpr bool value = true;
16441619
};
1645-
1646-
// Convenience template to detect a `jsg::Promise` type.
16471620
template <typename T>
16481621
constexpr bool isPromise() {
16491622
return IsPromise_<T>::value;
@@ -1654,67 +1627,45 @@ template <typename T>
16541627
struct RemovePromise_ {
16551628
using Type = T;
16561629
};
1657-
1658-
// Convenience template to strip off `jsg::Promise`.
16591630
template <typename T>
16601631
struct RemovePromise_<Promise<T>> {
16611632
using Type = T;
16621633
};
1663-
1664-
// Convenience template to strip off `jsg::Promise`.
16651634
template <typename T>
16661635
using RemovePromise = typename RemovePromise_<T>::Type;
16671636

1637+
// Convenience template to add `jsg::Promise` if it is not present.
16681638
template <typename T>
16691639
struct MaintainPromise_ {
16701640
using Type = Promise<T>;
16711641
};
1672-
1673-
// Convenience template to add `jsg::Promise` if it is not present.
16741642
template <typename T>
16751643
struct MaintainPromise_<Promise<T>> {
16761644
using Type = Promise<T>;
16771645
};
1678-
1679-
// Convenience template to add `jsg::Promise` if it is not present.
16801646
template <typename T>
16811647
using MaintainPromise = typename MaintainPromise_<T>::Type;
16821648

16831649
// Convenience template to calculate the return type of a function when passed parameter type T.
16841650
// `T = void` is understood to mean no parameters.
16851651
template <typename Func, typename T, bool passLock>
16861652
struct ReturnType_;
1687-
1688-
// Convenience template to calculate the return type of a function when passed parameter type T.
1689-
// `T = void` is understood to mean no parameters.
16901653
template <typename Func, typename T>
16911654
struct ReturnType_<Func, T, false> {
16921655
using Type = decltype(kj::instance<Func>()(kj::instance<T>()));
16931656
};
1694-
1695-
// Convenience template to calculate the return type of a function when passed parameter type T.
1696-
// `T = void` is understood to mean no parameters.
16971657
template <typename Func, typename T>
16981658
struct ReturnType_<Func, T, true> {
16991659
using Type = decltype(kj::instance<Func>()(kj::instance<Lock&>(), kj::instance<T>()));
17001660
};
1701-
1702-
// Convenience template to calculate the return type of a function when passed parameter type T.
1703-
// `T = void` is understood to mean no parameters.
17041661
template <typename Func>
17051662
struct ReturnType_<Func, void, false> {
17061663
using Type = decltype(kj::instance<Func>()());
17071664
};
1708-
1709-
// Convenience template to calculate the return type of a function when passed parameter type T.
1710-
// `T = void` is understood to mean no parameters.
17111665
template <typename Func>
17121666
struct ReturnType_<Func, void, true> {
17131667
using Type = decltype(kj::instance<Func>()(kj::instance<Lock&>()));
17141668
};
1715-
1716-
// Convenience template to calculate the return type of a function when passed parameter type T.
1717-
// `T = void` is understood to mean no parameters.
17181669
template <typename Func, typename T, bool passLock = false>
17191670
using ReturnType = typename ReturnType_<Func, T, passLock>::Type;
17201671

@@ -2922,11 +2873,11 @@ class Lock {
29222873
// The lock must be assignable to a jsg::Lock, and the context must be or be assignable
29232874
// to a v8::Local<v8::Context>. The context will be evaluated within the handle scope.
29242875
#define JSG_WITHIN_CONTEXT_SCOPE(lock, context, fn) \
2925-
((jsg::Lock&)lock).withinHandleScope([&]() -> auto { \
2876+
(static_cast<jsg::Lock&>(lock)).withinHandleScope([&]() -> auto { \
29262877
v8::Local<v8::Context> ctx = context; \
29272878
KJ_ASSERT(!ctx.IsEmpty(), "unable to enter invalid v8::Context"); \
29282879
v8::Context::Scope scope(ctx); \
2929-
return fn((jsg::Lock&)lock); \
2880+
return fn(static_cast<jsg::Lock&>(lock)); \
29302881
})
29312882

29322883
// The V8StackScope is used only as a marker to prove that we are running in the V8 stack
@@ -3013,9 +2964,6 @@ inline V8Ref<T> V8Ref<T>::addRef(jsg::Lock& js) {
30132964
return js.v8Ref(getHandle(js));
30142965
}
30152966

3016-
// v8::Local<v8::Value> deepClone(v8::Local<v8::Context> context, v8::Local<v8::Value> value);
3017-
// Already defined in util.c++.
3018-
30192967
template <typename T>
30202968
V8Ref<T> V8Ref<T>::deepClone(jsg::Lock& js) {
30212969
return js.v8Ref(jsg::deepClone(js.v8Context(), getHandle(js)).template As<T>());

0 commit comments

Comments
 (0)