From bc8119a800535d247ab85162498f7d22af696f67 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Thu, 11 Jul 2024 16:54:19 -0700 Subject: [PATCH] Add support for `#[turbo_tasks::value(resolved)]` and `#[turbo_tasks::value_trait(resolved)]` (vercel/turbo#8720) ### Description - `#[turbo_tasks::value(resolved)]`: This is essentially just shorthand for also doing `#[derive(ResolvedValue)]`. We'll want this on-by-default eventually, but it's strictly opt-in for now. - `#[turbo_tasks::value_trait(resolved)]`: This adds an additional supertrait for `ResolvedValue`. This wasn't quite possible to do without this flag as most supertraits are required to implement `VcValueTrait` because they're used with the `Upcast`/`Downcast` traits. ### Testing Instructions ``` TRYBUILD=overwrite cargo nextest r -p turbo-tasks-macros-tests ``` --- .../src/value_trait_arguments.rs | 14 +++++++- .../fail_contains_vc_inside_generic.stderr | 4 +-- .../tests/trybuild.rs | 14 ++++++++ .../tests/value/fail_resolved.rs | 13 +++++++ .../tests/value/fail_resolved.stderr | 22 ++++++++++++ .../tests/value/pass_resolved.rs | 14 ++++++++ .../tests/value_trait/pass_resolved.rs | 10 ++++++ .../src/derive/resolved_value_macro.rs | 6 ++-- .../src/derive/value_macro.rs | 1 - crates/turbo-tasks-macros/src/lib.rs | 25 +++++++++++++ crates/turbo-tasks-macros/src/value_macro.rs | 36 ++++++++++++++++--- .../src/value_trait_macro.rs | 18 +++++++--- crates/turbo-tasks/src/macro_helpers.rs | 1 + 13 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 crates/turbo-tasks-macros-tests/tests/value/fail_resolved.rs create mode 100644 crates/turbo-tasks-macros-tests/tests/value/fail_resolved.stderr create mode 100644 crates/turbo-tasks-macros-tests/tests/value/pass_resolved.rs create mode 100644 crates/turbo-tasks-macros-tests/tests/value_trait/pass_resolved.rs delete mode 100644 crates/turbo-tasks-macros/src/derive/value_macro.rs diff --git a/crates/turbo-tasks-macros-shared/src/value_trait_arguments.rs b/crates/turbo-tasks-macros-shared/src/value_trait_arguments.rs index e16a7a5159d49..a28b4e45d81fc 100644 --- a/crates/turbo-tasks-macros-shared/src/value_trait_arguments.rs +++ b/crates/turbo-tasks-macros-shared/src/value_trait_arguments.rs @@ -1,6 +1,8 @@ +use proc_macro2::Span; use syn::{ parse::{Parse, ParseStream}, punctuated::Punctuated, + spanned::Spanned, Meta, Token, }; @@ -10,11 +12,18 @@ pub struct ValueTraitArguments { /// Whether the macro should generate a `ValueDebug`-like `dbg()` /// implementation on the trait's `Vc`. pub debug: bool, + /// Should the trait have a `turbo_tasks::ResolvedValue` constraint? + /// + /// `Some(...)` if enabled, containing the span that enabled the constraint. + pub resolved: Option, } impl Default for ValueTraitArguments { fn default() -> Self { - Self { debug: true } + Self { + debug: true, + resolved: None, + } } } @@ -31,6 +40,9 @@ impl Parse for ValueTraitArguments { Some("no_debug") => { result.debug = false; } + Some("resolved") => { + result.resolved = Some(meta.span()); + } _ => { return Err(syn::Error::new_spanned(meta, "unknown parameter")); } diff --git a/crates/turbo-tasks-macros-tests/tests/derive_resolved_value/fail_contains_vc_inside_generic.stderr b/crates/turbo-tasks-macros-tests/tests/derive_resolved_value/fail_contains_vc_inside_generic.stderr index dc91f11ada319..5851763aa4698 100644 --- a/crates/turbo-tasks-macros-tests/tests/derive_resolved_value/fail_contains_vc_inside_generic.stderr +++ b/crates/turbo-tasks-macros-tests/tests/derive_resolved_value/fail_contains_vc_inside_generic.stderr @@ -2,7 +2,7 @@ error[E0277]: the trait bound `Vc: ResolvedValue` is not satisfied --> tests/derive_resolved_value/fail_contains_vc_inside_generic.rs:7:8 | 7 | a: Option; 4]>>, - | ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `ResolvedValue` is not implemented for `Vc`, which is required by `Option; 4]>>: ResolvedValue` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `ResolvedValue` is not implemented for `Vc`, which is required by `std::option::Option; 4]>>: ResolvedValue` | = help: the following other types implement trait `ResolvedValue`: &T @@ -16,7 +16,7 @@ error[E0277]: the trait bound `Vc: ResolvedValue` is not satisfied and $N others = note: required for `[Vc; 4]` to implement `ResolvedValue` = note: 2 redundant requirements hidden - = note: required for `Option; 4]>>` to implement `ResolvedValue` + = note: required for `std::option::Option; 4]>>` to implement `ResolvedValue` note: required by a bound in `DeriveResolvedValueAssertion::assert_impl_resolved_value` --> tests/derive_resolved_value/fail_contains_vc_inside_generic.rs:5:10 | diff --git a/crates/turbo-tasks-macros-tests/tests/trybuild.rs b/crates/turbo-tasks-macros-tests/tests/trybuild.rs index 749369094b88e..8620bbd901638 100644 --- a/crates/turbo-tasks-macros-tests/tests/trybuild.rs +++ b/crates/turbo-tasks-macros-tests/tests/trybuild.rs @@ -4,3 +4,17 @@ fn derive_resolved_value() { t.pass("tests/derive_resolved_value/pass_*.rs"); t.compile_fail("tests/derive_resolved_value/fail_*.rs"); } + +#[test] +fn value() { + let t = trybuild::TestCases::new(); + t.pass("tests/value/pass_*.rs"); + t.compile_fail("tests/value/fail_*.rs"); +} + +#[test] +fn value_trait() { + let t = trybuild::TestCases::new(); + t.pass("tests/value_trait/pass_*.rs"); + t.compile_fail("tests/value_trait/fail_*.rs"); +} diff --git a/crates/turbo-tasks-macros-tests/tests/value/fail_resolved.rs b/crates/turbo-tasks-macros-tests/tests/value/fail_resolved.rs new file mode 100644 index 0000000000000..0280713b8722c --- /dev/null +++ b/crates/turbo-tasks-macros-tests/tests/value/fail_resolved.rs @@ -0,0 +1,13 @@ +#![feature(arbitrary_self_types)] + +use turbo_tasks::Vc; + +#[turbo_tasks::value(resolved)] +struct MyValue { + value: Vc, +} + +fn main() { + let v = MyValue { value: Vc::cell(0) }; + let _ = v.value; +} diff --git a/crates/turbo-tasks-macros-tests/tests/value/fail_resolved.stderr b/crates/turbo-tasks-macros-tests/tests/value/fail_resolved.stderr new file mode 100644 index 0000000000000..d100c44fdc5b0 --- /dev/null +++ b/crates/turbo-tasks-macros-tests/tests/value/fail_resolved.stderr @@ -0,0 +1,22 @@ +error[E0277]: the trait bound `Vc: ResolvedValue` is not satisfied + --> tests/value/fail_resolved.rs:7:12 + | +7 | value: Vc, + | ^^^^^^^ the trait `ResolvedValue` is not implemented for `Vc` + | + = help: the following other types implement trait `ResolvedValue`: + &T + &mut T + () + (A, Z, Y, X, W, V, U, T) + (B, A, Z, Y, X, W, V, U, T) + (C, B, A, Z, Y, X, W, V, U, T) + (D, C, B, A, Z, Y, X, W, V, U, T) + (E, D, C, B, A, Z, Y, X, W, V, U, T) + and $N others +note: required by a bound in `DeriveResolvedValueAssertion::assert_impl_resolved_value` + --> tests/value/fail_resolved.rs:5:22 + | +5 | #[turbo_tasks::value(resolved)] + | ^^^^^^^^ required by this bound in `DeriveResolvedValueAssertion::assert_impl_resolved_value` + = note: this error originates in the derive macro `turbo_tasks::ResolvedValue` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/crates/turbo-tasks-macros-tests/tests/value/pass_resolved.rs b/crates/turbo-tasks-macros-tests/tests/value/pass_resolved.rs new file mode 100644 index 0000000000000..5fd5ea50638e8 --- /dev/null +++ b/crates/turbo-tasks-macros-tests/tests/value/pass_resolved.rs @@ -0,0 +1,14 @@ +#![feature(arbitrary_self_types)] + +#[turbo_tasks::value(resolved)] +struct MyValue { + value: i32, +} + +fn expects_resolved(value: T) {} + +fn main() { + let v = MyValue { value: 0 }; + expects_resolved(v); + let _ = v.value; +} diff --git a/crates/turbo-tasks-macros-tests/tests/value_trait/pass_resolved.rs b/crates/turbo-tasks-macros-tests/tests/value_trait/pass_resolved.rs new file mode 100644 index 0000000000000..dbdca31fb5add --- /dev/null +++ b/crates/turbo-tasks-macros-tests/tests/value_trait/pass_resolved.rs @@ -0,0 +1,10 @@ +#![feature(arbitrary_self_types)] + +#[turbo_tasks::value_trait(resolved)] +trait MyTrait {} + +fn expects_resolved() {} + +fn main() { + expects_resolved::<&dyn MyTrait>(); +} diff --git a/crates/turbo-tasks-macros/src/derive/resolved_value_macro.rs b/crates/turbo-tasks-macros/src/derive/resolved_value_macro.rs index 86ce9dca298fa..de703384b00b8 100644 --- a/crates/turbo-tasks-macros/src/derive/resolved_value_macro.rs +++ b/crates/turbo-tasks-macros/src/derive/resolved_value_macro.rs @@ -12,7 +12,7 @@ pub fn derive_resolved_value(input: TokenStream) -> TokenStream { let (impl_generics, ty_generics, where_clause) = derive_input.generics.split_for_impl(); quote! { - unsafe impl #impl_generics ::turbo_tasks::ResolvedValue + unsafe impl #impl_generics turbo_tasks::ResolvedValue for #ident #ty_generics #where_clause {} #assertions } @@ -47,7 +47,9 @@ fn assert_fields_impl_resolved_value(generics: &Generics, data: &Data) -> TokenS struct DeriveResolvedValueAssertion #impl_generics (#(#field_types),*) #where_clause; impl #impl_generics DeriveResolvedValueAssertion #ty_generics #where_clause { - fn assert_impl_resolved_value() {} + fn assert_impl_resolved_value< + ExpectedResolvedValue: turbo_tasks::ResolvedValue + ?Sized + >() {} fn field_types() { #(#assertion_calls)* } diff --git a/crates/turbo-tasks-macros/src/derive/value_macro.rs b/crates/turbo-tasks-macros/src/derive/value_macro.rs deleted file mode 100644 index 8b137891791fe..0000000000000 --- a/crates/turbo-tasks-macros/src/derive/value_macro.rs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/crates/turbo-tasks-macros/src/lib.rs b/crates/turbo-tasks-macros/src/lib.rs index c1ced476b8e28..b7a0e3bcca25c 100644 --- a/crates/turbo-tasks-macros/src/lib.rs +++ b/crates/turbo-tasks-macros/src/lib.rs @@ -119,6 +119,13 @@ pub fn derive_task_input(input: TokenStream) -> TokenStream { /// /// Example: `#[turbo_tasks::value(transparent)]` /// +/// ### `resolved` +/// +/// A shorthand syntax for +/// [`#[derive(turbo_tasks::ResolvedValue)]`][macro@turbo_tasks::ResolvedValue] +/// +/// Example: `#[turbo_tasks::value(resolved)]` +/// /// TODO: add more documentation: presets, traits #[allow_internal_unstable(min_specialization, into_future, trivial_bounds)] #[proc_macro_error] @@ -127,6 +134,24 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream { value_macro::value(args, input) } +/// Allows this trait to be used as part of a trait object inside of a value +/// cell, in the form of `Vc`. +/// +/// ## Arguments +/// +/// Example: `#[turbo_tasks::value_trait(no_debug, resolved)]` +/// +/// ### 'no_debug` +/// +/// Disables the automatic implementation of [`turbo_tasks::debug::ValueDebug`]. +/// +/// Example: `#[turbo_tasks::value_trait(no_debug)]` +/// +/// ### 'resolved` +/// +/// Adds [`turbo_tasks::ResolvedValue`] as a supertrait of this trait. +/// +/// Example: `#[turbo_tasks::value_trait(resolved)]` #[allow_internal_unstable(min_specialization, into_future, trivial_bounds)] #[proc_macro_error] #[proc_macro_attribute] diff --git a/crates/turbo-tasks-macros/src/value_macro.rs b/crates/turbo-tasks-macros/src/value_macro.rs index c4393b2e73cde..10155d62625cf 100644 --- a/crates/turbo-tasks-macros/src/value_macro.rs +++ b/crates/turbo-tasks-macros/src/value_macro.rs @@ -1,8 +1,8 @@ use std::sync::OnceLock; use proc_macro::TokenStream; -use proc_macro2::Ident; -use quote::{quote, ToTokens}; +use proc_macro2::{Ident, Span}; +use quote::{quote, quote_spanned, ToTokens}; use regex::Regex; use syn::{ parse::{Parse, ParseStream}, @@ -110,6 +110,10 @@ struct ValueArguments { cell_mode: CellMode, manual_eq: bool, transparent: bool, + /// Should we `#[derive(turbo_tasks::ResolvedValue)]`? + /// + /// `Some(...)` if enabled, containing the span that enabled the derive. + resolved: Option, } impl Parse for ValueArguments { @@ -119,6 +123,7 @@ impl Parse for ValueArguments { into_mode: IntoMode::None, cell_mode: CellMode::Shared, manual_eq: false, + resolved: None, transparent: false, }; let punctuated: Punctuated = input.parse_terminated(Meta::parse)?; @@ -174,6 +179,9 @@ impl Parse for ValueArguments { ("transparent", Meta::Path(_)) => { result.transparent = true; } + ("resolved", Meta::Path(path)) => { + result.resolved = Some(path.span()); + } (_, meta) => { return Err(Error::new_spanned( &meta, @@ -199,6 +207,7 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream { cell_mode, manual_eq, transparent, + resolved, } = parse_macro_input!(args as ValueArguments); let mut inner_type = None; @@ -328,7 +337,12 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream { } } SerializationMode::Auto | SerializationMode::AutoForInput => quote! { - #[derive(turbo_tasks::trace::TraceRawVcs, serde::Serialize, serde::Deserialize)] + #[derive( + turbo_tasks::trace::TraceRawVcs, + turbo_tasks::macro_helpers::serde::Serialize, + turbo_tasks::macro_helpers::serde::Deserialize, + )] + #[serde(crate = "turbo_tasks::macro_helpers::serde")] }, }; let debug_derive = if inner_type.is_some() { @@ -338,7 +352,10 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream { } } else { quote! { - #[derive(turbo_tasks::debug::ValueDebugFormat, turbo_tasks::debug::internal::ValueDebug)] + #[derive( + turbo_tasks::debug::ValueDebugFormat, + turbo_tasks::debug::internal::ValueDebug, + )] } }; let eq_derive = if manual_eq { @@ -348,6 +365,14 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream { #[derive(PartialEq, Eq)] ) }; + let resolved_derive = if let Some(span) = resolved { + quote_spanned!( + span => + #[derive(turbo_tasks::ResolvedValue)] + ) + } else { + quote!() + }; let new_value_type = match serialization_mode { SerializationMode::None => quote! { @@ -406,8 +431,9 @@ pub fn value(args: TokenStream, input: TokenStream) -> TokenStream { let expanded = quote! { #derive - #eq_derive #debug_derive + #eq_derive + #resolved_derive #item impl #ident { diff --git a/crates/turbo-tasks-macros/src/value_trait_macro.rs b/crates/turbo-tasks-macros/src/value_trait_macro.rs index 4e5657f4dd43e..23ee201369b1f 100644 --- a/crates/turbo-tasks-macros/src/value_trait_macro.rs +++ b/crates/turbo-tasks-macros/src/value_trait_macro.rs @@ -1,6 +1,6 @@ use proc_macro::TokenStream; use proc_macro2::{Ident, TokenStream as TokenStream2}; -use quote::quote; +use quote::{quote, quote_spanned}; use syn::{ parse_macro_input, parse_quote, spanned::Spanned, ExprPath, ItemTrait, TraitItem, TraitItemMethod, @@ -13,7 +13,7 @@ use turbo_tasks_macros_shared::{ use crate::func::{DefinitionContext, NativeFn, TurboFn}; pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream { - let ValueTraitArguments { debug } = parse_macro_input!(args as ValueTraitArguments); + let ValueTraitArguments { debug, resolved } = parse_macro_input!(args as ValueTraitArguments); let item = parse_macro_input!(input as ItemTrait); @@ -59,7 +59,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream { .emit(); } - let supertraits = supertraits.into_iter().collect::>(); + let supertraits = supertraits.iter().collect::>(); let trait_type_ident = get_trait_type_ident(trait_ident); let trait_type_id_ident = get_trait_type_id_ident(trait_ident); @@ -194,10 +194,18 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream { quote! {} }; + let mut extended_supertraits = Vec::new(); + if let Some(span) = resolved { + extended_supertraits.push(quote_spanned! { + span => turbo_tasks::ResolvedValue + }); + } + extended_supertraits.push(quote!(::std::marker::Send)); + let expanded = quote! { #[must_use] #(#attrs)* - #vis #trait_token #trait_ident: std::marker::Send + #(#supertraits)+* + #vis #trait_token #trait_ident: #(#supertraits +)* #(#extended_supertraits +)* { #(#items)* } @@ -230,7 +238,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream { impl #trait_ident for T where - T: turbo_tasks::Dynamic> #(+ #supertraits)*, + T: turbo_tasks::Dynamic> + #(#supertraits +)* #(#extended_supertraits +)*, { #(#dynamic_trait_fns)* } diff --git a/crates/turbo-tasks/src/macro_helpers.rs b/crates/turbo-tasks/src/macro_helpers.rs index 80341228b3daf..3bd45451b8f27 100644 --- a/crates/turbo-tasks/src/macro_helpers.rs +++ b/crates/turbo-tasks/src/macro_helpers.rs @@ -1,5 +1,6 @@ //! Runtime helpers for [turbo-tasks-macro]. pub use once_cell::sync::{Lazy, OnceCell}; +pub use serde; pub use tracing; pub use super::manager::{find_cell_by_type, notify_scheduled_tasks, spawn_detached};