Skip to content

Remove SourceMap wrapper providing Send+Sync #71402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Remove SourceMap wrapper providing Send+Sync #71402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cargo.lock
View file
Open in desktop

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

3 changes: 3 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,6 @@ unsize = "1.1.0"
url = "2.2.2"
urlencoding = "2.1.2"
webbrowser = "0.8.7"

[patch.crates-io]
sourcemap = { git = "https://github.com/wbinnssmith/rust-sourcemap", branch = "wbinnssmith/decoded-serde" }
117 changes: 26 additions & 91 deletions turbopack/crates/turbopack-core/src/source_map/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use std::{borrow::Cow, io::Write, ops::Deref, sync::Arc};

use anyhow::Result;
use once_cell::sync::Lazy;
use ref_cast::RefCast;
use regex::Regex;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use serde::{Deserialize, Serialize};
use sourcemap::{DecodedMap, SourceMap as RegularMap, SourceMapBuilder, SourceMapIndex};
use turbo_tasks::{RcStr, TryJoinIterExt, ValueToString, Vc};
use turbo_tasks_fs::{
Expand Down Expand Up @@ -217,7 +216,7 @@ impl SourceMap {
}

impl SourceMap {
pub async fn to_source_map(&self) -> Result<Arc<CrateMapWrapper>> {
pub async fn to_source_map(&self) -> Result<Arc<DecodedMap>> {
Ok(match self {
Self::Decoded(m) => m.map.clone(),
Self::Sectioned(m) => {
Expand All @@ -229,13 +228,11 @@ impl SourceMap {
sourcemap::SourceMapSection::new(
(s.offset.line as u32, s.offset.column as u32),
None,
Some(s.map.0.clone()),
Some((*s.map.clone()).clone()),
)
})
.collect::<Vec<sourcemap::SourceMapSection>>();
Arc::new(CrateMapWrapper(DecodedMap::Index(SourceMapIndex::new(
None, sections,
))))
Arc::new(DecodedMap::Index(SourceMapIndex::new(None, sections)))
}
})
}
Expand All @@ -261,7 +258,7 @@ impl SourceMap {
let rope = match &*this {
SourceMap::Decoded(r) => {
let mut bytes = vec![];
r.0.to_writer(&mut bytes)?;
r.to_writer(&mut bytes)?;
Rope::from(bytes)
}

Expand Down Expand Up @@ -391,10 +388,9 @@ impl SourceMap {
)
}
async fn regular_map_with_resolved_sources(
map: &RegularMapWrapper,
map: &RegularMap,
origin: Vc<FileSystemPath>,
) -> Result<RegularMap> {
let map = &map.0;
let file = map.get_file().map(Arc::<str>::from);
let tokens = map.tokens().map(|t| t.get_raw_token()).collect();
let names = map.names().map(Arc::<str>::from).collect();
Expand All @@ -420,12 +416,11 @@ impl SourceMap {
))
}
async fn decoded_map_with_resolved_sources(
map: &CrateMapWrapper,
map: &DecodedMap,
origin: Vc<FileSystemPath>,
) -> Result<CrateMapWrapper> {
Ok(CrateMapWrapper(match &map.0 {
) -> Result<DecodedMap> {
Ok(match &map {
DecodedMap::Regular(map) => {
let map = RegularMapWrapper::ref_cast(map);
DecodedMap::Regular(regular_map_with_resolved_sources(map, origin).await?)
}
DecodedMap::Index(map) => {
Expand All @@ -434,9 +429,7 @@ impl SourceMap {
let sections = map
.sections()
.filter_map(|section| {
section
.get_sourcemap()
.map(|s| (section.get_offset(), CrateMapWrapper::ref_cast(s)))
section.get_sourcemap().map(|s| (section.get_offset(), s))
})
.collect::<Vec<_>>();
let sections = sections
Expand All @@ -455,20 +448,20 @@ impl SourceMap {
offset,
// Urls are deprecated and we don't accept them
None,
Some(map.0),
Some(map),
));
}
DecodedMap::Index(SourceMapIndex::new(file, new_sections))
}
DecodedMap::Hermes(_) => {
todo!("hermes source maps are not implemented");
}
}))
})
}
Ok(match &*self.await? {
Self::Decoded(m) => {
let map = Box::pin(decoded_map_with_resolved_sources(&m.map, origin)).await?;
Self::Decoded(InnerSourceMap::new(map.0))
Self::Decoded(InnerSourceMap::new(map))
}
Self::Sectioned(m) => {
let mut sections = Vec::with_capacity(m.sections.len());
Expand Down Expand Up @@ -511,7 +504,7 @@ impl SourceMap {
..
}) = &mut token
{
if let DecodedMap::Regular(map) = &map.map.0 {
if let DecodedMap::Regular(map) = &*map.map {
if map.get_source_count() == 1 {
let source = map.sources().next().unwrap();
*guessed_original_file = Some(source.to_string());
Expand All @@ -520,7 +513,7 @@ impl SourceMap {
}

if need_source_content && content.is_none() {
if let Some(map) = map.map.as_regular_source_map() {
if let Some(map) = as_regular_source_map(&map.map) {
content = tok.and_then(|tok| {
let src_id = tok.get_src_id();

Expand Down Expand Up @@ -606,22 +599,21 @@ impl GenerateSourceMap for SourceMap {
}
}

/// A regular source map covers an entire file.
/// A regular source map covers an entire file. This wraps the sourcemap crate's struct to provide
/// Eq, PartialEq, and references via an Arc.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct InnerSourceMap {
map: Arc<CrateMapWrapper>,
map: Arc<DecodedMap>,
}

impl InnerSourceMap {
pub fn new(map: DecodedMap) -> Self {
InnerSourceMap {
map: Arc::new(CrateMapWrapper(map)),
}
InnerSourceMap { map: Arc::new(map) }
}
}

impl Deref for InnerSourceMap {
type Target = Arc<CrateMapWrapper>;
type Target = Arc<DecodedMap>;

fn deref(&self) -> &Self::Target {
&self.map
Expand All @@ -635,34 +627,6 @@ impl PartialEq for InnerSourceMap {
}
}

/// Wraps the DecodedMap struct so that it is Sync and Send.
///
/// # Safety
///
/// Must not use per line access to the SourceMap, as it is not thread safe.
#[derive(Debug, RefCast)]
#[repr(transparent)]
pub struct CrateMapWrapper(DecodedMap);

// Safety: DecodedMap contains a raw pointer, which isn't Send, which is
// required to cache in a Vc. So, we have wrap it in 4 layers of cruft to do it.
unsafe impl Send for CrateMapWrapper {}
unsafe impl Sync for CrateMapWrapper {}

/// Wraps the RegularMap struct so that it is Sync and Send.
///
/// # Safety
///
/// Must not use per line access to the SourceMap, as it is not thread safe.
#[derive(Debug, RefCast)]
#[repr(transparent)]
pub struct RegularMapWrapper(RegularMap);

// Safety: RegularMap contains a raw pointer, which isn't Send, which is
// required to cache in a Vc. So, we have wrap it in 4 layers of cruft to do it.
unsafe impl Send for RegularMapWrapper {}
unsafe impl Sync for RegularMapWrapper {}

#[derive(Debug)]
pub struct CrateIndexWrapper {
pub sections: Vec<CrateSectionWrapper>,
Expand All @@ -671,45 +635,16 @@ pub struct CrateIndexWrapper {
#[derive(Debug)]
pub struct CrateSectionWrapper {
pub offset: SourcePos,
pub map: Arc<CrateMapWrapper>,
pub map: Arc<DecodedMap>,
}

impl CrateMapWrapper {
pub fn as_regular_source_map(&self) -> Option<Cow<'_, RegularMap>> {
match &self.0 {
DecodedMap::Regular(m) => Some(Cow::Borrowed(m)),
DecodedMap::Index(m) => m.flatten().map(Cow::Owned).ok(),
_ => None,
}
pub fn as_regular_source_map(decoded_map: &DecodedMap) -> Option<Cow<'_, RegularMap>> {
match decoded_map {
DecodedMap::Regular(m) => Some(Cow::Borrowed(m)),
DecodedMap::Index(m) => m.flatten().map(Cow::Owned).ok(),
_ => None,
}
}

impl Deref for CrateMapWrapper {
type Target = DecodedMap;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl Serialize for CrateMapWrapper {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
use serde::ser::Error;
let mut bytes = vec![];
self.0.to_writer(&mut bytes).map_err(Error::custom)?;
serializer.serialize_bytes(bytes.as_slice())
}
}

impl<'de> Deserialize<'de> for CrateMapWrapper {
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
use serde::de::Error;
let bytes = <&[u8]>::deserialize(deserializer)?;
let map = DecodedMap::from_reader(bytes).map_err(Error::custom)?;
Ok(CrateMapWrapper(map))
}
}

/// A sectioned source map contains many (possibly recursive) maps covering
/// different regions of the file.
#[derive(Eq, PartialEq, Debug, Serialize, Deserialize)]
Expand Down
4 changes: 2 additions & 2 deletions turbopack/crates/turbopack-ecmascript/src/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use turbopack_core::{
error::PrettyPrintError,
issue::{Issue, IssueExt, IssueSeverity, IssueStage, OptionStyledString, StyledString},
source::Source,
source_map::{GenerateSourceMap, OptionSourceMap, SourceMap},
source_map::{as_regular_source_map, GenerateSourceMap, OptionSourceMap, SourceMap},
SOURCE_MAP_PREFIX,
};
use turbopack_swc_utils::emitter::IssueEmitter;
Expand Down Expand Up @@ -119,7 +119,7 @@ impl GenerateSourceMap for ParseResultSourceMap {
None
};
let input_map = if let Some(map) = original_src_map.as_ref() {
map.as_regular_source_map()
as_regular_source_map(map)
} else {
None
};
Expand Down
Loading