diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs index b069bcac9..0aee674b6 100644 --- a/components/salsa-macro-rules/src/setup_interned_struct.rs +++ b/components/salsa-macro-rules/src/setup_interned_struct.rs @@ -78,6 +78,8 @@ macro_rules! setup_interned_struct { // The path to the `serialize` function for the value's fields. deserialize_fn: $($deserialize_fn:path)?, + assert_types_are_update: {$($assert_types_are_update:tt)*}, + // Annoyingly macro-rules hygiene does not extend to items defined in the macro. // We have the procedural macro generate names for those items that are // not used elsewhere in the user's code. @@ -291,6 +293,8 @@ macro_rules! setup_interned_struct { unsafe impl< $($db_lt_arg)? > $zalsa::Update for $Struct< $($db_lt_arg)? > { unsafe fn maybe_update(old_pointer: *mut Self, new_value: Self) -> bool { + $($assert_types_are_update)* + if unsafe { *old_pointer } != new_value { unsafe { *old_pointer = new_value }; true diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index e1619095f..97ef7177d 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/components/salsa-macro-rules/src/setup_tracked_fn.rs @@ -67,7 +67,7 @@ macro_rules! setup_tracked_fn { // If true, the input and output values implement `serde::{Serialize, Deserialize}`. persist: $persist:tt, - assert_return_type_is_update: {$($assert_return_type_is_update:tt)*}, + assert_types_are_update: {$($assert_types_are_update:tt)*}, $(self_ty: $self_ty:ty,)? @@ -295,7 +295,7 @@ macro_rules! setup_tracked_fn { )? fn execute<$db_lt>($db: &$db_lt Self::DbView, ($($input_id),*): ($($interned_input_ty),*)) -> Self::Output<$db_lt> { - $($assert_return_type_is_update)* + $($assert_types_are_update)* $($inner_fn)* diff --git a/components/salsa-macros/src/accumulator.rs b/components/salsa-macros/src/accumulator.rs index 1c443a688..abc694809 100644 --- a/components/salsa-macros/src/accumulator.rs +++ b/components/salsa-macros/src/accumulator.rs @@ -33,7 +33,7 @@ impl AllowedOptions for Accumulator { const SPECIFY: bool = false; const NO_EQ: bool = false; const DEBUG: bool = false; - const NON_UPDATE_RETURN_TYPE: bool = false; + const NON_UPDATE_TYPES: bool = false; const NO_LIFETIME: bool = false; const SINGLETON: bool = false; const DATA: bool = false; diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index 9b4901419..6449f0316 100644 --- a/components/salsa-macros/src/input.rs +++ b/components/salsa-macros/src/input.rs @@ -43,7 +43,7 @@ impl AllowedOptions for InputStruct { const NO_LIFETIME: bool = false; - const NON_UPDATE_RETURN_TYPE: bool = false; + const NON_UPDATE_TYPES: bool = false; const SINGLETON: bool = true; diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index 8b72b3510..86c244ed8 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -43,7 +43,7 @@ impl AllowedOptions for InternedStruct { const NO_LIFETIME: bool = true; - const NON_UPDATE_RETURN_TYPE: bool = false; + const NON_UPDATE_TYPES: bool = false; const SINGLETON: bool = true; @@ -145,6 +145,12 @@ impl Macro { let CACHE = self.hygiene.ident("CACHE"); let Db = self.hygiene.ident("Db"); + let assert_types_are_update = if self.args.non_update_types.is_none() { + crate::update::assert_update(&db_lt, &zalsa, field_tys.iter().map(|ty| (**ty).clone())) + } else { + quote! {} + }; + Ok(crate::debug::dump_tokens( struct_ident, quote! { @@ -173,6 +179,7 @@ impl Macro { persist: #persist, serialize_fn: #(#serialize_fn)*, deserialize_fn: #(#deserialize_fn)*, + assert_types_are_update: { #assert_types_are_update }, unused_names: [ #zalsa, #zalsa_struct, diff --git a/components/salsa-macros/src/options.rs b/components/salsa-macros/src/options.rs index b7bdc807b..c73e4e485 100644 --- a/components/salsa-macros/src/options.rs +++ b/components/salsa-macros/src/options.rs @@ -43,12 +43,13 @@ pub(crate) struct Options { /// If this is `Some`, the value is the `specify` identifier. pub specify: Option, - /// The `non_update_return_type` option is used to signal that a tracked function's - /// return type does not require `Update` to be implemented. This is unsafe and - /// generally discouraged as it allows for dangling references. + /// The `non_update_types` option is used to signal that a tracked function's + /// return type or interned parameter, or an interned struct's fields does not + /// require `Update` to be implemented. This is unsafe and generally discouraged + /// as it allows for dangling references. /// - /// If this is `Some`, the value is the `non_update_return_type` identifier. - pub non_update_return_type: Option, + /// If this is `Some`, the value is the `non_update_types` identifier. + pub non_update_types: Option, /// The `persist` options indicates that the ingredient should be persisted with the database. /// @@ -139,7 +140,7 @@ impl Default for Options { Self { returns: Default::default(), specify: Default::default(), - non_update_return_type: Default::default(), + non_update_types: Default::default(), no_eq: Default::default(), debug: Default::default(), no_lifetime: Default::default(), @@ -168,7 +169,7 @@ pub(crate) trait AllowedOptions { const NO_EQ: bool; const DEBUG: bool; const NO_LIFETIME: bool; - const NON_UPDATE_RETURN_TYPE: bool; + const NON_UPDATE_TYPES: bool; const SINGLETON: bool; const DATA: bool; const DB: bool; @@ -263,12 +264,12 @@ impl syn::parse::Parse for Options { )); } } else if ident == "unsafe" { - if A::NON_UPDATE_RETURN_TYPE { + if A::NON_UPDATE_TYPES { let content; parenthesized!(content in input); let ident = syn::Ident::parse_any(&content)?; if ident == "non_update_return_type" { - if let Some(old) = options.non_update_return_type.replace(ident) { + if let Some(old) = options.non_update_types.replace(ident) { return Err(syn::Error::new( old.span(), "option `non_update_return_type` provided twice", @@ -562,7 +563,7 @@ impl quote::ToTokens for Options { no_lifetime, singleton, specify, - non_update_return_type, + non_update_types, db_path, cycle_fn, cycle_initial, @@ -595,8 +596,8 @@ impl quote::ToTokens for Options { if specify.is_some() { tokens.extend(quote::quote! { specify, }); } - if non_update_return_type.is_some() { - tokens.extend(quote::quote! { unsafe(non_update_return_type), }); + if non_update_types.is_some() { + tokens.extend(quote::quote! { unsafe(non_update_types), }); } if let Some(db_path) = db_path { tokens.extend(quote::quote! { db = #db_path, }); diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index 12f9170c7..ad43c106f 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/components/salsa-macros/src/tracked_fn.rs @@ -36,7 +36,7 @@ impl AllowedOptions for TrackedFn { const NO_LIFETIME: bool = false; - const NON_UPDATE_RETURN_TYPE: bool = true; + const NON_UPDATE_TYPES: bool = true; const SINGLETON: bool = false; @@ -102,7 +102,7 @@ impl Macro { let (cycle_recovery_fn, cycle_recovery_initial, cycle_recovery_strategy) = self.cycle_recovery()?; let is_specifiable = self.args.specify.is_some(); - let requires_update = self.args.non_update_return_type.is_none(); + let requires_update = self.args.non_update_types.is_none(); let heap_size_fn = self.args.heap_size_fn.iter(); let eq = if let Some(token) = &self.args.no_eq { if self.args.cycle_fn.is_some() { @@ -187,21 +187,12 @@ impl Macro { let persist = self.args.persist(); - // The path expression is responsible for emitting the primary span in the diagnostic we - // want, so by uniformly using `output_ty.span()` we ensure that the diagnostic is emitted - // at the return type in the original input. - // See the tests/compile-fail/tracked_fn_return_ref.rs test - let maybe_update_path = quote_spanned! {output_ty.span() => - UpdateDispatch::<#output_ty>::maybe_update - }; - let assert_return_type_is_update = if requires_update { - quote! { - #[allow(clippy::all, warnings)] - fn _assert_return_type_is_update<#db_lt>() { - use #zalsa::{UpdateFallback, UpdateDispatch}; - #maybe_update_path; - } + let assert_types_are_update = if requires_update { + let mut assert_update = vec![output_ty.clone()]; + if needs_interner { + assert_update.extend(interned_input_tys.clone()); } + crate::update::assert_update(&db_lt, &zalsa, assert_update) } else { quote! {} }; @@ -234,7 +225,7 @@ impl Macro { lru: #lru, return_mode: #return_mode, persist: #persist, - assert_return_type_is_update: { #assert_return_type_is_update }, + assert_types_are_update: { #assert_types_are_update }, #self_ty unused_names: [ #zalsa, diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs index d6ee32d47..8afd5a8dd 100644 --- a/components/salsa-macros/src/tracked_struct.rs +++ b/components/salsa-macros/src/tracked_struct.rs @@ -39,7 +39,7 @@ impl AllowedOptions for TrackedStruct { const NO_LIFETIME: bool = false; - const NON_UPDATE_RETURN_TYPE: bool = false; + const NON_UPDATE_TYPES: bool = false; const SINGLETON: bool = true; diff --git a/components/salsa-macros/src/update.rs b/components/salsa-macros/src/update.rs index 6973054ec..2931893ca 100644 --- a/components/salsa-macros/src/update.rs +++ b/components/salsa-macros/src/update.rs @@ -147,3 +147,26 @@ pub(crate) fn update_derive(input: syn::DeriveInput) -> syn::Result Ok(crate::debug::dump_tokens(&input.ident, tokens)) } + +pub(crate) fn assert_update( + db_lt: &syn::Lifetime, + zalsa: &syn::Ident, + types: impl IntoIterator, +) -> TokenStream { + // The path expression is responsible for emitting the primary span in the diagnostic we + // want, so by uniformly using `ty.span()` we ensure that the diagnostic is emitted + // at the type in the original input. + // See the tests/compile-fail/tracked_fn_return_ref.rs test + let maybe_update_paths = types.into_iter().map(|ty| { + quote_spanned! {ty.span() => + UpdateDispatch::<#ty>::maybe_update + } + }); + quote! { + #[allow(clippy::all, warnings)] + fn _assert_return_type_is_update<#db_lt>() { + use #zalsa::{UpdateFallback, UpdateDispatch}; + #( #maybe_update_paths; )* + } + } +} diff --git a/tests/compile-fail/interned_not_update.rs b/tests/compile-fail/interned_not_update.rs new file mode 100644 index 000000000..9b1556b61 --- /dev/null +++ b/tests/compile-fail/interned_not_update.rs @@ -0,0 +1,14 @@ +use salsa::Database as Db; + +#[salsa::input] +struct MyInput {} + +#[salsa::tracked] +fn tracked_fn<'db>(_db: &'db dyn Db, _: (), _: &'db str) {} + +#[salsa::interned] +struct Interned<'db> { + _field: &'db str, +} + +fn main() {} diff --git a/tests/compile-fail/interned_not_update.stderr b/tests/compile-fail/interned_not_update.stderr new file mode 100644 index 000000000..8aa885b36 --- /dev/null +++ b/tests/compile-fail/interned_not_update.stderr @@ -0,0 +1,13 @@ +error: lifetime may not live long enough + --> tests/compile-fail/interned_not_update.rs:7:48 + | +7 | fn tracked_fn<'db>(_db: &'db dyn Db, _: (), _: &'db str) {} + | --- lifetime `'db` defined here ^ requires that `'db` must outlive `'static` + +error: lifetime may not live long enough + --> tests/compile-fail/interned_not_update.rs:11:13 + | +10 | struct Interned<'db> { + | --- lifetime `'db` defined here +11 | _field: &'db str, + | ^ requires that `'db` must outlive `'static`