From 4aa0dfae3dedb207d3aca7e786905d3cd5476453 Mon Sep 17 00:00:00 2001 From: snek Date: Fri, 12 Jul 2024 09:52:25 -0700 Subject: [PATCH 1/2] rust 1.79.0 (#820) since i had to annotate all the `transmute`s for clippy i removed a few that were not needed. --- core/fast_string.rs | 2 +- core/runtime/bindings.rs | 5 +++- core/runtime/jsrealm.rs | 5 ---- core/runtime/jsruntime.rs | 34 +++++++------------------ core/runtime/op_driver/erased_future.rs | 5 +++- core/runtime/snapshot.rs | 4 ++- core/tasks.rs | 3 ++- rust-toolchain.toml | 2 +- serde_v8/magic/value.rs | 4 ++- 9 files changed, 27 insertions(+), 37 deletions(-) diff --git a/core/fast_string.rs b/core/fast_string.rs index b5f515dea..f4c3d9bfd 100644 --- a/core/fast_string.rs +++ b/core/fast_string.rs @@ -442,7 +442,7 @@ macro_rules! ascii_str { ($str:expr) => {{ const C: $crate::v8::OneByteConst = $crate::FastStaticString::create_external_onebyte_const($str.as_bytes()); - unsafe { std::mem::transmute::<_, $crate::FastStaticString>(&C) } + $crate::FastStaticString::new(&C) }}; } diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs index 0b63fd63e..e7c70d7ed 100644 --- a/core/runtime/bindings.rs +++ b/core/runtime/bindings.rs @@ -148,7 +148,10 @@ pub(crate) fn externalize_sources( let offset = snapshot_sources.len(); for (index, source) in sources.into_iter().enumerate() { externals[index + offset] = - FastStaticString::create_external_onebyte_const(std::mem::transmute( + FastStaticString::create_external_onebyte_const(std::mem::transmute::< + &[u8], + &[u8], + >( source.code.as_bytes(), )); let ptr = &externals[index + offset] as *const v8::OneByteConst; diff --git a/core/runtime/jsrealm.rs b/core/runtime/jsrealm.rs index e3bacc110..0dde0095b 100644 --- a/core/runtime/jsrealm.rs +++ b/core/runtime/jsrealm.rs @@ -26,7 +26,6 @@ use std::hash::BuildHasherDefault; use std::hash::Hasher; use std::rc::Rc; use std::sync::Arc; -use v8::Handle; pub const CONTEXT_STATE_SLOT_INDEX: i32 = 1; pub const MODULE_MAP_SLOT_INDEX: i32 = 2; @@ -291,10 +290,6 @@ impl JsRealm { self.0.context() } - pub(crate) fn context_ptr(&self) -> *mut v8::Context { - unsafe { self.0.context.get_unchecked() as *const _ as _ } - } - /// Executes traditional JavaScript code (traditional = not ES modules) in the /// realm's context. /// diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index ce658acae..e29db5f5d 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -775,9 +775,9 @@ impl JsRuntime { ))); let external_refs: &v8::ExternalReferences = - isolate_allocations.external_refs.as_ref().unwrap().as_ref(); + isolate_allocations.external_refs.as_ref().unwrap(); // SAFETY: We attach external_refs to IsolateAllocations which will live as long as the isolate - let external_refs_static = unsafe { std::mem::transmute(external_refs) }; + let external_refs_static = unsafe { &*(external_refs as *const _) }; let has_snapshot = maybe_startup_snapshot.is_some(); let mut isolate = setup::create_isolate( @@ -1068,23 +1068,6 @@ impl JsRuntime { self.inner.main_realm.handle_scope(isolate) } - #[inline(always)] - /// Create a scope on the stack with the given context - fn with_context_scope<'s, T>( - isolate: *mut v8::Isolate, - context: *mut v8::Context, - f: impl FnOnce(&mut v8::HandleScope<'s>) -> T, - ) -> T { - // SAFETY: We know this isolate is valid and non-null at this time - let mut isolate_scope = - v8::HandleScope::new(unsafe { isolate.as_mut().unwrap_unchecked() }); - // SAFETY: We know the context is valid and non-null at this time, and that a Local and pointer share the - // same representation - let context = unsafe { std::mem::transmute(context) }; - let mut scope = v8::ContextScope::new(&mut isolate_scope, context); - f(&mut scope) - } - /// Create a synthetic module - `ext:core/ops` - that exports all ops registered /// with the runtime. fn execute_virtual_ops_module( @@ -1726,12 +1709,13 @@ impl JsRuntime { cx: &mut Context, poll_options: PollEventLoopOptions, ) -> Poll> { - let isolate = self.v8_isolate_ptr(); - Self::with_context_scope( - isolate, - self.inner.main_realm.context_ptr(), - move |scope| self.poll_event_loop_inner(cx, scope, poll_options), - ) + // SAFETY: We know this isolate is valid and non-null at this time + let mut isolate_scope = + v8::HandleScope::new(unsafe { &mut *self.v8_isolate_ptr() }); + let context = + v8::Local::new(&mut isolate_scope, self.inner.main_realm.context()); + let mut scope = v8::ContextScope::new(&mut isolate_scope, context); + self.poll_event_loop_inner(cx, &mut scope, poll_options) } fn poll_event_loop_inner( diff --git a/core/runtime/op_driver/erased_future.rs b/core/runtime/op_driver/erased_future.rs index bc0a14aa1..2399992b9 100644 --- a/core/runtime/op_driver/erased_future.rs +++ b/core/runtime/op_driver/erased_future.rs @@ -94,7 +94,10 @@ impl ErasedFuture { where F: Future, { - F::poll(std::mem::transmute(pin), cx) + F::poll( + std::mem::transmute::>, Pin<&mut F>>(pin), + cx, + ) } #[allow(dead_code)] diff --git a/core/runtime/snapshot.rs b/core/runtime/snapshot.rs index 871806181..66aa8b61e 100644 --- a/core/runtime/snapshot.rs +++ b/core/runtime/snapshot.rs @@ -102,7 +102,9 @@ impl SnapshotStoreDataStore { // TODO(mmastrac): v8::Global needs From/Into // SAFETY: Because we've tested that Local: From>, we can assume this is safe. unsafe { - self.data.push(std::mem::transmute(global)); + self.data.push( + std::mem::transmute::, v8::Global>(global), + ); } id as _ } diff --git a/core/tasks.rs b/core/tasks.rs index cfc260205..39b0c286a 100644 --- a/core/tasks.rs +++ b/core/tasks.rs @@ -84,7 +84,8 @@ impl V8TaskSpawnerFactory { // SAFETY: we are removing the Send trait as we return the tasks here to prevent // these tasks from accidentally leaking to another thread. - let tasks = unsafe { std::mem::transmute(tasks) }; + let tasks = + unsafe { std::mem::transmute::, Vec>(tasks) }; Poll::Ready(tasks) } diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 2535b0131..874939176 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "1.78.0" +channel = "1.79.0" components = ["rustfmt", "clippy"] diff --git a/serde_v8/magic/value.rs b/serde_v8/magic/value.rs index 4ecd1aff7..da4dfa9bd 100644 --- a/serde_v8/magic/value.rs +++ b/serde_v8/magic/value.rs @@ -38,7 +38,9 @@ impl ToV8 for Value<'_> { _scope: &mut v8::HandleScope<'a>, ) -> Result, crate::Error> { // SAFETY: not fully safe, since lifetimes are detached from original scope - Ok(unsafe { transmute(self.v8_value) }) + Ok(unsafe { + transmute::, v8::Local>(self.v8_value) + }) } } From b5218a541a6d6c6ceeb2163897a328b5e0c3db52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 13 Jul 2024 00:35:01 +0100 Subject: [PATCH 2/2] refactor: Simplify SourceMapGetter and SourceMapper (#821) Splitting out a simplification from https://github.com/denoland/deno_core/pull/819. --- core/extension_set.rs | 5 ++--- core/runtime/jsruntime.rs | 5 ++--- core/source_map.rs | 27 +++++---------------------- 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/core/extension_set.rs b/core/extension_set.rs index 938a771ea..fafa6a6a6 100644 --- a/core/extension_set.rs +++ b/core/extension_set.rs @@ -22,7 +22,6 @@ use crate::ModuleCodeString; use crate::OpDecl; use crate::OpMetricsFactoryFn; use crate::OpState; -use crate::SourceMapGetter; /// Contribute to the `OpState` from each extension. pub fn setup_op_state(op_state: &mut OpState, extensions: &mut [Extension]) { @@ -242,7 +241,7 @@ impl<'a> IntoIterator for &'a mut LoadedSources { fn load( transpiler: Option<&ExtensionTranspiler>, source: &ExtensionFileSource, - source_mapper: &mut SourceMapper>, + source_mapper: &mut SourceMapper, load_callback: &mut impl FnMut(&ExtensionFileSource), ) -> Result { load_callback(source); @@ -263,7 +262,7 @@ fn load( pub fn into_sources( transpiler: Option<&ExtensionTranspiler>, extensions: &[Extension], - source_mapper: &mut SourceMapper>, + source_mapper: &mut SourceMapper, mut load_callback: impl FnMut(&ExtensionFileSource), ) -> Result { let mut sources = LoadedSources::default(); diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index e29db5f5d..1081dde13 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -407,7 +407,7 @@ pub type CompiledWasmModuleStore = CrossIsolateStore; /// Internal state for JsRuntime which is stored in one of v8::Isolate's /// embedder slots. pub struct JsRuntimeState { - pub(crate) source_mapper: RefCell>>, + pub(crate) source_mapper: RefCell, pub(crate) op_state: Rc>, pub(crate) shared_array_buffer_store: Option, pub(crate) compiled_wasm_module_store: Option, @@ -672,8 +672,7 @@ impl JsRuntime { // Load the sources and source maps let mut files_loaded = Vec::with_capacity(128); - let mut source_mapper: SourceMapper> = - SourceMapper::new(options.source_map_getter); + let mut source_mapper = SourceMapper::new(options.source_map_getter); let mut sources = extension_set::into_sources( options.extension_transpiler.as_deref(), &extensions, diff --git a/core/source_map.rs b/core/source_map.rs index 396e21370..97e8b4fdc 100644 --- a/core/source_map.rs +++ b/core/source_map.rs @@ -19,23 +19,6 @@ pub trait SourceMapGetter { ) -> Option; } -impl SourceMapGetter for Rc -where - T: SourceMapGetter + ?Sized, -{ - fn get_source_map(&self, file_name: &str) -> Option> { - (**self).get_source_map(file_name) - } - - fn get_source_line( - &self, - file_name: &str, - line_number: usize, - ) -> Option { - (**self).get_source_line(file_name, line_number) - } -} - pub enum SourceMapApplication { /// No mapping was applied, the location is unchanged. Unchanged, @@ -54,18 +37,18 @@ pub enum SourceMapApplication { pub type SourceMapData = Cow<'static, [u8]>; -pub struct SourceMapper { +pub struct SourceMapper { maps: HashMap>, source_lines: HashMap<(String, i64), Option>, - getter: Option, + getter: Option>, pub(crate) ext_source_maps: HashMap, // This is not the right place for this, but it's the easiest way to make // op_apply_source_map a fast op. This stashing should happen in #[op2]. pub(crate) stashed_file_name: Option, } -impl SourceMapper { - pub fn new(getter: Option) -> Self { +impl SourceMapper { + pub fn new(getter: Option>) -> Self { Self { maps: Default::default(), source_lines: Default::default(), @@ -75,7 +58,7 @@ impl SourceMapper { } } - pub fn has_user_sources(&self) -> bool { + pub(crate) fn has_user_sources(&self) -> bool { self.getter.is_some() }