Skip to content

Commit

Permalink
refactor: encapsulate source map logic in SourceMapper (denoland#532)
Browse files Browse the repository at this point in the history 
This is necessary for denoland#530
  • Loading branch information
@Digifox03
Digifox03 committed Feb 1, 2024
1 parent 2c0302b commit 8c3dc51
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 202 deletions.
69 changes: 15 additions & 54 deletions core/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ use anyhow::Error;

use crate::runtime::JsRealm;
use crate::runtime::JsRuntime;
use crate::source_map::apply_source_map;
use crate::source_map::get_source_line;
use crate::source_map::SourceMapApplication;
use crate::url::Url;

Expand Down Expand Up @@ -213,21 +211,8 @@ impl JsStackFrame {
// V8's column numbers are 0-based, we want 1-based.
let c = message.get_start_column() as u32 + 1;
let state = JsRuntime::state_from(scope);
let (getter, cache) = {
(
state.source_map_getter.clone(),
state.source_map_cache.clone(),
)
};

let application = if let Some(source_map_getter) = getter {
let mut cache = cache.borrow_mut();
apply_source_map(&f, l, c, &mut cache, &**source_map_getter)
} else {
SourceMapApplication::Unchanged
};

match application {
let mut source_mapper = state.source_mapper.borrow_mut();
match source_mapper.apply_source_map(&f, l, c) {
SourceMapApplication::Unchanged => Some(JsStackFrame::from_location(
Some(f),
Some(l.into()),
Expand Down Expand Up @@ -331,28 +316,15 @@ impl JsError {
}
{
let state = JsRuntime::state_from(scope);
let (getter, cache) = {
(
state.source_map_getter.clone(),
state.source_map_cache.clone(),
)
};
if let Some(source_map_getter) = getter {
let mut cache = cache.borrow_mut();
for (i, frame) in frames.iter().enumerate() {
if let (Some(file_name), Some(line_number)) =
(&frame.file_name, frame.line_number)
{
if !file_name.trim_start_matches('[').starts_with("ext:") {
source_line = get_source_line(
file_name,
line_number,
&mut cache,
&**source_map_getter,
);
source_line_frame_index = Some(i);
break;
}
let mut source_mapper = state.source_mapper.borrow_mut();
for (i, frame) in frames.iter().enumerate() {
if let (Some(file_name), Some(line_number)) =
(&frame.file_name, frame.line_number)
{
if !file_name.trim_start_matches('[').starts_with("ext:") {
source_line = source_mapper.get_source_line(file_name, line_number);
source_line_frame_index = Some(i);
break;
}
}
}
Expand Down Expand Up @@ -462,26 +434,15 @@ impl JsError {
}
{
let state = JsRuntime::state_from(scope);
let (getter, cache) = {
(
state.source_map_getter.clone(),
state.source_map_cache.clone(),
)
};
if let Some(source_map_getter) = getter {
let mut cache = cache.borrow_mut();

let mut source_mapper = state.source_mapper.borrow_mut();
if source_mapper.has_user_sources() {
for (i, frame) in frames.iter().enumerate() {
if let (Some(file_name), Some(line_number)) =
(&frame.file_name, frame.line_number)
{
if !file_name.trim_start_matches('[').starts_with("ext:") {
source_line = get_source_line(
file_name,
line_number,
&mut cache,
&**source_map_getter,
);
source_line =
source_mapper.get_source_line(file_name, line_number);
source_line_frame_index = Some(i);
break;
}
Expand Down
66 changes: 23 additions & 43 deletions core/ops_builtin_v8.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::resolve_url;
use crate::runtime::script_origin;
use crate::runtime::JsRealm;
use crate::runtime::JsRuntimeState;
use crate::source_map::apply_source_map;
use crate::source_map::SourceMapApplication;
use crate::JsBuffer;
use crate::JsRuntime;
Expand Down Expand Up @@ -896,36 +895,27 @@ pub fn op_apply_source_map(
if ret_buf.len() != 8 {
return Err(type_error("retBuf must be 8 bytes"));
}
if let Some(source_map_getter) = state.source_map_getter.as_ref() {
let mut cache = state.source_map_cache.borrow_mut();
let application = apply_source_map(
file_name,
let mut source_mapper = state.source_mapper.borrow_mut();
let application =
source_mapper.apply_source_map(file_name, line_number, column_number);
match application {
SourceMapApplication::Unchanged => Ok(0),
SourceMapApplication::LineAndColumn {
line_number,
column_number,
&mut cache,
&***source_map_getter,
);
match application {
SourceMapApplication::Unchanged => Ok(0),
SourceMapApplication::LineAndColumn {
line_number,
column_number,
} => {
write_line_and_col_to_ret_buf(ret_buf, line_number, column_number);
Ok(1)
}
SourceMapApplication::LineAndColumnAndFileName {
line_number,
column_number,
file_name,
} => {
write_line_and_col_to_ret_buf(ret_buf, line_number, column_number);
cache.stashed_file_name.replace(file_name);
Ok(2)
}
} => {
write_line_and_col_to_ret_buf(ret_buf, line_number, column_number);
Ok(1)
}
SourceMapApplication::LineAndColumnAndFileName {
line_number,
column_number,
file_name,
} => {
write_line_and_col_to_ret_buf(ret_buf, line_number, column_number);
source_mapper.stashed_file_name.replace(file_name);
Ok(2)
}
} else {
Ok(0)
}
}

Expand All @@ -937,7 +927,7 @@ pub fn op_apply_source_map_filename(
state: &JsRuntimeState,
) -> Result<String, Error> {
state
.source_map_cache
.source_mapper
.borrow_mut()
.stashed_file_name
.take()
Expand Down Expand Up @@ -970,20 +960,10 @@ pub fn op_current_user_call_site(
}
let line_number = frame.get_line_number() as u32;
let column_number = frame.get_column() as u32;
let application = if let Some(source_map_getter) =
js_runtime_state.source_map_getter.as_ref()
{
let mut cache = js_runtime_state.source_map_cache.borrow_mut();
apply_source_map(
&file_name,
line_number,
column_number,
&mut cache,
&***source_map_getter,
)
} else {
SourceMapApplication::Unchanged
};
let application = js_runtime_state
.source_mapper
.borrow_mut()
.apply_source_map(&file_name, line_number, column_number);

match application {
SourceMapApplication::Unchanged => {
Expand Down
8 changes: 3 additions & 5 deletions core/runtime/jsruntime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ use crate::ops_metrics::OpMetricsFactoryFn;
use crate::runtime::ContextState;
use crate::runtime::JsRealm;
use crate::runtime::OpDriverImpl;
use crate::source_map::SourceMapCache;
use crate::source_map::SourceMapGetter;
use crate::source_map::SourceMapper;
use crate::Extension;
use crate::ExtensionFileSource;
use crate::ExtensionFileSourceCode;
Expand Down Expand Up @@ -375,8 +375,7 @@ pub type CompiledWasmModuleStore = CrossIsolateStore<v8::CompiledWasmModule>;
/// Internal state for JsRuntime which is stored in one of v8::Isolate's
/// embedder slots.
pub struct JsRuntimeState {
pub(crate) source_map_getter: Option<Rc<Box<dyn SourceMapGetter>>>,
pub(crate) source_map_cache: Rc<RefCell<SourceMapCache>>,
pub(crate) source_mapper: RefCell<SourceMapper<Box<dyn SourceMapGetter>>>,
pub(crate) op_state: Rc<RefCell<OpState>>,
pub(crate) shared_array_buffer_store: Option<SharedArrayBufferStore>,
pub(crate) compiled_wasm_module_store: Option<CompiledWasmModuleStore>,
Expand Down Expand Up @@ -608,8 +607,7 @@ impl JsRuntime {
let waker = op_state.waker.clone();
let op_state = Rc::new(RefCell::new(op_state));
let state_rc = Rc::new(JsRuntimeState {
source_map_getter: options.source_map_getter.map(Rc::new),
source_map_cache: Default::default(),
source_mapper: RefCell::new(SourceMapper::new(options.source_map_getter)),
shared_array_buffer_store: options.shared_array_buffer_store,
compiled_wasm_module_store: options.compiled_wasm_module_store,
wait_for_inspector_disconnect_callback: options
Expand Down
Loading

0 comments on commit 8c3dc51

Please sign in to comment.