diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs index 66d74543d..cbb605cc9 100644 --- a/components/salsa-macro-rules/src/setup_interned_struct.rs +++ b/components/salsa-macro-rules/src/setup_interned_struct.rs @@ -39,26 +39,35 @@ macro_rules! setup_interned_struct { // Name user gave for `new` new_fn: $new_fn:ident, - // A series of option tuples; see `setup_tracked_struct` macro - field_options: [$($field_option:tt),*], - - // Field names - field_ids: [$($field_id:ident),*], - - // Names for field setter methods (typically `set_foo`) - field_getters: [$($field_getter_vis:vis $field_getter_id:ident),*], - - // Field types - field_tys: [$($field_ty:ty),*], - - // Indices for each field from 0..N -- must be unsuffixed (e.g., `0`, `1`). - field_indices: [$($field_index:tt),*], - - // Indexed types for each field (T0, T1, ...) - field_indexed_tys: [$($indexed_ty:ident),*], - - // Attrs for each field. - field_attrs: [$([$(#[$field_attr:meta]),*]),*], + // Fields in declaration order. + fields: [$({ + option: $field_option:tt, + self_ref: $field_self_ref:tt, + id: $field_id:ident, + getter: $field_getter_vis:vis $field_getter_id:ident, + ty: $field_ty:ty, + index: $field_index:tt, + constructor_arg: ($constructor_arg_id:ident: $constructor_arg_ty:ty), + value: $field_value:expr, + attrs: [$(#[$field_attr:meta]),*] + }),*], + + // Fields that form the hashed lookup key. + identity_fields: [$({ + id: $key_field_id:ident, + ty: $key_field_ty:ty, + indexed_ty: $key_indexed_ty:ident, + field_index: $key_field_index:tt, + key_index: $key_index:tt + }),*], + + // Fields that can refer to the value being constructed. + self_ref_fields: [$({ + id: $self_ref_field_id:ident, + ty: $self_ref_field_ty:ty, + field_index: $self_ref_field_index:tt, + key_index: $self_ref_key_index:tt + }),*], // Number of fields num_fields: $N:literal, @@ -90,6 +99,9 @@ macro_rules! setup_interned_struct { $Configuration:ident, $CACHE:ident, $Db:ident, + $assembled_id:ident, + $assembled_data:ident, + $default_debug_fmt:ident, ] ) => { $(#[$attr])* @@ -127,33 +139,47 @@ macro_rules! setup_interned_struct { /// Key to use during hash lookups. Each field is some type that implements `Lookup` /// for the owned type. This permits interning with an `&str` when a `String` is required and so forth. - #[derive(Hash)] - struct StructKey<$db_lt, $($indexed_ty),*>( - $($indexed_ty,)* + struct StructKey<$db_lt, $($key_indexed_ty),*>( + ($($key_indexed_ty,)*), + ($(::std::option::Option<$self_ref_field_ty>,)*), ::std::marker::PhantomData<&$db_lt ()>, ); - impl<$db_lt, $($indexed_ty,)*> $zalsa::HashEqLike> - for $StructDataIdent<$db_lt> - where - $($field_ty: $zalsa::HashEqLike<$indexed_ty>),* - { - + impl<$db_lt, $($key_indexed_ty: ::std::hash::Hash,)*> ::std::hash::Hash + for StructKey<$db_lt, $($key_indexed_ty),*> + { fn hash(&self, h: &mut H) { - $($zalsa::HashEqLike::<$indexed_ty>::hash(&self.$field_index, &mut *h);)* - } - - fn eq(&self, data: &StructKey<$db_lt, $($indexed_ty),*>) -> bool { - ($($zalsa::HashEqLike::<$indexed_ty>::eq(&self.$field_index, &data.$field_index) && )* true) + $(::std::hash::Hash::hash(&self.0.$key_index, &mut *h);)* } } - impl<$db_lt, $($indexed_ty: $zalsa::Lookup<$field_ty>),*> $zalsa::Lookup<$StructDataIdent<$db_lt>> - for StructKey<$db_lt, $($indexed_ty),*> { - - #[allow(unused_unit)] - fn into_owned(self) -> $StructDataIdent<$db_lt> { - ($($zalsa::Lookup::into_owned(self.$field_index),)*) + impl<$db_lt, $($key_indexed_ty,)*> $zalsa::HashEqLike> + for $StructDataIdent<$db_lt> + where + (): Sized, + $($key_field_ty: $zalsa::HashEqLike<$key_indexed_ty>,)* + { + fn eq( + &self, + id: $zalsa::Id, + data: &StructKey<$db_lt, $($key_indexed_ty),*>, + ) -> bool { + ($( + $zalsa::HashEqLike::<$key_indexed_ty>::eq( + &self.$key_field_index, + id, + &data.0.$key_index, + ) && + )* $( + match &data.1.$self_ref_key_index { + ::std::option::Option::Some(other) => { + self.$self_ref_field_index == *other + } + ::std::option::Option::None => { + $zalsa::AsId::as_id(&self.$self_ref_field_index) == id + } + } && + )* true) } } @@ -174,6 +200,10 @@ macro_rules! setup_interned_struct { type Fields<'a> = $StructDataIdent<'a>; type Struct<'db> = $Struct< $($db_lt_arg)? >; + fn hash_fields(value: &Self::Fields<'_>, h: &mut H) { + $(::std::hash::Hash::hash(&value.$key_field_index, &mut *h);)* + } + $( fn heap_size(value: &Self::Fields<'_>) -> Option { Some($heap_size_fn(value)) @@ -303,17 +333,28 @@ macro_rules! setup_interned_struct { unsafe impl< $($db_lt_arg)? > $zalsa::SalsaValue for $Struct< $($db_lt_arg)? > {} impl<$db_lt> $Struct< $($db_lt_arg)? > { - pub fn $new_fn<$Db, $($indexed_ty: $zalsa::Lookup<$field_ty> + ::std::hash::Hash,)*>(db: &$db_lt $Db, $($field_id: $indexed_ty),*) -> Self + pub fn $new_fn<$Db, $($key_indexed_ty: $zalsa::Lookup<$key_field_ty> + ::std::hash::Hash,)*>( + db: &$db_lt $Db, + $($constructor_arg_id: $constructor_arg_ty),* + ) -> Self where // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + ::salsa::Database, $( - $field_ty: $zalsa::HashEqLike<$indexed_ty>, + $key_field_ty: $zalsa::HashEqLike<$key_indexed_ty>, )* { let (zalsa, zalsa_local) = db.zalsas(); - $Configuration::ingredient(zalsa).intern(zalsa, zalsa_local, - StructKey::<$db_lt>($($field_id,)* ::std::marker::PhantomData::default()), |_, data| $zalsa::Lookup::into_owned(data)) + $Configuration::ingredient(zalsa).intern( + zalsa, + zalsa_local, + StructKey::<$db_lt>( + ($($key_field_id,)*), + ($($self_ref_field_id,)*), + ::std::marker::PhantomData::default(), + ), + |$assembled_id, $assembled_data| ($($field_value,)*), + ) } $( @@ -334,6 +375,41 @@ macro_rules! setup_interned_struct { )* } + fn $default_debug_fmt( + id: $zalsa::Id, + f: &mut ::std::fmt::Formatter<'_>, + ) -> ::std::fmt::Result + where + $(for<$db_lt> $field_ty: ::std::fmt::Debug),* + { + $zalsa::with_attached_database(|db| { + let zalsa = db.zalsa(); + let this = $zalsa::FromId::from_id(id); + let fields = $Configuration::ingredient(zalsa).fields(zalsa, this); + let mut f = f.debug_struct(stringify!($Struct)); + $( + let f = $zalsa::macro_if! { + if $field_self_ref { + if $zalsa::AsId::as_id(&fields.$field_index) == id + { + f.field( + stringify!($field_id), + &$zalsa::AsId::as_id(&fields.$field_index), + ) + } else { + f.field(stringify!($field_id), &fields.$field_index) + } + } else { + f.field(stringify!($field_id), &fields.$field_index) + } + }; + )* + f.finish() + }).unwrap_or_else(|| { + f.debug_tuple(stringify!($Struct)).field(&id).finish() + }) + } + // Duplication can be dropped here once we no longer allow the `no_lifetime` hack $zalsa::macro_if! { iftt ($($db_lt_arg)?) { @@ -345,19 +421,7 @@ macro_rules! setup_interned_struct { // with its check :^) $(for<$db_lt> $field_ty: ::std::fmt::Debug),* { - $zalsa::with_attached_database(|db| { - let zalsa = db.zalsa(); - let fields = $Configuration::ingredient(zalsa).fields(zalsa, this); - let mut f = f.debug_struct(stringify!($Struct)); - $( - let f = f.field(stringify!($field_id), &fields.$field_index); - )* - f.finish() - }).unwrap_or_else(|| { - f.debug_tuple(stringify!($Struct)) - .field(&$zalsa::AsId::as_id(&this)) - .finish() - }) + $default_debug_fmt($zalsa::AsId::as_id(&this), f) } } } else { @@ -369,19 +433,7 @@ macro_rules! setup_interned_struct { // with its check :^) $(for<$db_lt> $field_ty: ::std::fmt::Debug),* { - $zalsa::with_attached_database(|db| { - let zalsa = db.zalsa(); - let fields = $Configuration::ingredient(zalsa).fields(zalsa, this); - let mut f = f.debug_struct(stringify!($Struct)); - $( - let f = f.field(stringify!($field_id), &fields.$field_index); - )* - f.finish() - }).unwrap_or_else(|| { - f.debug_tuple(stringify!($Struct)) - .field(&$zalsa::AsId::as_id(&this)) - .finish() - }) + $default_debug_fmt($zalsa::AsId::as_id(&this), f) } } } diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index 5fd1fbac2..e6037d3fc 100644 --- a/components/salsa-macros/src/input.rs +++ b/components/salsa-macros/src/input.rs @@ -84,6 +84,8 @@ impl SalsaStructAllowedOptions for InputStruct { const ALLOW_DEFAULT: bool = true; const ALLOW_MANUAL_RETENTION_PROOF: bool = false; + + const ALLOW_SELF_REF: bool = false; } struct Macro { diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index cb6126e96..7edd06040 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -84,6 +84,8 @@ impl SalsaStructAllowedOptions for InternedStruct { const ALLOW_DEFAULT: bool = false; const ALLOW_MANUAL_RETENTION_PROOF: bool = true; + + const ALLOW_SELF_REF: bool = true; } struct Macro { @@ -103,16 +105,7 @@ impl Macro { let struct_data_ident = format_ident!("{}Data", struct_ident); let db_lt = db_lifetime::db_lifetime(&self.struct_item.generics); let new_fn = salsa_struct.constructor_name(); - let field_ids = salsa_struct.field_ids(); - let field_indices = salsa_struct.field_indices(); let num_fields = salsa_struct.num_fields(); - let field_vis = salsa_struct.field_vis(); - let field_getter_ids = salsa_struct.field_getter_ids(); - let field_options = salsa_struct.field_options(); - let field_tys = salsa_struct.field_tys(); - let field_manual_retention_proofs = salsa_struct.field_manual_retention_proofs(); - let field_indexed_tys = salsa_struct.field_indexed_tys(); - let field_unused_attrs = salsa_struct.field_attrs(); let generate_debug_impl = salsa_struct.generate_debug_impl(); let has_lifetime = salsa_struct.generate_lifetime(); let id = salsa_struct.id(); @@ -145,16 +138,118 @@ impl Macro { let Configuration = self.hygiene.ident("Configuration"); let CACHE = self.hygiene.ident("CACHE"); let Db = self.hygiene.ident("Db"); + let assembled_id = self.hygiene.ident("assembled_id"); + let assembled_data = self.hygiene.ident("assembled_data"); + let default_debug_fmt = self.hygiene.ident("default_debug_fmt"); + + let mut identity_index = 0; + let mut self_ref_index = 0; + let fields = salsa_struct + .fields_iter() + .map(|(field_index, field)| { + let partition_index = if field.has_self_ref_attr { + let index = self_ref_index; + self_ref_index += 1; + index + } else { + let index = identity_index; + identity_index += 1; + index + }; + (field_index, partition_index, field) + }) + .collect::>(); + + let field_descriptors = fields.iter().map(|(field_index, partition_index, field)| { + let field_id = field.field.ident.as_ref().unwrap(); + let field_ty = &field.field.ty; + let field_vis = &field.field.vis; + let field_getter_id = field.getter_name(); + let field_option = field.options(); + let field_self_ref = field.has_self_ref_attr; + let indexed_ty = format_ident!("T{field_index}"); + let field_index = proc_macro2::Literal::usize_unsuffixed(*field_index); + let partition_index = proc_macro2::Literal::usize_unsuffixed(*partition_index); + let field_attrs = field.attrs(); + + let (constructor_arg_ty, field_value) = if field_self_ref { + ( + quote!(::std::option::Option<#field_ty>), + quote!(#assembled_data.1.#partition_index.unwrap_or_else(|| { + let this: Self = #zalsa::FromId::from_id(#assembled_id); + this + })), + ) + } else { + ( + quote!(#indexed_ty), + quote!(#zalsa::Lookup::into_owned( + #assembled_data.0.#partition_index + )), + ) + }; + + quote! { + { + option: #field_option, + self_ref: #field_self_ref, + id: #field_id, + getter: #field_vis #field_getter_id, + ty: #field_ty, + index: #field_index, + constructor_arg: (#field_id: #constructor_arg_ty), + value: #field_value, + attrs: [#(#field_attrs),*] + } + } + }); + + let identity_field_descriptors = salsa_struct.non_self_ref_fields_iter().enumerate().map( + |(key_index, (field_index, field))| { + let field_id = field.field.ident.as_ref().unwrap(); + let field_ty = &field.field.ty; + let indexed_ty = format_ident!("T{field_index}"); + let field_index = proc_macro2::Literal::usize_unsuffixed(field_index); + let key_index = proc_macro2::Literal::usize_unsuffixed(key_index); + quote! { + { + id: #field_id, + ty: #field_ty, + indexed_ty: #indexed_ty, + field_index: #field_index, + key_index: #key_index + } + } + }, + ); + + let self_ref_field_descriptors = salsa_struct.self_ref_fields_iter().enumerate().map( + |(key_index, (field_index, field))| { + let field_id = field.field.ident.as_ref().unwrap(); + let field_ty = &field.field.ty; + let field_index = proc_macro2::Literal::usize_unsuffixed(field_index); + let key_index = proc_macro2::Literal::usize_unsuffixed(key_index); + quote! { + { + id: #field_id, + ty: #field_ty, + field_index: #field_index, + key_index: #key_index + } + } + }, + ); let self_type = if has_lifetime { syn::parse_quote!(#struct_ident<#db_lt>) } else { syn::parse_quote!(#struct_ident) }; - let assert_fields_are_salsa_values: TokenStream = field_tys - .iter() - .zip(field_manual_retention_proofs) - .map(|(field_ty, proof)| { + let assert_fields_are_salsa_values: TokenStream = salsa_struct + .fields_iter() + .map(|(_, field)| { + let field_ty = &field.field.ty; + let proof = field.manual_retention_proof.as_ref(); if self.args.non_salsa_values.is_some() && proof.is_none() { quote! {} } else { @@ -180,13 +275,9 @@ impl Macro { revisions: #(#revisions)*, interior_lt: #interior_lt, new_fn: #new_fn, - field_options: [#(#field_options),*], - field_ids: [#(#field_ids),*], - field_getters: [#(#field_vis #field_getter_ids),*], - field_tys: [#(#field_tys),*], - field_indices: [#(#field_indices),*], - field_indexed_tys: [#(#field_indexed_tys),*], - field_attrs: [#([#(#field_unused_attrs),*]),*], + fields: [#(#field_descriptors),*], + identity_fields: [#(#identity_field_descriptors),*], + self_ref_fields: [#(#self_ref_field_descriptors),*], num_fields: #num_fields, generate_debug_impl: #generate_debug_impl, heap_size_fn: #(#heap_size_fn)*, @@ -200,6 +291,9 @@ impl Macro { #Configuration, #CACHE, #Db, + #assembled_id, + #assembled_data, + #default_debug_fmt, ] ); }, diff --git a/components/salsa-macros/src/lib.rs b/components/salsa-macros/src/lib.rs index 5b3f2910b..c0719325f 100644 --- a/components/salsa-macros/src/lib.rs +++ b/components/salsa-macros/src/lib.rs @@ -141,6 +141,11 @@ pub fn db(args: TokenStream, input: TokenStream) -> TokenStream { /// [`Clone`] + [`Eq`] + [`Hash`] + [`Send`] + [`Sync`]. A field whose type is unconditionally /// `'static` is accepted directly; any other field must implement [`salsa::SalsaValue`]. /// +/// A field marked `#[self_ref]` becomes an [`Option`] constructor parameter. [`Some`] stores +/// the supplied value, while [`None`] stores the interned value being constructed. The resolved +/// field value contributes to the struct's identity, although Salsa omits self-referential fields +/// from the identity hash. +/// /// See [interned structs in the `salsa` crate documentation] for their identity and lifecycle. /// /// # Options @@ -584,6 +589,21 @@ pub fn salsa_value(input: TokenStream) -> TokenStream { } pub(crate) fn token_stream_with_error(mut tokens: TokenStream, error: syn::Error) -> TokenStream { + if let Ok(mut struct_item) = syn::parse::(tokens.clone()) { + let mut removed_salsa_attribute = false; + for field in &mut struct_item.fields { + field.attrs.retain(|attr| { + let is_salsa_attribute = salsa_struct::FIELD_OPTION_ATTRIBUTES + .iter() + .any(|attribute| attr.path().is_ident(attribute.0)); + removed_salsa_attribute |= is_salsa_attribute; + !is_salsa_attribute + }); + } + if removed_salsa_attribute { + tokens = quote!(#struct_item).into(); + } + } tokens.extend(TokenStream::from(error.into_compile_error())); tokens } diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs index 88210251a..5d204adb7 100644 --- a/components/salsa-macros/src/salsa_struct.rs +++ b/components/salsa-macros/src/salsa_struct.rs @@ -55,6 +55,9 @@ pub(crate) trait SalsaStructAllowedOptions: AllowedOptions { /// Are manual `#[salsa_value(...)]` retention proofs allowed on fields? const ALLOW_MANUAL_RETENTION_PROOF: bool; + + /// Are `#[self_ref]` fields allowed? + const ALLOW_SELF_REF: bool; } pub(crate) struct SalsaField<'s> { @@ -65,6 +68,7 @@ pub(crate) struct SalsaField<'s> { pub(crate) returns: syn::Ident, pub(crate) has_no_eq_attr: bool, pub(crate) manual_retention_proof: Option, + pub(crate) has_self_ref_attr: bool, get_name: syn::Ident, set_name: syn::Ident, unknown_attrs: Vec<&'s syn::Attribute>, @@ -94,6 +98,19 @@ pub(crate) const FIELD_OPTION_ATTRIBUTES: &[( ef.has_no_eq_attr = true; Ok(()) }), + ("self_ref", |attr, ef| { + match &attr.meta { + syn::Meta::Path(_) => {} + syn::Meta::List(_) | syn::Meta::NameValue(_) => { + return Err(syn::Error::new_spanned( + attr, + "`#[self_ref]` does not accept arguments", + )); + } + } + ef.has_self_ref_attr = true; + Ok(()) + }), ("salsa_value", |attr, ef| { if ef.manual_retention_proof.is_some() { return Err(syn::Error::new_spanned( @@ -141,6 +158,7 @@ where this.maybe_disallow_tracked_fields()?; this.maybe_disallow_default_fields()?; this.maybe_disallow_manual_retention_proofs()?; + this.maybe_disallow_self_ref_fields()?; this.check_generics()?; @@ -218,6 +236,23 @@ where Ok(()) } + fn maybe_disallow_self_ref_fields(&self) -> syn::Result<()> { + if A::ALLOW_SELF_REF { + return Ok(()); + } + + for field in &self.fields { + if field.has_self_ref_attr { + return Err(syn::Error::new_spanned( + field.field, + format!("`#[self_ref]` cannot be used with `#[salsa::{}]`", A::KIND), + )); + } + } + + Ok(()) + } + fn maybe_disallow_manual_retention_proofs(&self) -> syn::Result<()> { if A::ALLOW_MANUAL_RETENTION_PROOF { return Ok(()); @@ -373,14 +408,6 @@ where .collect() } - pub(crate) fn field_indexed_tys(&self) -> Vec { - self.fields - .iter() - .enumerate() - .map(|(i, _)| quote::format_ident!("T{i}")) - .collect() - } - pub(crate) fn field_attrs(&self) -> Vec<&[&syn::Attribute]> { self.fields.iter().map(|f| &*f.unknown_attrs).collect() } @@ -421,18 +448,24 @@ where self.args.no_lifetime.is_none() } + pub fn fields_iter(&self) -> impl Iterator)> { + self.fields.iter().enumerate() + } + pub fn tracked_fields_iter(&self) -> impl Iterator)> { - self.fields - .iter() - .enumerate() - .filter(|(_, f)| f.has_tracked_attr) + self.fields_iter().filter(|(_, f)| f.has_tracked_attr) } pub fn untracked_fields_iter(&self) -> impl Iterator)> { - self.fields - .iter() - .enumerate() - .filter(|(_, f)| !f.has_tracked_attr) + self.fields_iter().filter(|(_, f)| !f.has_tracked_attr) + } + + pub fn self_ref_fields_iter(&self) -> impl Iterator)> { + self.fields_iter().filter(|(_, f)| f.has_self_ref_attr) + } + + pub fn non_self_ref_fields_iter(&self) -> impl Iterator)> { + self.fields_iter().filter(|(_, f)| !f.has_self_ref_attr) } /// Returns the path to the `serialize` function as an optional iterator. @@ -487,6 +520,7 @@ impl<'s> SalsaField<'s> { has_default_attr: false, has_no_eq_attr: false, manual_retention_proof: None, + has_self_ref_attr: false, get_name, set_name, unknown_attrs: Default::default(), @@ -521,7 +555,15 @@ impl<'s> SalsaField<'s> { Ok(result) } - fn options(&self) -> TokenStream { + pub(crate) fn getter_name(&self) -> &syn::Ident { + &self.get_name + } + + pub(crate) fn attrs(&self) -> &[&syn::Attribute] { + &self.unknown_attrs + } + + pub(crate) fn options(&self) -> TokenStream { let returns = &self.returns; let default_ident = if self.has_default_attr { diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs index faca13c47..cd3288fc5 100644 --- a/components/salsa-macros/src/tracked_struct.rs +++ b/components/salsa-macros/src/tracked_struct.rs @@ -80,6 +80,8 @@ impl SalsaStructAllowedOptions for TrackedStruct { const ALLOW_DEFAULT: bool = false; const ALLOW_MANUAL_RETENTION_PROOF: bool = true; + + const ALLOW_SELF_REF: bool = false; } struct Macro { diff --git a/src/interned.rs b/src/interned.rs index 736a2ccff..d7c617281 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -60,6 +60,11 @@ pub unsafe trait Configuration: Sized + 'static { /// The end user struct type Struct<'db>: Copy + FromId + AsId; + /// Hashes the fields that determine the struct's identity. + fn hash_fields(value: &Self::Fields<'_>, h: &mut H) { + value.hash(h); + } + /// Returns the size of any heap allocations in the output value, in bytes. fn heap_size(_value: &Self::Fields<'_>) -> Option { None @@ -570,7 +575,7 @@ where let new_fields = unsafe { self.to_internal_data(assemble(slot.new_id, key)) }; // SAFETY: We hold the lock for the shard containing the value. - let old_hash = self.hasher.hash_one(unsafe { &*value.fields.get() }); + let old_hash = unsafe { self.value_hash(value) }; let index = self.database_key_index(slot.new_id); @@ -942,7 +947,13 @@ where // The lock must be held for the shard containing the value. unsafe fn value_hash(&self, value: &Value) -> u64 { // SAFETY: We hold the lock for the shard containing the value. - unsafe { self.hasher.hash_one(&*value.fields.get()) } + self.fields_hash(unsafe { &*value.fields.get() }) + } + + fn fields_hash(&self, fields: &C::Fields<'_>) -> u64 { + let mut hasher = self.hasher.build_hasher(); + C::hash_fields(fields, &mut hasher); + hasher.finish() } // Compares the value by its fields to the given key. @@ -957,7 +968,10 @@ where // SAFETY: We hold the lock for the shard containing the value. let fields = unsafe { &*value.fields.get() }; - HashEqLike::eq(Self::from_internal_data(fields), key) + // SAFETY: We hold the lock for the shard containing the value. + let id = unsafe { (*value.lru.metadata.get()).id }; + + HashEqLike::eq(Self::from_internal_data(fields), id, key) } /// Returns the database key index for an interned value with the given id. @@ -1419,10 +1433,12 @@ impl RevisionQueue { } } -/// A trait for types that hash and compare like `O`. +/// Compares a stored value with a lookup value of type `O`. +/// +/// The stored and lookup values must produce the same hash when they compare equal. The ID passed +/// to [`HashEqLike::eq`] identifies the interned value containing the stored value. pub trait HashEqLike { - fn hash(&self, h: &mut H); - fn eq(&self, data: &O) -> bool; + fn eq(&self, id: Id, data: &O) -> bool; } /// The `Lookup` trait is a more flexible variant on [`std::borrow::Borrow`] @@ -1438,7 +1454,7 @@ pub trait HashEqLike { /// multiple keys accumulated into a struct, like `ViewStruct: Lookup<(K1, ...)>`, /// where `struct ViewStruct...>(K1...)`. The `Borrow` trait /// requires that `&(K1...)` be convertible to `&ViewStruct` which just isn't -/// possible. `Lookup` instead offers direct `hash` and `eq` methods. +/// possible. [`HashEqLike`] instead compares the stored and lookup representations directly. pub trait Lookup { fn into_owned(self) -> O; } @@ -1453,11 +1469,7 @@ impl HashEqLike for T where T: Hash + Eq, { - fn hash(&self, h: &mut H) { - Hash::hash(self, &mut *h); - } - - fn eq(&self, data: &T) -> bool { + fn eq(&self, _id: Id, data: &T) -> bool { self == data } } @@ -1466,11 +1478,7 @@ impl HashEqLike for &T where T: Hash + Eq, { - fn hash(&self, h: &mut H) { - Hash::hash(*self, &mut *h); - } - - fn eq(&self, data: &T) -> bool { + fn eq(&self, _id: Id, data: &T) -> bool { **self == *data } } @@ -1479,11 +1487,7 @@ impl HashEqLike<&T> for T where T: Hash + Eq, { - fn hash(&self, h: &mut H) { - Hash::hash(self, &mut *h); - } - - fn eq(&self, data: &&T) -> bool { + fn eq(&self, _id: Id, data: &&T) -> bool { *self == **data } } @@ -1502,10 +1506,7 @@ where T: ?Sized + Hash + Eq, Box: From<&'a T>, { - fn hash(&self, h: &mut H) { - Hash::hash(self, &mut *h) - } - fn eq(&self, data: &&T) -> bool { + fn eq(&self, _id: Id, data: &&T) -> bool { **self == **data } } @@ -1525,10 +1526,7 @@ where T: ?Sized + Hash + Eq, Arc: From<&'a T>, { - fn hash(&self, h: &mut H) { - Hash::hash(&**self, &mut *h) - } - fn eq(&self, data: &&T) -> bool { + fn eq(&self, _id: Id, data: &&T) -> bool { **self == **data } } @@ -1549,10 +1547,7 @@ where T: ?Sized + Hash + Eq, triomphe::Arc: From<&'a T>, { - fn hash(&self, h: &mut H) { - Hash::hash(&**self, &mut *h) - } - fn eq(&self, data: &&T) -> bool { + fn eq(&self, _id: Id, data: &&T) -> bool { **self == **data } } @@ -1583,22 +1578,14 @@ impl Lookup for &str { #[cfg(feature = "compact_str")] impl HashEqLike<&str> for compact_str::CompactString { - fn hash(&self, h: &mut H) { - Hash::hash(self, &mut *h) - } - - fn eq(&self, data: &&str) -> bool { + fn eq(&self, _id: Id, data: &&str) -> bool { self == *data } } #[cfg(feature = "compact_str")] impl HashEqLike> for compact_str::CompactString { - fn hash(&self, h: &mut H) { - self.as_str().hash(h); - } - - fn eq(&self, data: &Cow<'_, str>) -> bool { + fn eq(&self, _id: Id, data: &Cow<'_, str>) -> bool { self.as_str() == data.as_ref() } } @@ -1611,21 +1598,13 @@ impl Lookup for Cow<'_, str> { } impl HashEqLike<&str> for String { - fn hash(&self, h: &mut H) { - Hash::hash(self, &mut *h) - } - - fn eq(&self, data: &&str) -> bool { + fn eq(&self, _id: Id, data: &&str) -> bool { self == *data } } impl> HashEqLike<&[A]> for Vec { - fn hash(&self, h: &mut H) { - Hash::hash(self, h); - } - - fn eq(&self, data: &&[A]) -> bool { + fn eq(&self, _id: Id, data: &&[A]) -> bool { self.len() == data.len() && data.iter().enumerate().all(|(i, a)| &self[i] == a) } } @@ -1637,11 +1616,7 @@ impl + Clone + Lookup, T> Lookup> for &[A] } impl> HashEqLike<[A; N]> for Vec { - fn hash(&self, h: &mut H) { - Hash::hash(self, h); - } - - fn eq(&self, data: &[A; N]) -> bool { + fn eq(&self, _id: Id, data: &[A; N]) -> bool { self.len() == data.len() && data.iter().enumerate().all(|(i, a)| &self[i] == a) } } @@ -1655,11 +1630,7 @@ impl + Clone + Lookup, T> Lookup< } impl HashEqLike<&Path> for PathBuf { - fn hash(&self, h: &mut H) { - Hash::hash(self, h); - } - - fn eq(&self, data: &&Path) -> bool { + fn eq(&self, _id: Id, data: &&Path) -> bool { self == data } } @@ -1671,11 +1642,7 @@ impl Lookup for &Path { } impl HashEqLike> for T { - fn hash(&self, h: &mut H) { - Hash::hash(self, h); - } - - fn eq(&self, data: &Cow<'_, T>) -> bool { + fn eq(&self, _id: Id, data: &Cow<'_, T>) -> bool { self == data.as_ref() } } @@ -1687,11 +1654,7 @@ impl Lookup for Cow<'_, T> { } impl HashEqLike> for String { - fn hash(&self, h: &mut H) { - self.as_str().hash(h); - } - - fn eq(&self, data: &Cow<'_, str>) -> bool { + fn eq(&self, _id: Id, data: &Cow<'_, str>) -> bool { self.as_str() == data.as_ref() } } @@ -1703,11 +1666,7 @@ impl Lookup for Cow<'_, str> { } impl HashEqLike> for PathBuf { - fn hash(&self, h: &mut H) { - self.as_path().hash(h); - } - - fn eq(&self, data: &Cow<'_, Path>) -> bool { + fn eq(&self, _id: Id, data: &Cow<'_, Path>) -> bool { self.as_path() == data.as_ref() } } @@ -1719,11 +1678,7 @@ impl Lookup for Cow<'_, Path> { } impl HashEqLike> for Box<[T]> { - fn hash(&self, h: &mut H) { - self.as_ref().hash(h); - } - - fn eq(&self, data: &Cow<'_, [T]>) -> bool { + fn eq(&self, _id: Id, data: &Cow<'_, [T]>) -> bool { self.as_ref() == data.as_ref() } } @@ -1735,11 +1690,7 @@ impl Lookup> for Cow<'_, [T]> { } impl HashEqLike> for Vec { - fn hash(&self, h: &mut H) { - self.as_slice().hash(h); - } - - fn eq(&self, data: &Cow<'_, [T]>) -> bool { + fn eq(&self, _id: Id, data: &Cow<'_, [T]>) -> bool { self.as_slice() == data.as_ref() } } @@ -1754,7 +1705,6 @@ impl Lookup> for Cow<'_, [T]> { mod persistence { use std::cell::UnsafeCell; use std::fmt; - use std::hash::BuildHasher; use intrusive_collections::LinkedListLink; use serde::ser::{SerializeMap, SerializeStruct}; @@ -1903,7 +1853,7 @@ mod persistence { let (page_idx, _) = crate::table::split_id(id); // Determine the value shard. - let hash = ingredient.hasher.hash_one(&value.fields.0); + let hash = ingredient.fields_hash(&value.fields.0); let shard_index = ingredient.shard(hash); // SAFETY: `shard_index` is guaranteed to be in-bounds for `self.shards`. diff --git a/tests/compile-fail/input_struct_incompatibles.stderr b/tests/compile-fail/input_struct_incompatibles.stderr index 1aedb870d..a2d9989d9 100644 --- a/tests/compile-fail/input_struct_incompatibles.stderr +++ b/tests/compile-fail/input_struct_incompatibles.stderr @@ -46,9 +46,3 @@ error: `#[tracked]` cannot be used with `#[salsa::input]` 24 | / #[tracked] 25 | | field: u32, | |______________^ - -error: cannot find attribute `tracked` in this scope - --> tests/compile-fail/input_struct_incompatibles.rs:24:7 - | -24 | #[tracked] - | ^^^^^^^ diff --git a/tests/compile-fail/interned_self_ref_wrong_type.rs b/tests/compile-fail/interned_self_ref_wrong_type.rs new file mode 100644 index 000000000..153ac8a79 --- /dev/null +++ b/tests/compile-fail/interned_self_ref_wrong_type.rs @@ -0,0 +1,13 @@ +#[salsa::interned] +struct Other<'db> { + value: u32, +} + +#[salsa::interned] +struct Bad<'db> { + key: u32, + #[self_ref] + other: Other<'db>, +} + +fn main() {} diff --git a/tests/compile-fail/interned_self_ref_wrong_type.stderr b/tests/compile-fail/interned_self_ref_wrong_type.stderr new file mode 100644 index 000000000..defc50a11 --- /dev/null +++ b/tests/compile-fail/interned_self_ref_wrong_type.stderr @@ -0,0 +1,7 @@ +error[E0308]: mismatched types + --> tests/compile-fail/interned_self_ref_wrong_type.rs:6:1 + | +6 | #[salsa::interned] + | ^^^^^^^^^^^^^^^^^^ expected `Other<'_>`, found `Bad<'_>` + | + = note: this error originates in the attribute macro `salsa::interned` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/compile-fail/interned_struct_incompatibles.stderr b/tests/compile-fail/interned_struct_incompatibles.stderr index 27d72e44f..5dc88bef4 100644 --- a/tests/compile-fail/interned_struct_incompatibles.stderr +++ b/tests/compile-fail/interned_struct_incompatibles.stderr @@ -58,9 +58,3 @@ error: `unsafe(no_lifetime)` requires `revisions = usize::MAX` | 52 | #[salsa::interned(unsafe(no_lifetime), revisions = 3)] | ^ - -error: cannot find attribute `tracked` in this scope - --> tests/compile-fail/interned_struct_incompatibles.rs:33:7 - | -33 | #[tracked] - | ^^^^^^^ diff --git a/tests/compile-fail/invalid_return_mode.stderr b/tests/compile-fail/invalid_return_mode.stderr index fd3390909..1e945c5e0 100644 --- a/tests/compile-fail/invalid_return_mode.stderr +++ b/tests/compile-fail/invalid_return_mode.stderr @@ -9,9 +9,3 @@ error: Invalid return mode. Allowed modes are: ["copy", "clone", "ref", "deref", | 16 | #[returns(not_a_return_mode)] | ^^^^^^^^^^^^^^^^^ - -error: cannot find attribute `returns` in this scope - --> tests/compile-fail/invalid_return_mode.rs:16:7 - | -16 | #[returns(not_a_return_mode)] - | ^^^^^^^ diff --git a/tests/compile-fail/salsa_value_invalid_field_attributes.stderr b/tests/compile-fail/salsa_value_invalid_field_attributes.stderr index 9fa9ac4d6..40dd77509 100644 --- a/tests/compile-fail/salsa_value_invalid_field_attributes.stderr +++ b/tests/compile-fail/salsa_value_invalid_field_attributes.stderr @@ -43,19 +43,3 @@ error: `#[salsa_value(...)]` cannot be used with `#[salsa::input]` 40 | / #[salsa_value(unsafe(prove(String: salsa::SalsaValue)))] 41 | | field: String, | |_________________^ - -error: cannot find attribute `salsa_value` in this scope - --> tests/compile-fail/salsa_value_invalid_field_attributes.rs:40:7 - | -40 | #[salsa_value(unsafe(prove(String: salsa::SalsaValue)))] - | ^^^^^^^^^^^ - | - = note: `salsa_value` is an attribute that can be used by the derive macro `SalsaValue`, you might be missing a `derive` attribute - -error: cannot find attribute `salsa_value` in this scope - --> tests/compile-fail/salsa_value_invalid_field_attributes.rs:34:7 - | -34 | #[salsa_value(unsafe(prove_safe_to_retain_manually))] - | ^^^^^^^^^^^ - | - = note: `salsa_value` is an attribute that can be used by the derive macro `SalsaValue`, you might be missing a `derive` attribute diff --git a/tests/compile-fail/self_ref_attribute_restrictions.rs b/tests/compile-fail/self_ref_attribute_restrictions.rs new file mode 100644 index 000000000..b46ad0840 --- /dev/null +++ b/tests/compile-fail/self_ref_attribute_restrictions.rs @@ -0,0 +1,19 @@ +#[salsa::input] +struct InputWithSelfRef { + #[self_ref] + field: u32, +} + +#[salsa::tracked] +struct TrackedWithSelfRef { + #[self_ref] + field: u32, +} + +#[salsa::interned] +struct SelfRefWithArguments { + #[self_ref(other)] + field: u32, +} + +fn main() {} diff --git a/tests/compile-fail/self_ref_attribute_restrictions.stderr b/tests/compile-fail/self_ref_attribute_restrictions.stderr new file mode 100644 index 000000000..ee6c74a77 --- /dev/null +++ b/tests/compile-fail/self_ref_attribute_restrictions.stderr @@ -0,0 +1,19 @@ +error: `#[self_ref]` cannot be used with `#[salsa::input]` + --> tests/compile-fail/self_ref_attribute_restrictions.rs:3:5 + | +3 | / #[self_ref] +4 | | field: u32, + | |______________^ + +error: `#[self_ref]` cannot be used with `#[salsa::tracked]` + --> tests/compile-fail/self_ref_attribute_restrictions.rs:9:5 + | + 9 | / #[self_ref] +10 | | field: u32, + | |______________^ + +error: `#[self_ref]` does not accept arguments + --> tests/compile-fail/self_ref_attribute_restrictions.rs:15:5 + | +15 | #[self_ref(other)] + | ^^^^^^^^^^^^^^^^^^ diff --git a/tests/interned-revisions.rs b/tests/interned-revisions.rs index 73994da71..690fa34a4 100644 --- a/tests/interned-revisions.rs +++ b/tests/interned-revisions.rs @@ -48,11 +48,7 @@ impl Lookup for PanickingLookup { } impl HashEqLike for BadHash { - fn hash(&self, state: &mut H) { - state.write_i16(0); - } - - fn eq(&self, data: &PanickingLookup) -> bool { + fn eq(&self, _id: salsa::Id, data: &PanickingLookup) -> bool { self.0 == data.0 } } @@ -62,11 +58,55 @@ struct PanickingInterned<'db> { value: BadHash, } +#[salsa::interned(revisions = 1)] +struct SelfRefInterned<'db> { + key: BadHash, + #[self_ref] + other: SelfRefInterned<'db>, +} + #[salsa::tracked(returns(copy))] fn intern_panicking(db: &dyn Database, input: Input) -> PanickingInterned<'_> { PanickingInterned::new(db, PanickingLookup(input.field1(db))) } +#[test] +fn self_references_participate_in_identity_across_reuse() { + use salsa::plumbing::AsId; + + #[salsa::tracked(returns(copy))] + fn intern(db: &dyn Database, activity: Input, key: usize) -> SelfRefInterned<'_> { + let _ = activity.field1(db); + SelfRefInterned::new(db, BadHash(key), None) + } + + #[salsa::tracked(returns(copy))] + fn intern_wrapper<'db>( + db: &'db dyn Database, + activity: Input, + other: SelfRefInterned<'db>, + ) -> SelfRefInterned<'db> { + let _ = activity.field1(db); + SelfRefInterned::new(db, BadHash(usize::MAX), Some(other)) + } + + let mut db = common::LoggerDatabase::default(); + let activity = Input::new(&db, 0); + let target = intern(&db, activity, 0); + let target_id = target.as_id(); + let wrapper = intern_wrapper(&db, activity, target); + + assert_eq!(wrapper.other(&db).key(&db).0, 0); + + activity.set_field1(&mut db).to(1); + let replacement = intern(&db, activity, 1); + let wrapper = intern_wrapper(&db, activity, replacement); + + assert_eq!(replacement.as_id(), target_id.next_generation().unwrap()); + assert!(*replacement.other(&db) == replacement); + assert_eq!(wrapper.other(&db).key(&db).0, 1); +} + #[test] fn panic_during_reuse_does_not_orphan_slot() { use std::panic::{AssertUnwindSafe, catch_unwind}; diff --git a/tests/interned-structs_self_ref.rs b/tests/interned-structs_self_ref.rs index 84a629381..9ca532e18 100644 --- a/tests/interned-structs_self_ref.rs +++ b/tests/interned-structs_self_ref.rs @@ -1,235 +1,174 @@ #![cfg(feature = "inventory")] -//! Test that a `tracked` fn on a `salsa::input` -//! compiles and executes successfully. +use salsa::Database; +use test_log::test; -use std::any::TypeId; -use std::convert::identity; +#[salsa::interned] +struct InternedString<'db> { + data: String, + #[self_ref] + other: InternedString<'db>, +} -use salsa::plumbing::Zalsa; -use test_log::test; +#[salsa::interned] +struct SelfOnly<'db> { + #[self_ref] + other: SelfOnly<'db>, +} + +#[salsa::interned(unsafe(no_lifetime), revisions = usize::MAX)] +struct SelfOnlyNoLifetime { + #[self_ref] + other: SelfOnlyNoLifetime, +} + +#[salsa::interned] +struct Interleaved<'db> { + first: String, + #[self_ref] + other: Interleaved<'db>, + second: u32, +} + +#[salsa::interned] +struct MultipleSelfRefs<'db> { + key: u32, + #[self_ref] + first: MultipleSelfRefs<'db>, + #[self_ref] + second: MultipleSelfRefs<'db>, +} + +#[salsa::interned(debug)] +struct DebugRecursive<'db> { + key: u32, + #[self_ref] + other: DebugRecursive<'db>, +} + +#[salsa::interned(heap_size = self_ref_heap_size)] +struct HeapRecursive<'db> { + data: String, + #[self_ref] + other: HeapRecursive<'db>, +} + +fn self_ref_heap_size((data, _other): &(String, HeapRecursive<'_>)) -> usize { + data.capacity() +} + +#[test] +fn self_ref_fields_accept_explicit_or_self_values() { + let db = salsa::DatabaseImpl::new(); + let s1 = InternedString::new(&db, "Hello, ".to_string(), None); + let s2 = InternedString::new(&db, "World, ".to_string(), Some(s1)); + + assert!(*s1.other(&db) == s1); + assert!(*s2.other(&db) == s1); + + let s1_again = InternedString::new(&db, "Hello, ", Some(s2)); + let s2_again = InternedString::new(&db, "World, ", None); + + assert!(s1_again != s1); + assert!(s2_again != s2); + assert!(*s1_again.other(&db) == s2); + assert!(*s2_again.other(&db) == s2_again); +} + +#[test] +fn self_ref_can_be_the_only_field() { + let db = salsa::DatabaseImpl::new(); + let value = SelfOnly::new(&db, None); + + assert!(*value.other(&db) == value); + assert!(SelfOnly::new(&db, Some(value)) == value); +} + +#[test] +fn self_ref_supports_no_lifetime() { + let db = salsa::DatabaseImpl::new(); + let value = SelfOnlyNoLifetime::new(&db, None); + + assert!(*value.other(&db) == value); +} #[test] -fn interning_returns_equal_keys_for_equal_data() { +fn self_ref_can_be_interleaved_with_identity_fields() { let db = salsa::DatabaseImpl::new(); - let s1 = InternedString::new(&db, "Hello, ".to_string(), identity); - let s2 = InternedString::new(&db, "World, ".to_string(), |_| s1); - let s1_2 = InternedString::new(&db, "Hello, ", identity); - let s2_2 = InternedString::new(&db, "World, ", |_| s2); - assert_eq!(s1, s1_2); - assert_eq!(s2, s2_2); + let value = Interleaved::new(&db, "first".to_string(), None, 1); + let other = Interleaved::new(&db, "other".to_string(), None, 2); + + let explicit = Interleaved::new(&db, "first", Some(other), 1); + + assert!(explicit != value); + assert!(Interleaved::new(&db, "different", None, 1) != value); + assert!(Interleaved::new(&db, "first", None, 2) != value); + assert!(value.first(&db) == "first"); + assert!(*value.other(&db) == value); + assert!(*value.second(&db) == 1); + assert!(*explicit.other(&db) == other); } -// Recursive expansion of interned macro -// #[salsa::interned] -// struct InternedString<'db> { -// data: String, -// other: InternedString<'db>, -// } -// ====================================== - -#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] -struct InternedString<'db>( - salsa::Id, - std::marker::PhantomData<&'db salsa::plumbing::interned::Value>>, -); - -#[allow(warnings)] -const _: () = { - use salsa::plumbing as zalsa_; - use zalsa_::interned as zalsa_struct_; - - type Configuration_ = InternedString<'static>; - - impl<'db> zalsa_::HasJar for InternedString<'db> { - type Jar = zalsa_struct_::JarImpl; - const KIND: zalsa_::JarKind = zalsa_::JarKind::Struct; - } - - zalsa_::register_jar! { - zalsa_::ErasedJar::erase::>() - } - - #[derive(Clone, salsa::SalsaValue)] - struct StructData<'db>(String, InternedString<'db>); - - impl<'db> Eq for StructData<'db> {} - impl<'db> PartialEq for StructData<'db> { - fn eq(&self, other: &Self) -> bool { - self.0 == other.0 - } - } - - impl<'db> std::hash::Hash for StructData<'db> { - fn hash(&self, state: &mut H) { - self.0.hash(state); - } - } - - #[doc = r" Key to use during hash lookups. Each field is some type that implements `Lookup`"] - #[doc = r" for the owned type. This permits interning with an `&str` when a `String` is required and so forth."] - #[derive(Hash)] - struct StructKey<'db, T0>(T0, std::marker::PhantomData<&'db ()>); - - impl<'db, T0> zalsa_::HashEqLike> for StructData<'db> - where - String: zalsa_::HashEqLike, - { - fn hash(&self, h: &mut H) { - zalsa_::HashEqLike::::hash(&self.0, &mut *h); - } - fn eq(&self, data: &StructKey<'db, T0>) -> bool { - (zalsa_::HashEqLike::::eq(&self.0, &data.0) && true) - } - } - // SAFETY: `StructData<'db>` contains only an owned `String` and a phantom lifetime. - unsafe impl zalsa_struct_::Configuration for Configuration_ { - const LOCATION: zalsa_::Location = zalsa_::Location { - file: file!(), - line: line!(), - }; - const DEBUG_NAME: &'static str = "InternedString"; - type Fields<'a> = StructData<'a>; - type Struct<'a> = InternedString<'a>; - - const PERSIST: bool = false; - - fn serialize(value: &Self::Fields<'_>, serializer: S) -> Result - where - S: zalsa_::serde::Serializer, - { - panic!("attempted to serialize value not marked with `persist` attribute") - } - - fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> - where - D: zalsa_::serde::Deserializer<'de>, - { - panic!("attempted to deserialize value not marked with `persist` attribute") - } - } - impl Configuration_ { - pub fn ingredient(zalsa: &zalsa_::Zalsa) -> &zalsa_struct_::IngredientImpl { - static CACHE: zalsa_::IngredientCache> = - zalsa_::IngredientCache::new(); - - // SAFETY: The ingredient at offset 0 in `JarImpl` has type - // `IngredientImpl`. - unsafe { CACHE.get_or_create::, 0>(zalsa) } - } - } - impl zalsa_::AsId for InternedString<'_> { - fn as_id(&self) -> salsa::Id { - self.0 - } - } - impl zalsa_::FromId for InternedString<'_> { - fn from_id(id: salsa::Id) -> Self { - Self(id, std::marker::PhantomData) - } - } - unsafe impl Send for InternedString<'_> {} - - unsafe impl Sync for InternedString<'_> {} - - impl std::fmt::Debug for InternedString<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - Self::default_debug_fmt(*self, f) - } - } - impl zalsa_::SalsaStructInDb for InternedString<'_> { - type MemoIngredientMap = zalsa_::MemoIngredientSingletonIndex; - - const LEAF_TYPE_IDS: &'static [salsa::plumbing::ConstTypeId] = - &[salsa::plumbing::ConstTypeId::of::()]; - - fn lookup_ingredient_index(aux: &Zalsa) -> salsa::plumbing::IngredientIndices { - aux.lookup_jar_by_type::>() - .into() - } - - fn entries(zalsa: &zalsa_::Zalsa) -> impl Iterator + '_ { - let ingredient_index = - zalsa.lookup_jar_by_type::>(); - ::ingredient(zalsa) - .entries(zalsa) - .map(|entry| entry.key()) - } - - #[inline] - fn cast(id: zalsa_::Id, type_id: TypeId) -> Option { - if type_id == TypeId::of::() { - Some(::from_id(id)) - } else { - None - } - } - - #[inline] - unsafe fn memo_table( - zalsa: &zalsa_::Zalsa, - id: zalsa_::Id, - current_revision: zalsa_::Revision, - ) -> zalsa_::MemoTableWithTypes<'_> { - // SAFETY: Guaranteed by caller. - unsafe { - zalsa - .table() - .memos::>(id, current_revision) - } - } - } - - unsafe impl zalsa_::SalsaValue for InternedString<'_> {} - impl<'db> InternedString<'db> { - pub fn new + std::hash::Hash>( - db: &'db Db_, - data: T0, - other: impl FnOnce(InternedString<'db>) -> InternedString<'db>, - ) -> Self - where - Db_: ?Sized + salsa::Database, - String: zalsa_::HashEqLike, - { - Configuration_::ingredient(db.zalsa()).intern( - db.zalsa(), - db.zalsa_local(), - StructKey::<'db>(data, std::marker::PhantomData::default()), - |id, data| { - StructData( - zalsa_::Lookup::into_owned(data.0), - other(zalsa_::FromId::from_id(id)), - ) - }, + +#[test] +fn multiple_self_ref_fields_are_assembled_independently() { + let db = salsa::DatabaseImpl::new(); + let anchor = MultipleSelfRefs::new(&db, 0, None, None); + let both_self = MultipleSelfRefs::new(&db, 1, None, None); + let first_self = MultipleSelfRefs::new(&db, 2, None, Some(anchor)); + let second_self = MultipleSelfRefs::new(&db, 3, Some(anchor), None); + + assert!(*both_self.first(&db) == both_self); + assert!(*both_self.second(&db) == both_self); + assert!(*first_self.first(&db) == first_self); + assert!(*first_self.second(&db) == anchor); + assert!(*second_self.first(&db) == anchor); + assert!(*second_self.second(&db) == second_self); +} + +#[test] +fn debug_formats_self_ref_fields_by_id() { + use salsa::plumbing::AsId; + + salsa::DatabaseImpl::new().attach(|db| { + let value = DebugRecursive::new(db, 0, None); + let value_id = value.as_id(); + let other = DebugRecursive::new(db, 1, Some(value)); + + assert_eq!( + format!("{value:?}"), + format!("DebugRecursive {{ key: 0, other: {value_id:?} }}") + ); + assert_eq!( + format!("{other:?}"), + format!( + "DebugRecursive {{ key: 1, other: DebugRecursive {{ key: 0, other: {value_id:?} }} }}" ) - } - fn data(self, db: &'db Db_) -> String - where - Db_: ?Sized + zalsa_::Database, - { - let fields = Configuration_::ingredient(db.zalsa()).fields(db.zalsa(), self); - std::clone::Clone::clone((&fields.0)) - } - fn other(self, db: &'db Db_) -> InternedString<'db> - where - Db_: ?Sized + zalsa_::Database, - { - let fields = Configuration_::ingredient(db.zalsa()).fields(db.zalsa(), self); - std::clone::Clone::clone((&fields.1)) - } - #[doc = r" Default debug formatting for this struct (may be useful if you define your own `Debug` impl)"] - pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - zalsa_::with_attached_database(|db| { - let fields = Configuration_::ingredient(db.zalsa()).fields(db.zalsa(), this); - let mut f = f.debug_struct("InternedString"); - let f = f.field("data", &fields.0); - let f = f.field("other", &fields.1); - f.finish() - }) - .unwrap_or_else(|| { - f.debug_tuple("InternedString") - .field(&zalsa_::AsId::as_id(&this)) - .finish() - }) - } - } -}; + ); + }); +} + +#[test] +fn heap_size_uses_stored_fields() { + let db = salsa::DatabaseImpl::new(); + let mut value_data = String::with_capacity(32); + value_data.push_str("one"); + let value_capacity = value_data.capacity(); + let value = HeapRecursive::new(&db, value_data, None); + + let mut other_data = String::with_capacity(64); + other_data.push_str("four"); + let other_capacity = other_data.capacity(); + let _other = HeapRecursive::new(&db, other_data, Some(value)); + + let memory_usage = ::memory_usage(&db); + let ingredient = memory_usage + .structs + .iter() + .find(|ingredient| ingredient.debug_name() == "HeapRecursive") + .unwrap(); + + assert_eq!( + ingredient.heap_size_of_fields(), + Some(value_capacity + other_capacity) + ); +} diff --git a/tests/persistence_self_ref.rs b/tests/persistence_self_ref.rs new file mode 100644 index 000000000..11b8a9000 --- /dev/null +++ b/tests/persistence_self_ref.rs @@ -0,0 +1,42 @@ +#![cfg(all(feature = "persistence", feature = "inventory"))] + +mod common; + +#[salsa::interned(persist)] +struct SelfRefInterned<'db> { + field: String, + #[self_ref] + other: SelfRefInterned<'db>, +} + +#[test] +fn self_ref_interned_round_trip() { + use salsa::plumbing::AsId; + + let (serialized, root_id, child_id) = { + let mut db = common::EventLoggerDatabase::default(); + let root = SelfRefInterned::new(&db, "root".to_string(), None); + let child = SelfRefInterned::new(&db, "child".to_string(), Some(root)); + let root_id = root.as_id(); + let child_id = child.as_id(); + let serialized = + serde_json::to_string(&::as_serialize(&mut db)).unwrap(); + + (serialized, root_id, child_id) + }; + + let mut db = common::EventLoggerDatabase::default(); + ::deserialize( + &mut db, + &mut serde_json::Deserializer::from_str(&serialized), + ) + .unwrap(); + + let root = SelfRefInterned::new(&db, "root", None); + let child = SelfRefInterned::new(&db, "child", Some(root)); + + assert_eq!(root.as_id(), root_id); + assert_eq!(child.as_id(), child_id); + assert!(*root.other(&db) == root); + assert!(*child.other(&db) == root); +}