Skip to content

Commit

Permalink
fix equal checks for cells and output (#8721)
Browse files Browse the repository at this point in the history
### Description

* Check for equal Vc on returning a value
* Avoid PartialEq checks for values that are never equal

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
  • Loading branch information
sokra committed Jul 15, 2024
1 parent 81659d2 commit f5a1e7d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 36 deletions.
24 changes: 11 additions & 13 deletions crates/turbo-tasks-memory/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,17 @@ impl Output {

pub fn link(&mut self, target: RawVc, turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>) {
debug_assert!(*self != target);
self.assign(OutputContent::Link(target), turbo_tasks)
if let OutputContent::Link(old_target) = &self.content {
if *old_target == target {
// unchanged
return;
}
}
self.content = OutputContent::Link(target);
// notify
if !self.dependent_tasks.is_empty() {
turbo_tasks.schedule_notify_tasks_set(&take(&mut self.dependent_tasks));
}
}

pub fn error(&mut self, error: Error, turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>) {
Expand All @@ -79,18 +89,6 @@ impl Output {
}
}

pub fn assign(
&mut self,
content: OutputContent,
turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>,
) {
self.content = content;
// notify
if !self.dependent_tasks.is_empty() {
turbo_tasks.schedule_notify_tasks_set(&take(&mut self.dependent_tasks));
}
}

pub fn gc_drop(self, turbo_tasks: &dyn TurboTasksBackendApi<MemoryBackend>) {
// notify
if !self.dependent_tasks.is_empty() {
Expand Down
2 changes: 1 addition & 1 deletion crates/turbopack-core/src/source_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub trait GenerateSourceMap {
/// The distinction between the source map spec's [sourcemap::Index] and our
/// [SourceMap::Sectioned] is whether the sections are represented with Vcs
/// pointers.
#[turbo_tasks::value(shared)]
#[turbo_tasks::value(shared, cell = "new")]
pub enum SourceMap {
/// A decoded source map contains no Vcs.
Decoded(#[turbo_tasks(trace_ignore)] InnerSourceMap),
Expand Down
32 changes: 10 additions & 22 deletions crates/turbopack-css/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ impl<'i, 'o> StyleSheetLike<'i, 'o> {
#[turbo_tasks::value(transparent)]
pub struct UnresolvedUrlReferences(pub Vec<(String, Vc<UrlAssetReference>)>);

#[turbo_tasks::value(shared, serialization = "none", eq = "manual")]
#[turbo_tasks::value(shared, serialization = "none", eq = "manual", cell = "new")]
pub enum ParseCssResult {
Ok {
#[turbo_tasks(debug_ignore, trace_ignore)]
Expand All @@ -296,13 +296,7 @@ pub enum ParseCssResult {
NotFound,
}

impl PartialEq for ParseCssResult {
fn eq(&self, _: &Self) -> bool {
false
}
}

#[turbo_tasks::value(shared, serialization = "none", eq = "manual")]
#[turbo_tasks::value(shared, serialization = "none", eq = "manual", cell = "new")]
pub enum CssWithPlaceholderResult {
Ok {
parse_result: Vc<ParseCssResult>,
Expand All @@ -324,12 +318,6 @@ pub enum CssWithPlaceholderResult {
NotFound,
}

impl PartialEq for CssWithPlaceholderResult {
fn eq(&self, _: &Self) -> bool {
false
}
}

#[turbo_tasks::value(shared, serialization = "none", eq = "manual")]
pub enum FinalCssResult {
Ok {
Expand Down Expand Up @@ -390,10 +378,10 @@ pub async fn process_css_with_placeholder(
url_references: *url_references,
placeholders: HashMap::new(),
}
.into())
.cell())
}
ParseCssResult::Unparseable => Ok(CssWithPlaceholderResult::Unparseable.into()),
ParseCssResult::NotFound => Ok(CssWithPlaceholderResult::NotFound.into()),
ParseCssResult::Unparseable => Ok(CssWithPlaceholderResult::Unparseable.cell()),
ParseCssResult::NotFound => Ok(CssWithPlaceholderResult::NotFound.cell()),
}
}

Expand Down Expand Up @@ -602,7 +590,7 @@ async fn process_content(
}
.cell()
.emit();
return Ok(ParseCssResult::Unparseable.into());
return Ok(ParseCssResult::Unparseable.cell());
}

_ => {
Expand All @@ -629,7 +617,7 @@ async fn process_content(
}
.cell()
.emit();
return Ok(ParseCssResult::Unparseable.into());
return Ok(ParseCssResult::Unparseable.cell());
}
}
})
Expand Down Expand Up @@ -669,12 +657,12 @@ async fn process_content(
Ok(v) => v,
Err(err) => {
err.to_diagnostics(&handler).emit();
return Ok(ParseCssResult::Unparseable.into());
return Ok(ParseCssResult::Unparseable.cell());
}
};

if handler.has_errors() {
return Ok(ParseCssResult::Unparseable.into());
return Ok(ParseCssResult::Unparseable.cell());
}

if matches!(ty, CssModuleAssetType::Module) {
Expand Down Expand Up @@ -722,7 +710,7 @@ async fn process_content(
url_references: Vc::cell(url_references),
options: config,
}
.into())
.cell())
}

/// Visitor that lints wrong css module usage.
Expand Down

0 comments on commit f5a1e7d

Please sign in to comment.