Skip to content

Commit

Permalink
Revert "Allow #[cppgc] &mut T in sync ops (#791)"
Browse files Browse the repository at this point in the history
This reverts commit 3cc4464.
  • Loading branch information
littledivy authored Jun 21, 2024
1 parent 2c438fb commit c480cfa
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 232 deletions.
15 changes: 4 additions & 11 deletions core/cppgc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,10 @@ pub fn make_cppgc_object<'a, T: GcResource + 'static>(
obj
}

// TODO(littledivy): https://github.com/denoland/rusty_v8/pull/1505
#[repr(C)]
struct InnerMember {
inner: [usize; 2],
ptr: *mut (),
}

#[allow(clippy::needless_lifetimes)]
pub fn try_unwrap_cppgc_object<'sc, T: GcResource + 'static>(
val: v8::Local<'sc, v8::Value>,
) -> Option<&'sc mut T> {
) -> Option<&'sc T> {
let Ok(obj): Result<v8::Local<v8::Object>, _> = val.try_into() else {
return None;
};
Expand All @@ -69,13 +62,13 @@ pub fn try_unwrap_cppgc_object<'sc, T: GcResource + 'static>(
// The object can only be created by `make_cppgc_object`.
let member = unsafe {
let ptr = obj.get_aligned_pointer_from_internal_field(SLOT_OFFSET);
let obj = &*(ptr as *mut InnerMember);
obj.ptr.cast::<CppGcObject<T>>().as_mut().unwrap()
let obj = &*(ptr as *const v8::cppgc::InnerMember);
obj.get::<CppGcObject<T>>()
};

if member.tag != TypeId::of::<T>() {
return None;
}

Some(&mut member.member)
Some(&member.member)
}
15 changes: 2 additions & 13 deletions core/runtime/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,6 @@ mod tests {
op_test_get_cppgc_resource,
op_test_get_cppgc_resource_option,
op_test_make_cppgc_resource,
op_test_set_cppgc_resource,
op_test_make_cppgc_resource_option,
op_external_make,
op_external_process,
Expand Down Expand Up @@ -1928,14 +1927,6 @@ mod tests {
resource.value
}

#[op2(fast)]
pub fn op_test_set_cppgc_resource(
#[cppgc] resource: &mut TestResource,
value: u32,
) {
resource.value = value;
}

#[op2(fast)]
#[smi]
pub fn op_test_get_cppgc_resource_option(
Expand All @@ -1953,7 +1944,7 @@ mod tests {
{
run_async_test(
10,
"op_test_make_cppgc_resource, op_test_get_cppgc_resource, op_test_get_cppgc_resource_option, op_test_make_cppgc_resource_option, op_test_set_cppgc_resource",
"op_test_make_cppgc_resource, op_test_get_cppgc_resource, op_test_get_cppgc_resource_option, op_test_make_cppgc_resource_option",
r"
const resource = op_test_make_cppgc_resource();
assert((await op_test_get_cppgc_resource(resource)) === 42);
Expand All @@ -1962,9 +1953,7 @@ mod tests {
const resource2 = op_test_make_cppgc_resource_option(false);
assert(resource2 === null);
const resource3 = op_test_make_cppgc_resource_option(true);
assert((await op_test_get_cppgc_resource(resource3)) === 42);
op_test_set_cppgc_resource(resource, 43);
assert((await op_test_get_cppgc_resource(resource)) == 43);"
assert((await op_test_get_cppgc_resource(resource3)) === 42);",
).await?;
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion ops/op2/dispatch_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(crate) fn generate_dispatch_async(
let mut output = TokenStream::new();

let with_self = if generator_state.needs_self {
with_self(generator_state, true, &signature.ret_val)
with_self(generator_state, &signature.ret_val)
.map_err(V8SignatureMappingError::NoSelfMapping)?
} else {
quote!()
Expand Down
2 changes: 1 addition & 1 deletion ops/op2/dispatch_fast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ fn map_v8_fastcall_arg_to_arg(
let #arg_ident = if #arg_ident.is_null_or_undefined() {
None
} else if let Some(#arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(#arg_ident) {
Some(#arg_ident as _)
Some(#arg_ident)
} else {
#fast_api_callback_options.fallback = true;
// SAFETY: All fast return types have zero as a valid value
Expand Down
6 changes: 2 additions & 4 deletions ops/op2/dispatch_slow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub(crate) fn generate_dispatch_slow(
)?);

let with_self = if generator_state.needs_self {
with_self(generator_state, false, &signature.ret_val)
with_self(generator_state, &signature.ret_val)
.map_err(V8SignatureMappingError::NoSelfMapping)?
} else {
quote!()
Expand Down Expand Up @@ -246,7 +246,6 @@ pub(crate) fn with_js_runtime_state(

pub(crate) fn with_self(
generator_state: &mut GeneratorState,
is_async: bool,
ret_val: &RetVal,
) -> Result<TokenStream, V8MappingError> {
generator_state.needs_opctx = true;
Expand All @@ -255,7 +254,6 @@ pub(crate) fn with_self(
format!("expected {}", &generator_state.self_ty),
)?;
let tokens = if matches!(ret_val, RetVal::Future(_) | RetVal::FutureResult(_))
|| is_async
{
let tokens = gs_quote!(generator_state(self_ty, fn_args, scope) => {
let Some(self_) = deno_core::_ops::CppGcObjectGuard::<#self_ty>::try_new_from_cppgc_object(&mut *#scope, #fn_args.this().into()) else {
Expand Down Expand Up @@ -619,7 +617,7 @@ pub fn from_arg(
let #arg_ident = if #arg_ident.is_null_or_undefined() {
None
} else if let Some(#arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(#arg_ident) {
Some(#arg_ident as _)
Some(#arg_ident)
} else {
#throw_exception;
};
Expand Down
16 changes: 9 additions & 7 deletions ops/op2/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1473,17 +1473,19 @@ fn parse_type_state(ty: &Type) -> Result<Arg, ArgError> {

fn parse_cppgc(position: Position, ty: &Type) -> Result<Arg, ArgError> {
match (position, ty) {
(Position::Arg, Type::Reference(of)) => match &*of.elem {
Type::Path(of) => Ok(Arg::CppGcResource(stringify_token(&of.path))),
_ => Err(ArgError::InvalidCppGcType(stringify_token(ty))),
},
(Position::Arg, Type::Reference(of)) if of.mutability.is_none() => {
match &*of.elem {
Type::Path(of) => Ok(Arg::CppGcResource(stringify_token(&of.path))),
_ => Err(ArgError::InvalidCppGcType(stringify_token(&of.elem))),
}
}
(Position::Arg, Type::Path(of)) => {
rules!(of.to_token_stream() => {
( Option < $ty:ty $(,)? > ) => {
match ty {
Type::Reference(of) => {
Type::Reference(of) if of.mutability.is_none() => {
match &*of.elem {
Type::Path(f) => Ok(Arg::OptionCppGcResource(stringify_token(&f.path))),
Type::Path(of) => Ok(Arg::OptionCppGcResource(stringify_token(&of.path))),
_ => Err(ArgError::InvalidCppGcType(stringify_token(&of.elem))),
}
}
Expand Down Expand Up @@ -2048,7 +2050,7 @@ mod tests {
);
expect_fail!(
op_cppgc_resource_invalid_type,
ArgError("resource", InvalidCppGcType("&[std :: fs :: File]")),
ArgError("resource", InvalidCppGcType("[std :: fs :: File]")),
fn f(#[cppgc] resource: &[std::fs::File]) {}
);
expect_fail!(
Expand Down
171 changes: 2 additions & 169 deletions ops/op2/test_cases/sync/cppgc_resource.out

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions ops/op2/test_cases/sync/cppgc_resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ fn op_make_cppgc_object() -> Wrap {
#[op2(fast)]
fn op_use_cppgc_object(#[cppgc] _wrap: &Wrap) {}

#[op2(fast)]
fn op_use_cppgc_object_mut(#[cppgc] _wrap: &mut Wrap) {}

#[op2]
#[cppgc]
fn op_make_cppgc_object_option() -> Option<Wrap> {
Expand Down
3 changes: 0 additions & 3 deletions ops/op2/test_cases_fail/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,3 @@ fn op_use_cppgc_object(#[cppgc] _wrap: &'static Wrap) {}

#[op2(fast)]
fn op_use_buffer(#[buffer] _buffer: &'static [u8]) {}

#[op2(async)]
async fn op_use_cppgc_object_mut_async(#[cppgc] _wrap: &mut Wrap) {}
Loading

0 comments on commit c480cfa

Please sign in to comment.