From 805760df2377d5859a6f43e1e6b1dc57cd7d4626 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 31 Jul 2024 18:59:51 +0200 Subject: [PATCH] Add support for `[#default]` and setting field-level durability --- components/salsa-macro-rules/src/lib.rs | 1 + .../salsa-macro-rules/src/maybe_backdate.rs | 4 +- .../salsa-macro-rules/src/maybe_clone.rs | 8 +-- .../salsa-macro-rules/src/maybe_default.rs | 32 ++++++++++ .../src/setup_input_struct.rs | 56 ++++++++++++++--- components/salsa-macros/src/input.rs | 6 ++ components/salsa-macros/src/interned.rs | 2 + components/salsa-macros/src/salsa_struct.rs | 62 ++++++++++++++++++- components/salsa-macros/src/tracked_struct.rs | 2 + src/lib.rs | 2 + tests/input_default.rs | 43 +++++++++++++ tests/input_field_durability.rs | 37 +++++++++++ 12 files changed, 239 insertions(+), 16 deletions(-) create mode 100644 components/salsa-macro-rules/src/maybe_default.rs create mode 100644 tests/input_default.rs create mode 100644 tests/input_field_durability.rs diff --git a/components/salsa-macro-rules/src/lib.rs b/components/salsa-macro-rules/src/lib.rs index 02ae8029..4834b0f2 100644 --- a/components/salsa-macro-rules/src/lib.rs +++ b/components/salsa-macro-rules/src/lib.rs @@ -15,6 +15,7 @@ mod macro_if; mod maybe_backdate; mod maybe_clone; +mod maybe_default; mod setup_accumulator_impl; mod setup_input_struct; mod setup_interned_struct; diff --git a/components/salsa-macro-rules/src/maybe_backdate.rs b/components/salsa-macro-rules/src/maybe_backdate.rs index 324b7081..22cc4fcb 100644 --- a/components/salsa-macro-rules/src/maybe_backdate.rs +++ b/components/salsa-macro-rules/src/maybe_backdate.rs @@ -2,7 +2,7 @@ #[macro_export] macro_rules! maybe_backdate { ( - ($maybe_clone:ident, no_backdate), + ($maybe_clone:ident, no_backdate, $maybe_default:ident), $field_ty:ty, $old_field_place:expr, $new_field_place:expr, @@ -20,7 +20,7 @@ macro_rules! maybe_backdate { }; ( - ($maybe_clone:ident, backdate), + ($maybe_clone:ident, backdate, $maybe_default:ident), $field_ty:ty, $old_field_place:expr, $new_field_place:expr, diff --git a/components/salsa-macro-rules/src/maybe_clone.rs b/components/salsa-macro-rules/src/maybe_clone.rs index b22c790b..313aaf9b 100644 --- a/components/salsa-macro-rules/src/maybe_clone.rs +++ b/components/salsa-macro-rules/src/maybe_clone.rs @@ -4,7 +4,7 @@ #[macro_export] macro_rules! maybe_clone { ( - (no_clone, $maybe_backdate:ident), + (no_clone, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, $field_ref_expr:expr, ) => { @@ -12,7 +12,7 @@ macro_rules! maybe_clone { }; ( - (clone, $maybe_backdate:ident), + (clone, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, $field_ref_expr:expr, ) => { @@ -23,7 +23,7 @@ macro_rules! maybe_clone { #[macro_export] macro_rules! maybe_cloned_ty { ( - (no_clone, $maybe_backdate:ident), + (no_clone, $maybe_backdate:ident, $maybe_default:ident), $db_lt:lifetime, $field_ty:ty ) => { @@ -31,7 +31,7 @@ macro_rules! maybe_cloned_ty { }; ( - (clone, $maybe_backdate:ident), + (clone, $maybe_backdate:ident, $maybe_default:ident), $db_lt:lifetime, $field_ty:ty ) => { diff --git a/components/salsa-macro-rules/src/maybe_default.rs b/components/salsa-macro-rules/src/maybe_default.rs new file mode 100644 index 00000000..a1dd5b7b --- /dev/null +++ b/components/salsa-macro-rules/src/maybe_default.rs @@ -0,0 +1,32 @@ +/// Generate either `field_ref_expr` or `field_ty::default` +/// +/// Used when generating an input's builder. +#[macro_export] +macro_rules! maybe_default { + ( + ($maybe_clone:ident, $maybe_backdate:ident, default), + $field_ty:ty, + $field_ref_expr:expr, + ) => { + <$field_ty>::default() + }; + + ( + ($maybe_clone:ident, $maybe_backdate:ident, required), + $field_ty:ty, + $field_ref_expr:expr, + ) => { + $field_ref_expr + }; +} + +#[macro_export] +macro_rules! maybe_default_tt { + (($maybe_clone:ident, $maybe_backdate:ident, default) => $($t:tt)*) => { + $($t)* + }; + + (($maybe_clone:ident, $maybe_backdate:ident, required) => $($t:tt)*) => { + + }; +} diff --git a/components/salsa-macro-rules/src/setup_input_struct.rs b/components/salsa-macro-rules/src/setup_input_struct.rs index dfe220cc..a2a1d2fb 100644 --- a/components/salsa-macro-rules/src/setup_input_struct.rs +++ b/components/salsa-macro-rules/src/setup_input_struct.rs @@ -32,6 +32,12 @@ macro_rules! setup_input_struct { // Indices for each field from 0..N -- must be unsuffixed (e.g., `0`, `1`). field_indices: [$($field_index:tt),*], + // Fields that are required (have no default value). Each item is the fields name and type. + required_fields: [$($required_field_id:ident $required_field_ty:ty),*], + + // Names for the field durability methods on the builder (typically `foo_durability`) + field_durability_ids: [$($field_durability_id:ident),*], + // Number of fields num_fields: $N:literal, @@ -123,19 +129,20 @@ macro_rules! setup_input_struct { impl $Struct { #[inline] - pub fn $new_fn<$Db>(db: &$Db, $($field_id: $field_ty),*) -> Self + pub fn $new_fn<$Db>(db: &$Db, $($required_field_id: $required_field_ty),*) -> Self where // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + salsa::Database, { - Self::builder($($field_id,)*).new(db) + Self::builder($($required_field_id,)*).new(db) } - pub fn builder($($field_id: $field_ty),*) -> ::Builder + pub fn builder($($required_field_id: $required_field_ty),*) -> ::Builder { // Implement `new` here instead of inside the builder module // because $Configuration can't be named in `builder`. impl builder::$Builder { + /// Creates the new input with the set values. pub fn new<$Db>(self, db: &$Db) -> $Struct where // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` @@ -148,7 +155,7 @@ macro_rules! setup_input_struct { } } - builder::new_builder($($field_id,)*) + builder::new_builder($($zalsa::maybe_default!($field_option, $field_ty, $field_id,)),*) } $( @@ -238,25 +245,56 @@ macro_rules! setup_input_struct { // These are standalone functions instead of methods on `Builder` to prevent // that the enclosing module can call them. pub(super) fn new_builder($($field_id: $field_ty),*) -> $Builder { - $Builder { fields: ($($field_id,)*), durability: $zalsa::Durability::default() } + $Builder { + fields: ($($field_id,)*), + durabilities: [$zalsa::Durability::default(); $N], + } } pub(super) fn builder_into_inner(builder: $Builder, revision: $zalsa::Revision) -> (($($field_ty,)*), $zalsa::Array<$zalsa::Stamp, $N>) { - let stamps = $zalsa::Array::new([$zalsa::stamp(revision, builder.durability); $N]); + let stamps = $zalsa::Array::new([ + $($zalsa::stamp(revision, builder.durabilities[$field_index])),* + ]); + (builder.fields, stamps) } pub struct $Builder { + /// The field values. fields: ($($field_ty,)*), - durability: $zalsa::Durability, + + /// The durabilities per field. + durabilities: [$zalsa::Durability; $N], } impl $Builder { - /// Sets the durability of all fields + /// Sets the durability of all fields. + /// + /// Overrides any previously set durabilities. pub fn durability(mut self, durability: $zalsa::Durability) -> Self { - self.durability = durability; + self.durabilities = [durability; $N]; self } + + $($zalsa::maybe_default_tt! { $field_option => + /// Sets the value of the field `$field_id`. + #[must_use] + pub fn $field_id(mut self, value: $field_ty) -> Self + { + self.fields.$field_index = value; + self + } + })* + + $( + /// Sets the durability for the field `$field_id`. + #[must_use] + pub fn $field_durability_id(mut self, durability: $zalsa::Durability) -> Self + { + self.durabilities[$field_index] = durability; + self + } + )* } } }; diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index 4040bf61..e5d67d53 100644 --- a/components/salsa-macros/src/input.rs +++ b/components/salsa-macros/src/input.rs @@ -62,6 +62,8 @@ impl SalsaStructAllowedOptions for InputStruct { const ALLOW_ID: bool = false; const HAS_LIFETIME: bool = false; + + const ALLOW_DEFAULT: bool = true; } struct Macro { @@ -85,8 +87,10 @@ impl Macro { let field_vis = salsa_struct.field_vis(); let field_getter_ids = salsa_struct.field_getter_ids(); let field_setter_ids = salsa_struct.field_setter_ids(); + let required_fields = salsa_struct.required_fields(); let field_options = salsa_struct.field_options(); let field_tys = salsa_struct.field_tys(); + let field_durability_ids = salsa_struct.field_durability_ids(); let is_singleton = self.args.singleton.is_some(); let generate_debug_impl = salsa_struct.generate_debug_impl(); @@ -111,6 +115,8 @@ impl Macro { field_setters: [#(#field_vis #field_setter_ids),*], field_tys: [#(#field_tys),*], field_indices: [#(#field_indices),*], + required_fields: [#(#required_fields),*], + field_durability_ids: [#(#field_durability_ids),*], num_fields: #num_fields, is_singleton: #is_singleton, generate_debug_impl: #generate_debug_impl, diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index 50862b88..b0fdb327 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -63,6 +63,8 @@ impl SalsaStructAllowedOptions for InternedStruct { const ALLOW_ID: bool = false; const HAS_LIFETIME: bool = true; + + const ALLOW_DEFAULT: bool = false; } struct Macro { diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs index 47027336..b1e9ad2c 100644 --- a/components/salsa-macros/src/salsa_struct.rs +++ b/components/salsa-macros/src/salsa_struct.rs @@ -47,12 +47,16 @@ pub(crate) trait SalsaStructAllowedOptions: AllowedOptions { /// Does this kind of struct have a `'db` lifetime? const HAS_LIFETIME: bool; + + /// Are `#[default]` fields allowed? + const ALLOW_DEFAULT: bool; } pub(crate) struct SalsaField<'s> { field: &'s syn::Field, pub(crate) has_id_attr: bool, + pub(crate) has_default_attr: bool, pub(crate) has_ref_attr: bool, pub(crate) has_no_eq_attr: bool, get_name: syn::Ident, @@ -64,6 +68,7 @@ const BANNED_FIELD_NAMES: &[&str] = &["from", "new"]; #[allow(clippy::type_complexity)] pub(crate) const FIELD_OPTION_ATTRIBUTES: &[(&str, fn(&syn::Attribute, &mut SalsaField))] = &[ ("id", |_, ef| ef.has_id_attr = true), + ("default", |_, ef| ef.has_default_attr = true), ("return_ref", |_, ef| ef.has_ref_attr = true), ("no_eq", |_, ef| ef.has_no_eq_attr = true), ("get", |attr, ef| { @@ -99,6 +104,7 @@ where }; this.maybe_disallow_id_fields()?; + this.maybe_disallow_default_fields()?; this.check_generics()?; @@ -138,6 +144,31 @@ where Ok(()) } + /// Disallow `#[default]` attributes on the fields of this struct. + /// + /// If an `#[default]` field is found, return an error. + /// + /// # Parameters + /// + /// * `kind`, the attribute name (e.g., `input` or `interned`) + fn maybe_disallow_default_fields(&self) -> syn::Result<()> { + if A::ALLOW_DEFAULT { + return Ok(()); + } + + // Check if any field has the `#[id]` attribute. + for ef in &self.fields { + if ef.has_default_attr { + return Err(syn::Error::new_spanned( + ef.field, + format!("`#[id]` cannot be used with `#[salsa::{}]`", A::KIND), + )); + } + } + + Ok(()) + } + /// Check that the generic parameters look as expected for this kind of struct. fn check_generics(&self) -> syn::Result<()> { if A::HAS_LIFETIME { @@ -173,6 +204,21 @@ where .collect() } + pub(crate) fn required_fields(&self) -> Vec { + self.fields + .iter() + .filter_map(|f| { + if f.has_default_attr { + None + } else { + let ident = f.field.ident.as_ref().unwrap(); + let ty = &f.field.ty; + Some(quote!(#ident #ty)) + } + }) + .collect() + } + pub(crate) fn field_vis(&self) -> Vec<&syn::Visibility> { self.fields.iter().map(|f| &f.field.vis).collect() } @@ -185,6 +231,13 @@ where self.fields.iter().map(|f| &f.set_name).collect() } + pub(crate) fn field_durability_ids(&self) -> Vec { + self.fields + .iter() + .map(|f| quote::format_ident!("{}_durability", f.field.ident.as_ref().unwrap())) + .collect() + } + pub(crate) fn field_tys(&self) -> Vec<&syn::Type> { self.fields.iter().map(|f| &f.field.ty).collect() } @@ -205,7 +258,13 @@ where syn::Ident::new("backdate", Span::call_site()) }; - quote!((#clone_ident, #backdate_ident)) + let default_ident = if f.has_default_attr { + syn::Ident::new("default", Span::call_site()) + } else { + syn::Ident::new("required", Span::call_site()) + }; + + quote!((#clone_ident, #backdate_ident, #default_ident)) }) .collect() } @@ -235,6 +294,7 @@ impl<'s> SalsaField<'s> { field, has_id_attr: false, has_ref_attr: false, + has_default_attr: false, has_no_eq_attr: false, get_name, set_name, diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs index 9af4b035..1730b340 100644 --- a/components/salsa-macros/src/tracked_struct.rs +++ b/components/salsa-macros/src/tracked_struct.rs @@ -58,6 +58,8 @@ impl SalsaStructAllowedOptions for TrackedStruct { const ALLOW_ID: bool = true; const HAS_LIFETIME: bool = true; + + const ALLOW_DEFAULT: bool = false; } struct Macro { diff --git a/src/lib.rs b/src/lib.rs index 6f671be1..c28230e0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -112,6 +112,8 @@ pub mod plumbing { pub use salsa_macro_rules::maybe_backdate; pub use salsa_macro_rules::maybe_clone; pub use salsa_macro_rules::maybe_cloned_ty; + pub use salsa_macro_rules::maybe_default; + pub use salsa_macro_rules::maybe_default_tt; pub use salsa_macro_rules::setup_accumulator_impl; pub use salsa_macro_rules::setup_input_struct; pub use salsa_macro_rules::setup_interned_struct; diff --git a/tests/input_default.rs b/tests/input_default.rs new file mode 100644 index 00000000..93990a96 --- /dev/null +++ b/tests/input_default.rs @@ -0,0 +1,43 @@ +//! Tests that fields attributed with `#[default]` are initialized with `Default::default()`. + +use salsa::{default_database, Durability}; +use test_log::test; + +#[salsa::input] +struct MyInput { + required: bool, + #[default] + optional: usize, +} + +#[test] +fn new_constructor() { + let db = default_database(); + + let input = MyInput::new(&db, true); + + assert_eq!(input.required(&db), true); + assert_eq!(input.optional(&db), 0); +} + +#[test] +fn builder_specify_optional() { + let db = default_database(); + + let input = MyInput::builder(true).optional(20).new(&db); + + assert_eq!(input.required(&db), true); + assert_eq!(input.optional(&db), 20); +} + +#[test] +fn builder_default_optional_value() { + let db = default_database(); + + let input = MyInput::builder(true) + .required_durability(Durability::HIGH) + .new(&db); + + assert_eq!(input.required(&db), true); + assert_eq!(input.optional(&db), 0); +} diff --git a/tests/input_field_durability.rs b/tests/input_field_durability.rs new file mode 100644 index 00000000..fba52f79 --- /dev/null +++ b/tests/input_field_durability.rs @@ -0,0 +1,37 @@ +//! Tests that code using the builder's durability methods compiles. + +use salsa::{default_database, Durability}; +use test_log::test; + +#[salsa::input] +struct MyInput { + required_field: bool, + + #[default] + optional_field: usize, +} + +#[test] +fn required_field_durability() { + let db = default_database(); + + let input = MyInput::builder(true) + .required_field_durability(Durability::HIGH) + .new(&db); + + assert_eq!(input.required_field(&db), true); + assert_eq!(input.optional_field(&db), 0); +} + +#[test] +fn optional_field_durability() { + let db = default_database(); + + let input = MyInput::builder(true) + .optional_field(20) + .optional_field_durability(Durability::HIGH) + .new(&db); + + assert_eq!(input.required_field(&db), true); + assert_eq!(input.optional_field(&db), 20); +}