made singleton struct panic upon duplication and added tests to immutable fields

components/salsa-2022-macros/src/input.rs

@@ -106,14 +106,11 @@ impl InputStruct {
 
         // setters
         let set_field_names = self.all_set_field_names();
-        let field_immuts = self.all_fields_immuts();
         let field_setters: Vec<syn::ImplItemMethod> = field_indices.iter()
             .zip(&set_field_names)
             .zip(&field_vises)
             .zip(&field_tys)
-            .zip(&field_immuts)
-            .filter(|(_, &is_immut )| !is_immut)
-            .map(|((((field_index, set_field_name), field_vis), field_ty), _)| {
+            .map(|(((field_index, set_field_name), field_vis), field_ty)| {
             parse_quote! {
                 #field_vis fn #set_field_name<'db>(self, __db: &'db mut #db_dyn_ty) -> salsa::setter::Setter<'db, #ident, #field_ty>
                 {
@@ -130,13 +127,18 @@ impl InputStruct {
 
         let constructor: syn::ImplItemMethod = if singleton {
             parse_quote! {
-                pub fn #constructor_name(__db: &mut #db_dyn_ty, #(#field_names: #field_tys,)*) -> Self
+                /// Creates a new singleton input
+                ///
+                /// # Panics
+                ///
+                /// If called when an instance already exists
+                pub fn #constructor_name(__db: &#db_dyn_ty, #(#field_names: #field_tys,)*) -> Self
                 {
-                    let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar_mut(__db);
-                    let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient_mut(__jar);
+                    let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db);
+                    let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar);
                     let __id = __ingredients.#input_index.new_singleton_input(__runtime);
                     #(
-                        __ingredients.#field_indices.store_mut(__runtime, __id, #field_names, salsa::Durability::LOW);
+                        __ingredients.#field_indices.store_new(__runtime, __id, #field_names, salsa::Durability::LOW);
                     )*
                     __id
                 }
@@ -162,7 +164,7 @@ impl InputStruct {
                 pub fn get(__db: &#db_dyn_ty) -> Self {
                     let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db);
                     let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar);
-                    __ingredients.#input_index.get_singleton_input(__runtime).expect("singleton not yet initialized")
+                    __ingredients.#input_index.get_singleton_input(__runtime).expect("singleton input struct not yet initialized")
                 }
             };
 
@@ -289,8 +291,13 @@ impl InputStruct {
             .collect()
     }
 
-    fn all_fields_immuts(&self) -> Vec<bool> {
-        self.all_fields().map(|f| f.has_id_attr).collect()
+    /// Names of setters of all fields
+    /// setters are not created for fields with #[id] tag so they'll be safe to include in debug formatting
+    pub(crate) fn all_set_field_names(&self) -> Vec<&syn::Ident> {
+        self.all_fields()
+            .filter(|&field| !field.has_id_attr)
+            .map(|ef| ef.set_name())
+            .collect()
     }
 
     /// Implementation of `SalsaStructInDb`.

components/salsa-2022-macros/src/salsa_struct.rs

@@ -121,11 +121,6 @@ impl<A: AllowedOptions> SalsaStruct<A> {
         self.all_fields().map(|ef| ef.get_name()).collect()
     }
 
-    /// Names of setters of all fields
-    pub(crate) fn all_set_field_names(&self) -> Vec<&syn::Ident> {
-        self.all_fields().map(|ef| ef.set_name()).collect()
-    }
-
     /// Types of all fields (id and value).
     ///
     /// If this is an enum, empty vec.

components/salsa-2022/src/input.rs

@@ -49,13 +49,16 @@ where
         Id::from_id(crate::Id::from_u32(next_id))
     }
 
-    pub fn new_singleton_input(&mut self, _runtime: &Runtime) -> Id {
-        // There's only one singleton so record that we've created it
-        // and return the only id.
+    pub fn new_singleton_input(&self, _runtime: &Runtime) -> Id {
+        // when one exists already, panic
+        if self.counter.load(Ordering::Relaxed) >= 1 {
+            panic!("Singleton struct may not be duplicated");
+        }
+        // fresh new ingredient
         self.counter.store(1, Ordering::Relaxed);
         Id::from_id(crate::Id::from_u32(0))
     }
-
+    
     pub fn get_singleton_input(&self, _runtime: &Runtime) -> Option<Id> {
         (self.counter.load(Ordering::Relaxed) > 0).then(|| Id::from_id(crate::Id::from_u32(0)))
     }

salsa-2022-tests/tests/compile-fail/input_struct_id_fields_no_setters.rs

@@ -0,0 +1,31 @@
+
+#[salsa::jar(db = Db)]
+struct Jar(MyInput);
+
+trait Db: salsa::DbWithJar<Jar> {}
+
+#[salsa::input(jar = Jar)]
+struct MyInput {
+    field: u32,
+    #[id]
+    id_one: u32,
+}
+
+#[salsa::db(Jar)]
+#[derive(Default)]
+struct Database {
+    storage: salsa::Storage<Self>,
+}
+
+impl salsa::Database for Database {}
+
+impl Db for Database {}
+
+
+fn main() {
+    let mut db = Database::default();
+    let input = MyInput::new(&mut db, 3, 4);
+    // should not compile as `id_one` should not have a setter
+    // compile error: method `set_id_one` not found in scope
+    input.set_id_one(1);
+}

salsa-2022-tests/tests/compile-fail/input_struct_id_fields_no_setters.stderr

@@ -0,0 +1,8 @@
+error[E0599]: no method named `set_id_one` found for struct `MyInput` in the current scope
+  --> tests/compile-fail/input_struct_id_fields_no_setters.rs:30:11
+   |
+7  | #[salsa::input(jar = Jar)]
+   | -------------------------- method `set_id_one` not found for this
+...
+30 |     input.set_id_one(1);
+   |           ^^^^^^^^^^ help: there is an associated function with a similar name: `id_one`

salsa-2022-tests/tests/singleton.rs

@@ -2,6 +2,8 @@
 //!
 //! Singleton structs are created only once. Subsequent `get`s and `new`s after creation return the same `Id`.
 
+use expect_test::expect;
+use salsa::DebugWithDb;
 use salsa_2022_tests::{HasLogger, Logger};
 
 use test_log::test;
@@ -14,6 +16,8 @@ trait Db: salsa::DbWithJar<Jar> + HasLogger {}
 #[salsa::input(singleton)]
 struct MyInput {
     field: u32,
+    #[id]
+    id_field: u16,
 }
 
 #[salsa::db(Jar)]
@@ -36,15 +40,33 @@ impl HasLogger for Database {
 #[test]
 fn basic() {
     let mut db = Database::default();
-    let input1 = MyInput::new(&mut db, 3);
+    let input1 = MyInput::new(&mut db, 3, 4);
     let input2 = MyInput::get(&db);
 
     assert_eq!(input1, input2);
 
     let input3 = MyInput::try_get(&db);
     assert_eq!(Some(input1), input3);
-
-    let input4 = MyInput::new(&mut db, 3);
-
-    assert_eq!(input2, input4)
+}
+
+#[test]
+#[should_panic]
+fn twice() {
+    let mut db = Database::default();
+    let input1 = MyInput::new(&mut db, 3, 4);
+    let input2 = MyInput::get(&db);
+
+    assert_eq!(input1, input2);
+
+    // should panic here
+    _ = MyInput::new(&mut db, 3, 5);
+}
+
+#[test]
+fn debug() {
+    let mut db = Database::default();
+    let input = MyInput::new(&mut db, 3, 4);
+    let actual = format!("{:?}", input.debug(&db));
+    let expected = expect![[r#"MyInput { [salsa id]: 0, id_field: 4 }"#]];
+    expected.assert_eq(&actual);
 }