Require interned structs' fields to be Update

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

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)*
 

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;

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;
 

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,

components/salsa-macros/src/options.rs

@@ -43,12 +43,13 @@ pub(crate) struct Options<A: AllowedOptions> {
     /// If this is `Some`, the value is the `specify` identifier.
     pub specify: Option<syn::Ident>,
 
-    /// 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<syn::Ident>,
+    /// If this is `Some`, the value is the `non_update_types` identifier.
+    pub non_update_types: Option<syn::Ident>,
 
     /// The `persist` options indicates that the ingredient should be persisted with the database.
     ///
@@ -139,7 +140,7 @@ impl<A: AllowedOptions> Default for Options<A> {
         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<A: AllowedOptions> syn::parse::Parse for Options<A> {
                     ));
                 }
             } 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<A: AllowedOptions> quote::ToTokens for Options<A> {
             no_lifetime,
             singleton,
             specify,
-            non_update_return_type,
+            non_update_types,
             db_path,
             cycle_fn,
             cycle_initial,
@@ -595,8 +596,8 @@ impl<A: AllowedOptions> quote::ToTokens for Options<A> {
         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, });

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,

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;
 

components/salsa-macros/src/update.rs

@@ -147,3 +147,26 @@ pub(crate) fn update_derive(input: syn::DeriveInput) -> syn::Result<TokenStream>
 
     Ok(crate::debug::dump_tokens(&input.ident, tokens))
 }
+
+pub(crate) fn assert_update(
+    db_lt: &syn::Lifetime,
+    zalsa: &syn::Ident,
+    types: impl IntoIterator<Item = syn::Type>,
+) -> 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; )*
+        }
+    }
+}

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() {}

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`