Skip to content

Commit 827afd9

Browse files
coolreader18Centril
andcommitted
Apply suggestions from code review
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com> Signed-off-by: Noa <coolreader18@gmail.com>
1 parent 09735ba commit 827afd9

File tree

3 files changed

+33
-17
lines changed

3 files changed

+33
-17
lines changed

crates/core/src/host/v8/string.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ impl StringConst {
3737
.expect("`create_external_onebyte_const` should've asserted `.len() < kMaxLength`")
3838
}
3939

40+
/// Returns the backing string slice.
4041
pub(super) fn as_str(&'static self) -> &'static str {
4142
self.0.as_str()
4243
}

crates/core/src/host/v8/syscall/hooks.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,19 @@ use crate::host::v8::error::TypeError;
1212
use crate::host::v8::from_value::cast;
1313
use crate::host::v8::string::StringConst;
1414

15-
pub(super) fn get_hook_function<'s>(
16-
scope: &mut PinScope<'s, '_>,
15+
/// Returns the hook function `name` on `hooks_obj`.
16+
pub(super) fn get_hook_function<'scope>(
17+
scope: &mut PinScope<'scope, '_>,
1718
hooks_obj: Local<'_, Object>,
1819
name: &'static StringConst,
19-
) -> ExcResult<Local<'s, Function>> {
20+
) -> ExcResult<Local<'scope, Function>> {
2021
let key = name.string(scope);
2122
let object = property(scope, hooks_obj, key)?;
2223
cast!(scope, object, Function, "module function hook `{}`", name.as_str()).map_err(|e| e.throw(scope))
2324
}
2425

26+
/// Registers all the module function `hooks`
27+
/// and sets the given `AbiVersion` to `abi`.
2528
pub(super) fn set_hook_slots(
2629
scope: &mut PinScope<'_, '_>,
2730
abi: AbiVersion,
@@ -34,7 +37,7 @@ pub(super) fn set_hook_slots(
3437
for &(hook, func) in hooks {
3538
hooks_info
3639
.register(hook, abi)
37-
.map_err(|_| TypeError("cannot call register_hooks multiple times").throw(scope))?;
40+
.map_err(|_| TypeError("cannot call `register_hooks` multiple times").throw(scope))?;
3841
ctx.set_embedder_data(hook.to_slot_index(), func.into());
3942
}
4043
Ok(())
@@ -47,7 +50,8 @@ pub(in crate::host::v8) enum ModuleHook {
4750
}
4851

4952
impl ModuleHook {
50-
/// Get the `v8::Context::{get,set}_embedder_data` slot that holds this hook.
53+
/// Returns the index for the slot that holds the module function hook.
54+
/// The index is passed to `v8::Context::{get,set}_embedder_data`.
5155
fn to_slot_index(self) -> i32 {
5256
match self {
5357
ModuleHook::DescribeModule => 20,
@@ -56,13 +60,17 @@ impl ModuleHook {
5660
}
5761
}
5862

63+
/// Holds the `AbiVersion` used by the module
64+
/// and the module hooks registered by the module
65+
/// for that version.
5966
#[derive(Default)]
6067
struct HooksInfo {
6168
abi: OnceCell<AbiVersion>,
6269
registered: EnumMap<ModuleHook, OnceCell<()>>,
6370
}
6471

6572
impl HooksInfo {
73+
/// Returns, and possibly creates, the [`HooksInfo`] stored in `ctx`.
6674
fn get_or_create(ctx: &Context) -> Rc<Self> {
6775
ctx.get_slot().unwrap_or_else(|| {
6876
let this = Rc::<Self>::default();
@@ -78,16 +86,18 @@ impl HooksInfo {
7886
self.registered[hook].set(())
7987
}
8088

89+
/// Returns the `AbiVersion` for the given `hook`, if any.
8190
fn get(&self, hook: ModuleHook) -> Option<AbiVersion> {
8291
self.registered[hook].get().and(self.abi.get().copied())
8392
}
8493
}
8594

8695
#[derive(Copy, Clone)]
87-
pub(in crate::host::v8) struct HookFunction<'s>(pub AbiVersion, pub Local<'s, Function>);
96+
/// The actual callable module hook function and its abi version.
97+
pub(in super::super) struct HookFunction<'scope>(pub AbiVersion, pub Local<'scope, Function>);
8898

8999
/// Returns the hook function previously registered in [`register_hooks`].
90-
pub(in crate::host::v8) fn get_hook<'scope>(
100+
pub(in super::super) fn get_hook<'scope>(
91101
scope: &mut PinScope<'scope, '_>,
92102
hook: ModuleHook,
93103
) -> Option<HookFunction<'scope>> {

crates/core/src/host/v8/syscall/mod.rs

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,20 @@ pub enum AbiVersion {
2323

2424
/// A dependency resolver for the user's module
2525
/// that will resolve `spacetimedb_sys` to a module that exposes the ABI.
26-
pub(super) fn resolve_sys_module<'s>(
27-
context: Local<'s, Context>,
28-
spec: Local<'s, v8::String>,
29-
_attrs: Local<'s, FixedArray>,
30-
_referrer: Local<'s, Module>,
31-
) -> Option<Local<'s, Module>> {
26+
pub(super) fn resolve_sys_module<'scope>(
27+
context: Local<'scope, Context>,
28+
spec: Local<'scope, v8::String>,
29+
_attrs: Local<'scope, FixedArray>,
30+
_referrer: Local<'scope, Module>,
31+
) -> Option<Local<'scope, Module>> {
3232
callback_scope!(unsafe scope, context);
3333
resolve_sys_module_inner(scope, spec).ok()
3434
}
3535

36-
fn resolve_sys_module_inner<'s>(
37-
scope: &mut PinScope<'s, '_>,
38-
spec: Local<'s, v8::String>,
39-
) -> ExcResult<Local<'s, Module>> {
36+
fn resolve_sys_module_inner<'scope>(
37+
scope: &mut PinScope<'scope, '_>,
38+
spec: Local<'scope, v8::String>,
39+
) -> ExcResult<Local<'scope, Module>> {
4040
let scratch = &mut scratch_buf::<32>();
4141
let spec = spec.to_rust_cow_lossy(scope, scratch);
4242

@@ -65,6 +65,9 @@ fn resolve_sys_module_inner<'s>(
6565
}
6666
}
6767

68+
/// Calls the registered `__call_reducer__` function hook.
69+
///
70+
/// This handles any (future) ABI version differences.
6871
pub(super) fn call_call_reducer(
6972
scope: &mut PinScope<'_, '_>,
7073
fun: HookFunction<'_>,
@@ -77,6 +80,8 @@ pub(super) fn call_call_reducer(
7780
}
7881

7982
/// Calls the registered `__describe_module__` function hook.
83+
///
84+
/// This handles any (future) ABI version differences.
8085
pub(super) fn call_describe_module<'scope>(
8186
scope: &mut PinScope<'scope, '_>,
8287
fun: HookFunction<'_>,

0 commit comments

Comments
 (0)