Skip to content

Commit

Permalink
Mark Vcs as must-use (#5387)
Browse files Browse the repository at this point in the history
This uses the `#[must-use]` macro [0] to mark Vcs as needing to be used
when created. The compiler creates warnings for each case.

Test Plan: Create a Vc in code and verify a warning is shown when
Turbopack is built.

[0]
https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
  • Loading branch information
wbinnssmith authored Jun 30, 2023
1 parent 87e1835 commit c51c7ab
Show file tree
Hide file tree
Showing 16 changed files with 79 additions and 59 deletions.
3 changes: 2 additions & 1 deletion crates/node-file-trace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,8 @@ async fn run<B: Backend + 'static, F: Future<Output = ()>>(
let console_ui = ConsoleUiVc::new(log_options);
console_ui
.as_issue_reporter()
.report_issues(TransientInstance::new(issues), source);
.report_issues(TransientInstance::new(issues), source)
.await?;

if has_return_value {
let output_read_ref = output.await?;
Expand Down
6 changes: 3 additions & 3 deletions crates/turbo-tasks-fs/examples/hash_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ async fn main() -> Result<()> {
let fs: FileSystemVc = disk_fs.into();
let input = fs.root().join("demo");
let dir_hash = hash_directory(input);
print_hash(dir_hash);
print_hash(dir_hash).await?;
Ok(NothingVc::new().into())
})
});
Expand All @@ -57,9 +57,9 @@ async fn main() -> Result<()> {
}

#[turbo_tasks::function]
async fn print_hash(dir_hash: StringVc) -> Result<()> {
async fn print_hash(dir_hash: StringVc) -> Result<NothingVc> {
println!("DIR HASH: {}", dir_hash.await?.as_str());
Ok(())
Ok(NothingVc::new())
}

async fn filename(path: FileSystemPathVc) -> Result<String> {
Expand Down
6 changes: 3 additions & 3 deletions crates/turbo-tasks-fs/examples/hash_glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ async fn main() -> Result<()> {
let glob = GlobVc::new("**/*.rs");
let glob_result = input.read_glob(glob, true);
let dir_hash = hash_glob_result(glob_result);
print_hash(dir_hash);
print_hash(dir_hash).await?;
Ok(NothingVc::new().into())
})
});
Expand All @@ -61,9 +61,9 @@ pub fn empty_string() -> StringVc {
}

#[turbo_tasks::function]
async fn print_hash(dir_hash: StringVc) -> Result<()> {
async fn print_hash(dir_hash: StringVc) -> Result<NothingVc> {
println!("DIR HASH: {}", dir_hash.await?.as_str());
Ok(())
Ok(NothingVc::new())
}

#[turbo_tasks::function]
Expand Down
27 changes: 15 additions & 12 deletions crates/turbo-tasks-macros/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,26 +208,29 @@ pub fn gen_native_function_code(

pub fn split_signature(sig: &Signature) -> (Signature, Signature, Type, TokenStream2) {
let output_type = get_return_type(&sig.output);
let (raw_output_type, _) = unwrap_result_type(&output_type);
if is_empty_type(raw_output_type) {
// Can't emit a diagnostic on OutputType alone as it can be omitted from the
// text for `()`. Use the whole signature span.
abort!(
sig.span(),
"Cannot return `()` from a turbo_tasks function. Return a NothingVc instead.",
)
}

let inline_ident = get_internal_function_ident(&sig.ident);
let mut inline_sig = sig.clone();
inline_sig.ident = inline_ident;

let mut external_sig = sig.clone();
external_sig.asyncness = None;

let (raw_output_type, _) = unwrap_result_type(&output_type);

let convert_result_code = if is_empty_type(raw_output_type) {
external_sig.output = ReturnType::Default;
quote! {}
} else {
external_sig.output = ReturnType::Type(
Token![->](raw_output_type.span()),
Box::new(raw_output_type.clone()),
);
quote! { std::convert::From::<turbo_tasks::RawVc>::from(result) }
};
external_sig.output = ReturnType::Type(
Token![->](raw_output_type.span()),
Box::new(raw_output_type.clone()),
);

let convert_result_code = quote! { std::convert::From::<turbo_tasks::RawVc>::from(result) };
let custom_self_type = if let Some(FnArg::Typed(PatType {
pat: box Pat::Ident(PatIdent { ident, .. }),
..
Expand Down
1 change: 1 addition & 0 deletions crates/turbo-tasks-macros/src/function_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn function(_args: TokenStream, input: TokenStream) -> TokenStream {
);

quote! {
#[must_use]
#(#attrs)*
#vis #external_sig {
let result = turbo_tasks::dynamic_call(*#function_id_ident, vec![#(#input_raw_vc_arguments),*]);
Expand Down
1 change: 1 addition & 0 deletions crates/turbo-tasks-macros/src/value_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,7 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream {
///
/// A reference is equal to another reference when it points to the same thing. No resolving is applied on comparison.
#[derive(Clone, Copy, Debug, std::cmp::PartialOrd, std::cmp::Ord, std::hash::Hash, std::cmp::Eq, std::cmp::PartialEq, serde::Serialize, serde::Deserialize)]
#[must_use]
#vis struct #ref_ident {
node: turbo_tasks::RawVc,
}
Expand Down
1 change: 1 addition & 0 deletions crates/turbo-tasks-macros/src/value_trait_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
};

let expanded = quote! {
#[must_use]
#(#attrs)*
#vis #trait_token #ident #colon_token #(#supertraits)+* #where_clause {
#(#items)*
Expand Down
20 changes: 11 additions & 9 deletions crates/turbo-tasks-memory/tests/collectibles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use std::{collections::HashSet, time::Duration};

use anyhow::{bail, Result};
use tokio::time::sleep;
use turbo_tasks::{emit, primitives::StringVc, CollectiblesSource, ValueToString, ValueToStringVc};
use turbo_tasks::{
emit, primitives::StringVc, CollectiblesSource, NothingVc, ValueToString, ValueToStringVc,
};
use turbo_tasks_testing::{register, run};
register!();

Expand Down Expand Up @@ -124,16 +126,16 @@ async fn my_collecting_function_indirect() -> Result<ThingVc> {
}

#[turbo_tasks::function]
fn my_multi_emitting_function() -> ThingVc {
my_transitive_emitting_function("", "a");
my_transitive_emitting_function("", "b");
my_emitting_function("");
ThingVc::cell(Thing(0))
async fn my_multi_emitting_function() -> Result<ThingVc> {
let _ = my_transitive_emitting_function("", "a");
let _ = my_transitive_emitting_function("", "b");
let _ = my_emitting_function("");
Ok(ThingVc::cell(Thing(0)))
}

#[turbo_tasks::function]
fn my_transitive_emitting_function(key: &str, _key2: &str) -> ThingVc {
my_emitting_function(key);
let _ = my_emitting_function(key);
ThingVc::cell(Thing(0))
}

Expand All @@ -150,11 +152,11 @@ async fn my_transitive_emitting_function_with_child_scope(
}

#[turbo_tasks::function]
async fn my_emitting_function(_key: &str) -> Result<()> {
async fn my_emitting_function(_key: &str) -> Result<NothingVc> {
sleep(Duration::from_millis(100)).await;
emit(ThingVc::new(123).as_value_to_string());
emit(ThingVc::new(42).as_value_to_string());
Ok(())
Ok(NothingVc::new())
}

#[turbo_tasks::function]
Expand Down
2 changes: 1 addition & 1 deletion crates/turbo-tasks/src/nothing.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{self as turbo_tasks};

/// Just an empty type.
/// [NothingVc] can be used as return value instead of `()`
/// [NothingVc] should be used as return value instead of `()`
/// to have a concrete reference that can be awaited.
#[turbo_tasks::value]
pub struct Nothing;
Expand Down
10 changes: 5 additions & 5 deletions crates/turbopack-core/src/resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,14 @@ impl ResolveResult {
#[turbo_tasks::value_impl]
impl ResolveResultVc {
#[turbo_tasks::function]
pub async fn add_reference(self, reference: AssetReferenceVc) -> Result<Self> {
pub async fn with_reference(self, reference: AssetReferenceVc) -> Result<Self> {
let mut this = self.await?.clone_value();
this.add_reference(reference);
Ok(this.into())
}

#[turbo_tasks::function]
pub async fn add_references(self, references: Vec<AssetReferenceVc>) -> Result<Self> {
pub async fn with_references(self, references: Vec<AssetReferenceVc>) -> Result<Self> {
let mut this = self.await?.clone_value();
for reference in references {
this.add_reference(reference);
Expand Down Expand Up @@ -279,7 +279,7 @@ impl ResolveResultVc {
.into_iter()
.next()
.unwrap()
.add_references(references));
.with_references(references));
}
let mut iter = results.into_iter().try_join().await?.into_iter();
if let Some(current) = iter.next() {
Expand Down Expand Up @@ -577,7 +577,7 @@ fn merge_results_with_references(
.into_iter()
.next()
.unwrap()
.add_references(references),
.with_references(references),
_ => ResolveResultVc::alternatives_with_references(results, references),
}
}
Expand Down Expand Up @@ -1170,7 +1170,7 @@ async fn resolve_alias_field_result(
RequestVc::parse(Value::new(Pattern::Constant(value.to_string()))),
resolve_options,
)
.add_references(refs));
.with_references(refs));
}
let issue: ResolvingIssueVc = ResolvingIssue {
severity: IssueSeverity::Error.cell(),
Expand Down
9 changes: 6 additions & 3 deletions crates/turbopack-dev-server/src/update/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use tokio::sync::mpsc::Sender;
use tokio_stream::wrappers::ReceiverStream;
use tracing::Instrument;
use turbo_tasks::{
primitives::StringVc, CollectiblesSource, IntoTraitRef, State, TraitRef, TransientInstance,
primitives::StringVc, CollectiblesSource, IntoTraitRef, NothingVc, State, TraitRef,
TransientInstance,
};
use turbo_tasks_fs::{FileSystem, FileSystemPathVc};
use turbopack_core::{
Expand Down Expand Up @@ -168,13 +169,15 @@ async fn compute_update_stream(
from: VersionStateVc,
get_content: TransientInstance<GetContentFn>,
sender: TransientInstance<Sender<Result<UpdateStreamItemReadRef>>>,
) {
) -> Result<NothingVc> {
let item = get_update_stream_item(resource, from, get_content)
.strongly_consistent()
.await;

// Send update. Ignore channel closed error.
let _ = sender.send(item).await;

Ok(NothingVc::new())
}

#[turbo_tasks::value]
Expand Down Expand Up @@ -231,7 +234,7 @@ impl UpdateStream {
};
let version_state = VersionStateVc::new(version.into_trait_ref().await?).await?;

compute_update_stream(
let _ = compute_update_stream(
&resource,
version_state,
get_content,
Expand Down
4 changes: 2 additions & 2 deletions crates/turbopack-ecmascript/src/resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ pub async fn url_resolve(
let result = if *rel_result.is_unresolveable().await? && rel_request.resolve().await? != request
{
resolve(origin.origin_path().parent(), request, resolve_options)
.add_references(rel_result.await?.get_references().clone())
.with_references(rel_result.await?.get_references().clone())
} else {
rel_result
};
handle_resolve_error(
let _ = handle_resolve_error(
result,
ty.clone(),
origin.origin_path(),
Expand Down
14 changes: 8 additions & 6 deletions crates/turbopack-node/src/evaluate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use turbo_tasks::{
duration_span, mark_finished,
primitives::{JsonValueVc, StringVc},
util::SharedError,
CompletionVc, RawVc, TryJoinIterExt, Value, ValueToString,
CompletionVc, NothingVc, RawVc, TryJoinIterExt, Value, ValueToString,
};
use turbo_tasks_bytes::{Bytes, Stream};
use turbo_tasks_env::{ProcessEnv, ProcessEnvVc};
Expand Down Expand Up @@ -252,7 +252,7 @@ pub fn evaluate(
let initial = Mutex::new(Some(sender));

// run the evaluation as side effect
compute_evaluate_stream(
let _ = compute_evaluate_stream(
module_asset,
cwd,
env,
Expand Down Expand Up @@ -299,11 +299,11 @@ async fn compute_evaluate_stream(
additional_invalidation: CompletionVc,
debug: bool,
sender: JavaScriptStreamSenderVc,
) {
) -> Result<NothingVc> {
mark_finished();
let Ok(sender) = sender.await else {
// Impossible to handle the error in a good way.
return;
return Ok(NothingVc::new());
};

let stream = generator! {
Expand Down Expand Up @@ -381,12 +381,14 @@ async fn compute_evaluate_stream(
pin_mut!(stream);
while let Some(value) = stream.next().await {
if sender.send(value).await.is_err() {
return;
return Ok(NothingVc::new());
}
if sender.flush().await.is_err() {
return;
return Ok(NothingVc::new());
}
}

Ok(NothingVc::new())
}

/// Repeatedly pulls from the NodeJsOperation until we receive a
Expand Down
15 changes: 9 additions & 6 deletions crates/turbopack-node/src/render/render_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use futures::{
};
use parking_lot::Mutex;
use turbo_tasks::{
duration_span, mark_finished, primitives::StringVc, util::SharedError, RawVc, ValueToString,
duration_span, mark_finished, primitives::StringVc, util::SharedError, NothingVc, RawVc,
ValueToString,
};
use turbo_tasks_bytes::{Bytes, Stream};
use turbo_tasks_env::ProcessEnvVc;
Expand Down Expand Up @@ -175,7 +176,7 @@ fn render_stream(
let initial = Mutex::new(Some(sender));

// run the evaluation as side effect
render_stream_internal(
let _ = render_stream_internal(
cwd,
env,
path,
Expand Down Expand Up @@ -223,11 +224,11 @@ async fn render_stream_internal(
body: BodyVc,
sender: RenderStreamSenderVc,
debug: bool,
) {
) -> Result<NothingVc> {
mark_finished();
let Ok(sender) = sender.await else {
// Impossible to handle the error in a good way.
return;
return Ok(NothingVc::new());
};

let stream = generator! {
Expand Down Expand Up @@ -329,10 +330,12 @@ async fn render_stream_internal(
pin_mut!(stream);
while let Some(value) = stream.next().await {
if sender.send(value).await.is_err() {
return;
return Ok(NothingVc::new());
}
if sender.flush().await.is_err() {
return;
return Ok(NothingVc::new());
}
}

Ok(NothingVc::new())
}
Loading

0 comments on commit c51c7ab

Please sign in to comment.