From 0cd932ffcdd3ec21d5982513a899902aea07da5c Mon Sep 17 00:00:00 2001 From: CheaterCodes Date: Sat, 22 Mar 2025 01:27:23 +0100 Subject: [PATCH 1/5] Changed `return_ref` syntax to `returns(as_ref)` and `returns(cloned)` --- benches/compare.rs | 4 ++-- benches/incremental.rs | 2 +- book/src/overview.md | 12 +++++----- book/src/tutorial/ir.md | 2 +- components/salsa-macro-rules/src/lib.rs | 2 +- .../salsa-macro-rules/src/maybe_backdate.rs | 4 ++-- .../salsa-macro-rules/src/maybe_default.rs | 8 +++---- .../src/{maybe_clone.rs => return_mode.rs} | 12 +++++----- .../src/setup_input_struct.rs | 4 ++-- .../src/setup_interned_struct.rs | 4 ++-- .../salsa-macro-rules/src/setup_tracked_fn.rs | 20 ++++------------ .../src/setup_tracked_struct.rs | 20 ++++++++-------- components/salsa-macros/src/accumulator.rs | 2 +- components/salsa-macros/src/input.rs | 2 +- components/salsa-macros/src/interned.rs | 2 +- components/salsa-macros/src/options.rs | 24 +++++++++++-------- components/salsa-macros/src/salsa_struct.rs | 18 +++++++------- components/salsa-macros/src/tracked_fn.rs | 12 ++++++---- components/salsa-macros/src/tracked_impl.rs | 6 ++--- components/salsa-macros/src/tracked_struct.rs | 2 +- examples/calc/ir.rs | 12 +++++----- examples/lazy-input/main.rs | 4 ++-- src/lib.rs | 2 +- tests/accumulate-reuse-workaround.rs | 2 +- .../compile-fail/accumulator_incompatibles.rs | 2 +- .../accumulator_incompatibles.stderr | 6 ++--- .../input_struct_incompatibles.rs | 2 +- .../input_struct_incompatibles.stderr | 6 ++--- .../interned_struct_incompatibles.rs | 2 +- .../interned_struct_incompatibles.stderr | 6 ++--- tests/compile-fail/tracked_fn_return_ref.rs | 2 +- .../tracked_impl_incompatibles.rs | 2 +- .../tracked_impl_incompatibles.stderr | 4 ++-- .../tracked_struct_incompatibles.rs | 2 +- .../tracked_struct_incompatibles.stderr | 6 ++--- tests/cycle.rs | 2 +- tests/cycle_maybe_changed_after.rs | 4 ++-- tests/cycle_tracked.rs | 6 ++--- tests/deletion-drops.rs | 2 +- tests/override_new_get_set.rs | 2 +- tests/tracked_fn_return_ref.rs | 2 +- tests/tracked_method.rs | 2 +- tests/tracked_method_inherent_return_ref.rs | 2 +- tests/tracked_method_on_tracked_struct.rs | 4 ++-- tests/tracked_method_trait_return_ref.rs | 2 +- tests/warnings/needless_borrow.rs | 2 +- tests/warnings/needless_lifetimes.rs | 4 ++-- 47 files changed, 126 insertions(+), 130 deletions(-) rename components/salsa-macro-rules/src/{maybe_clone.rs => return_mode.rs} (64%) diff --git a/benches/compare.rs b/benches/compare.rs index 3a1d6bb76..26ee36866 100644 --- a/benches/compare.rs +++ b/benches/compare.rs @@ -10,7 +10,7 @@ include!("shims/global_alloc_overwrite.rs"); #[salsa::input] pub struct Input { - #[return_ref] + #[returns(as_ref)] pub text: String, } @@ -22,7 +22,7 @@ pub fn length(db: &dyn salsa::Database, input: Input) -> usize { #[salsa::interned] pub struct InternedInput<'db> { - #[return_ref] + #[returns(as_ref)] pub text: String, } diff --git a/benches/incremental.rs b/benches/incremental.rs index 8cc9dab8c..432c958b1 100644 --- a/benches/incremental.rs +++ b/benches/incremental.rs @@ -15,7 +15,7 @@ struct Tracked<'db> { number: usize, } -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(as_ref))] #[inline(never)] fn index<'db>(db: &'db dyn salsa::Database, input: Input) -> Vec> { (0..input.field(db)).map(|i| Tracked::new(db, i)).collect() diff --git a/book/src/overview.md b/book/src/overview.md index 7e01b0a84..3c0c00f54 100644 --- a/book/src/overview.md +++ b/book/src/overview.md @@ -89,13 +89,13 @@ let contents: String = file.contents(&db); ``` Invoking the accessor clones the value from the database. -Sometimes this is not what you want, so you can annotate fields with `#[return_ref]` to indicate that they should return a reference into the database instead: +Sometimes this is not what you want, so you can annotate fields with `#[returns(as_ref)]` to indicate that they should return a reference into the database instead: ```rust #[salsa::input] pub struct ProgramFile { pub path: PathBuf, - #[return_ref] + #[returns(as_ref)] pub contents: String, } ``` @@ -145,7 +145,7 @@ Tracked functions have to follow a particular structure: - They must take a "Salsa struct" as the second argument -- in our example, this is an input struct, but there are other kinds of Salsa structs we'll describe shortly. - They _can_ take additional arguments, but it's faster and better if they don't. -Tracked functions can return any clone-able type. A clone is required since, when the value is cached, the result will be cloned out of the database. Tracked functions can also be annotated with `#[return_ref]` if you would prefer to return a reference into the database instead (if `parse_file` were so annotated, then callers would actually get back an `&Ast`, for example). +Tracked functions can return any clone-able type. A clone is required since, when the value is cached, the result will be cloned out of the database. Tracked functions can also be annotated with `#[returns(as_ref)]` if you would prefer to return a reference into the database instead (if `parse_file` were so annotated, then callers would actually get back an `&Ast`, for example). ## Tracked structs @@ -158,7 +158,7 @@ Example: ```rust #[salsa::tracked] struct Ast<'db> { - #[return_ref] + #[returns(as_ref)] top_level_items: Vec, } ``` @@ -252,7 +252,7 @@ Most compilers, for example, will define a type to represent a user identifier: ```rust #[salsa::interned] struct Word { - #[return_ref] + #[returns(as_ref)] pub text: String, } ``` @@ -269,7 +269,7 @@ let w3 = Word::new(db, "foo".to_string()); When you create two interned structs with the same field values, you are guaranteed to get back the same integer id. So here, we know that `assert_eq!(w1, w3)` is true and `assert_ne!(w1, w2)`. -You can access the fields of an interned struct using a getter, like `word.text(db)`. These getters respect the `#[return_ref]` annotation. Like tracked structs, the fields of interned structs are immutable. +You can access the fields of an interned struct using a getter, like `word.text(db)`. These getters respect the `#[returns(as_ref)]` annotation. Like tracked structs, the fields of interned structs are immutable. ## Accumulators diff --git a/book/src/tutorial/ir.md b/book/src/tutorial/ir.md index 7949c60bc..56c8ab6df 100644 --- a/book/src/tutorial/ir.md +++ b/book/src/tutorial/ir.md @@ -92,7 +92,7 @@ Apart from the fields being immutable, the API for working with a tracked struct - You can create a new value by using `new`: e.g., `Program::new(&db, some_statements)` - You use a getter to read the value of a field, just like with an input (e.g., `my_func.statements(db)` to read the `statements` field). - - In this case, the field is tagged as `#[return_ref]`, which means that the getter will return a `&Vec`, instead of cloning the vector. + - In this case, the field is tagged as `#[returns(as_ref)]`, which means that the getter will return a `&Vec`, instead of cloning the vector. ### The `'db` lifetime diff --git a/components/salsa-macro-rules/src/lib.rs b/components/salsa-macro-rules/src/lib.rs index 4834b0f2d..45f75355d 100644 --- a/components/salsa-macro-rules/src/lib.rs +++ b/components/salsa-macro-rules/src/lib.rs @@ -14,8 +14,8 @@ mod macro_if; mod maybe_backdate; -mod maybe_clone; mod maybe_default; +mod return_mode; 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 22cc4fcb4..bdbb3aa54 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_default:ident), + ($return_mode: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_default:ident), + ($return_mode: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_default.rs b/components/salsa-macro-rules/src/maybe_default.rs index a1dd5b7bb..1786ffcc4 100644 --- a/components/salsa-macro-rules/src/maybe_default.rs +++ b/components/salsa-macro-rules/src/maybe_default.rs @@ -4,7 +4,7 @@ #[macro_export] macro_rules! maybe_default { ( - ($maybe_clone:ident, $maybe_backdate:ident, default), + ($return_mode:ident, $maybe_backdate:ident, default), $field_ty:ty, $field_ref_expr:expr, ) => { @@ -12,7 +12,7 @@ macro_rules! maybe_default { }; ( - ($maybe_clone:ident, $maybe_backdate:ident, required), + ($return_mode:ident, $maybe_backdate:ident, required), $field_ty:ty, $field_ref_expr:expr, ) => { @@ -22,11 +22,11 @@ macro_rules! maybe_default { #[macro_export] macro_rules! maybe_default_tt { - (($maybe_clone:ident, $maybe_backdate:ident, default) => $($t:tt)*) => { + (($return_mode:ident, $maybe_backdate:ident, default) => $($t:tt)*) => { $($t)* }; - (($maybe_clone:ident, $maybe_backdate:ident, required) => $($t:tt)*) => { + (($return_mode:ident, $maybe_backdate:ident, required) => $($t:tt)*) => { }; } diff --git a/components/salsa-macro-rules/src/maybe_clone.rs b/components/salsa-macro-rules/src/return_mode.rs similarity index 64% rename from components/salsa-macro-rules/src/maybe_clone.rs rename to components/salsa-macro-rules/src/return_mode.rs index 313aaf9b0..9fdfa1959 100644 --- a/components/salsa-macro-rules/src/maybe_clone.rs +++ b/components/salsa-macro-rules/src/return_mode.rs @@ -2,9 +2,9 @@ /// /// Used when generating field getters. #[macro_export] -macro_rules! maybe_clone { +macro_rules! return_mode { ( - (no_clone, $maybe_backdate:ident, $maybe_default:ident), + (as_ref, $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, $maybe_default:ident), + (cloned, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, $field_ref_expr:expr, ) => { @@ -21,9 +21,9 @@ macro_rules! maybe_clone { } #[macro_export] -macro_rules! maybe_cloned_ty { +macro_rules! return_mode_ty { ( - (no_clone, $maybe_backdate:ident, $maybe_default:ident), + (as_ref, $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, $maybe_default:ident), + (cloned, $maybe_backdate:ident, $maybe_default:ident), $db_lt:lifetime, $field_ty:ty ) => { diff --git a/components/salsa-macro-rules/src/setup_input_struct.rs b/components/salsa-macro-rules/src/setup_input_struct.rs index 72e18343e..e44c8c9d8 100644 --- a/components/salsa-macro-rules/src/setup_input_struct.rs +++ b/components/salsa-macro-rules/src/setup_input_struct.rs @@ -182,7 +182,7 @@ macro_rules! setup_input_struct { } $( - $field_getter_vis fn $field_getter_id<'db, $Db>(self, db: &'db $Db) -> $zalsa::maybe_cloned_ty!($field_option, 'db, $field_ty) + $field_getter_vis fn $field_getter_id<'db, $Db>(self, db: &'db $Db) -> $zalsa::return_mode_ty!($field_option, 'db, $field_ty) where // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, @@ -192,7 +192,7 @@ macro_rules! setup_input_struct { self, $field_index, ); - $zalsa::maybe_clone!( + $zalsa::return_mode!( $field_option, $field_ty, &fields.$field_index, diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs index 8c2cda3c6..07cf35cf6 100644 --- a/components/salsa-macro-rules/src/setup_interned_struct.rs +++ b/components/salsa-macro-rules/src/setup_interned_struct.rs @@ -215,13 +215,13 @@ macro_rules! setup_interned_struct { } $( - $field_getter_vis fn $field_getter_id<$Db>(self, db: &'db $Db) -> $zalsa::maybe_cloned_ty!($field_option, 'db, $field_ty) + $field_getter_vis fn $field_getter_id<$Db>(self, db: &'db $Db) -> $zalsa::return_mode_ty!($field_option, 'db, $field_ty) where // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, { let fields = $Configuration::ingredient(db).fields(db.as_dyn_database(), self); - $zalsa::maybe_clone!( + $zalsa::return_mode!( $field_option, $field_ty, &fields.$field_index, diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index 6b2139963..6d920032b 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/components/salsa-macro-rules/src/setup_tracked_fn.rs @@ -55,8 +55,8 @@ macro_rules! setup_tracked_fn { // LRU capacity (a literal, maybe 0) lru: $lru:tt, - // True if we `return_ref` flag was given to the function - return_ref: $return_ref:tt, + // The return mode for the function, either `as_ref` or `cloned` + return_mode: $return_mode:tt, assert_return_type_is_update: {$($assert_return_type_is_update:tt)*}, @@ -80,13 +80,7 @@ macro_rules! setup_tracked_fn { $vis fn $fn_name<$db_lt>( $db: &$db_lt dyn $Db, $($input_id: $input_ty,)* - ) -> salsa::plumbing::macro_if! { - if $return_ref { - &$db_lt $output_ty - } else { - $output_ty - } - } { + ) -> salsa::plumbing::return_mode_ty!(($return_mode, __, __), $db_lt, $output_ty) { use salsa::plumbing as $zalsa; struct $Configuration; @@ -372,13 +366,7 @@ macro_rules! setup_tracked_fn { } }; - $zalsa::macro_if! { - if $return_ref { - result - } else { - <$output_ty as std::clone::Clone>::clone(result) - } - } + $zalsa::return_mode!(($return_mode, __, __), $output_ty, result,) }) } // The struct needs be last in the macro expansion in order to make the tracked diff --git a/components/salsa-macro-rules/src/setup_tracked_struct.rs b/components/salsa-macro-rules/src/setup_tracked_struct.rs index 34c740a02..eafe46801 100644 --- a/components/salsa-macro-rules/src/setup_tracked_struct.rs +++ b/components/salsa-macro-rules/src/setup_tracked_struct.rs @@ -52,24 +52,24 @@ macro_rules! setup_tracked_struct { // A set of "field options" for each tracked field. // - // Each field option is a tuple `(maybe_clone, maybe_backdate)` where: + // Each field option is a tuple `(return_mode, maybe_backdate)` where: // - // * `maybe_clone` is either the identifier `clone` or `no_clone` + // * `return_mode` is either the identifier `as_ref` or `cloned` // * `maybe_backdate` is either the identifier `backdate` or `no_backdate` // // These are used to drive conditional logic for each field via recursive macro invocation - // (see e.g. @maybe_clone below). + // (see e.g. @return_mode below). tracked_options: [$($tracked_option:tt),*], // A set of "field options" for each untracked field. // - // Each field option is a tuple `(maybe_clone, maybe_backdate)` where: + // Each field option is a tuple `(return_mode, maybe_backdate)` where: // - // * `maybe_clone` is either the identifier `clone` or `no_clone` + // * `return_mode` is either the identifier `as_ref` or `cloned` // * `maybe_backdate` is either the identifier `backdate` or `no_backdate` // // These are used to drive conditional logic for each field via recursive macro invocation - // (see e.g. @maybe_clone below). + // (see e.g. @return_mode below). untracked_options: [$($untracked_option:tt),*], // Number of tracked fields. @@ -260,14 +260,14 @@ macro_rules! setup_tracked_struct { } $( - $tracked_getter_vis fn $tracked_getter_id<$Db>(self, db: &$db_lt $Db) -> $crate::maybe_cloned_ty!($tracked_option, $db_lt, $tracked_ty) + $tracked_getter_vis fn $tracked_getter_id<$Db>(self, db: &$db_lt $Db) -> $crate::return_mode_ty!($tracked_option, $db_lt, $tracked_ty) where // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, { let db = db.as_dyn_database(); let fields = $Configuration::ingredient(db).tracked_field(db, self, $relative_tracked_index); - $crate::maybe_clone!( + $crate::return_mode!( $tracked_option, $tracked_ty, &fields.$absolute_tracked_index, @@ -276,14 +276,14 @@ macro_rules! setup_tracked_struct { )* $( - $untracked_getter_vis fn $untracked_getter_id<$Db>(self, db: &$db_lt $Db) -> $crate::maybe_cloned_ty!($untracked_option, $db_lt, $untracked_ty) + $untracked_getter_vis fn $untracked_getter_id<$Db>(self, db: &$db_lt $Db) -> $crate::return_mode_ty!($untracked_option, $db_lt, $untracked_ty) where // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database` $Db: ?Sized + $zalsa::Database, { let db = db.as_dyn_database(); let fields = $Configuration::ingredient(db).untracked_field(db, self); - $crate::maybe_clone!( + $crate::return_mode!( $untracked_option, $untracked_ty, &fields.$absolute_untracked_index, diff --git a/components/salsa-macros/src/accumulator.rs b/components/salsa-macros/src/accumulator.rs index 220d0e941..8973f0fa3 100644 --- a/components/salsa-macros/src/accumulator.rs +++ b/components/salsa-macros/src/accumulator.rs @@ -29,7 +29,7 @@ pub(crate) fn accumulator( struct Accumulator; impl AllowedOptions for Accumulator { - const RETURN_REF: bool = false; + const RETURNS: bool = false; const SPECIFY: bool = false; const NO_EQ: bool = false; const DEBUG: bool = false; diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index 38241a959..51410da4d 100644 --- a/components/salsa-macros/src/input.rs +++ b/components/salsa-macros/src/input.rs @@ -33,7 +33,7 @@ type InputArgs = Options; struct InputStruct; impl crate::options::AllowedOptions for InputStruct { - const RETURN_REF: bool = false; + const RETURNS: bool = false; const SPECIFY: bool = false; diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index 646470fce..b07a11aa5 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -33,7 +33,7 @@ type InternedArgs = Options; struct InternedStruct; impl crate::options::AllowedOptions for InternedStruct { - const RETURN_REF: bool = false; + const RETURNS: bool = false; const SPECIFY: bool = false; diff --git a/components/salsa-macros/src/options.rs b/components/salsa-macros/src/options.rs index 4852ee2be..56a15842f 100644 --- a/components/salsa-macros/src/options.rs +++ b/components/salsa-macros/src/options.rs @@ -1,6 +1,7 @@ use std::marker::PhantomData; use syn::ext::IdentExt; +use syn::parenthesized; use syn::spanned::Spanned; /// "Options" are flags that can be supplied to the various salsa related @@ -10,10 +11,10 @@ use syn::spanned::Spanned; /// trait. #[derive(Debug)] pub(crate) struct Options { - /// The `return_ref` option is used to signal that field/return type is "by ref" + /// The `returns` option is used to signal whether the field/return type is cloned or returned as a reference /// - /// If this is `Some`, the value is the `ref` identifier. - pub return_ref: Option, + /// If this is `Some`, the value is the return mode (`as_ref`, `cloned`). + pub returns: Option, /// The `no_eq` option is used to signal that a given field does not implement /// the `Eq` trait and cannot be compared for equality. @@ -96,7 +97,7 @@ pub(crate) struct Options { impl Default for Options { fn default() -> Self { Self { - return_ref: Default::default(), + returns: Default::default(), specify: Default::default(), no_eq: Default::default(), debug: Default::default(), @@ -118,7 +119,7 @@ impl Default for Options { /// These flags determine which options are allowed in a given context pub(crate) trait AllowedOptions { - const RETURN_REF: bool; + const RETURNS: bool; const SPECIFY: bool; const NO_EQ: bool; const DEBUG: bool; @@ -144,18 +145,21 @@ impl syn::parse::Parse for Options { while !input.is_empty() { let ident: syn::Ident = syn::Ident::parse_any(input)?; - if ident == "return_ref" { - if A::RETURN_REF { - if let Some(old) = options.return_ref.replace(ident) { + if ident == "returns" { + let content; + parenthesized!(content in input); + let mode = content.parse()?; + if A::RETURNS { + if let Some(old) = options.returns.replace(mode) { return Err(syn::Error::new( old.span(), - "option `return_ref` provided twice", + "option `returns` provided twice", )); } } else { return Err(syn::Error::new( ident.span(), - "`return_ref` option not allowed here", + "`returns` option not allowed here", )); } } else if ident == "no_eq" { diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs index c480ece4f..12171d8b8 100644 --- a/components/salsa-macros/src/salsa_struct.rs +++ b/components/salsa-macros/src/salsa_struct.rs @@ -26,6 +26,7 @@ //! * this could be optimized, particularly for interned fields use proc_macro2::{Ident, Literal, Span, TokenStream}; +use syn::spanned::Spanned; use crate::db_lifetime; use crate::options::{AllowedOptions, Options}; @@ -58,7 +59,7 @@ pub(crate) struct SalsaField<'s> { pub(crate) has_tracked_attr: bool, pub(crate) has_default_attr: bool, - pub(crate) has_ref_attr: bool, + pub(crate) returns: syn::Ident, pub(crate) has_no_eq_attr: bool, get_name: syn::Ident, set_name: syn::Ident, @@ -70,7 +71,9 @@ const BANNED_FIELD_NAMES: &[&str] = &["from", "new"]; pub(crate) const FIELD_OPTION_ATTRIBUTES: &[(&str, fn(&syn::Attribute, &mut SalsaField))] = &[ ("tracked", |_, ef| ef.has_tracked_attr = true), ("default", |_, ef| ef.has_default_attr = true), - ("return_ref", |_, ef| ef.has_ref_attr = true), + ("returns", |attr, ef| { + ef.returns = attr.parse_args().unwrap(); + }), ("no_eq", |_, ef| ef.has_no_eq_attr = true), ("get", |attr, ef| { ef.get_name = attr.parse_args().unwrap(); @@ -364,10 +367,11 @@ impl<'s> SalsaField<'s> { let get_name = Ident::new(&field_name_str, field_name.span()); let set_name = Ident::new(&format!("set_{field_name_str}",), field_name.span()); + let returns = Ident::new("cloned", field.span()); let mut result = SalsaField { field, has_tracked_attr: false, - has_ref_attr: false, + returns, has_default_attr: false, has_no_eq_attr: false, get_name, @@ -387,11 +391,7 @@ impl<'s> SalsaField<'s> { } fn options(&self) -> TokenStream { - let clone_ident = if self.has_ref_attr { - syn::Ident::new("no_clone", Span::call_site()) - } else { - syn::Ident::new("clone", Span::call_site()) - }; + let returns = &self.returns; let backdate_ident = if self.has_no_eq_attr { syn::Ident::new("no_backdate", Span::call_site()) @@ -405,6 +405,6 @@ impl<'s> SalsaField<'s> { syn::Ident::new("required", Span::call_site()) }; - quote!((#clone_ident, #backdate_ident, #default_ident)) + quote!((#returns, #backdate_ident, #default_ident)) } } diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index e8f4b71ff..58686144c 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/components/salsa-macros/src/tracked_fn.rs @@ -1,7 +1,7 @@ use proc_macro2::{Literal, Span, TokenStream}; use quote::ToTokens; use syn::spanned::Spanned; -use syn::ItemFn; +use syn::{Ident, ItemFn}; use crate::hygiene::Hygiene; use crate::options::Options; @@ -26,7 +26,7 @@ pub type FnArgs = Options; pub struct TrackedFn; impl crate::options::AllowedOptions for TrackedFn { - const RETURN_REF: bool = true; + const RETURNS: bool = true; const SPECIFY: bool = true; @@ -146,7 +146,11 @@ impl Macro { let lru = Literal::usize_unsuffixed(self.args.lru.unwrap_or(0)); - let return_ref: bool = self.args.return_ref.is_some(); + let return_mode = self + .args + .returns + .clone() + .unwrap_or(Ident::new("cloned", Span::call_site())); // 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 @@ -183,7 +187,7 @@ impl Macro { values_equal: {#eq}, needs_interner: #needs_interner, lru: #lru, - return_ref: #return_ref, + return_mode: #return_mode, assert_return_type_is_update: { #assert_return_type_is_update }, unused_names: [ #zalsa, diff --git a/components/salsa-macros/src/tracked_impl.rs b/components/salsa-macros/src/tracked_impl.rs index 27115f342..faa387da1 100644 --- a/components/salsa-macros/src/tracked_impl.rs +++ b/components/salsa-macros/src/tracked_impl.rs @@ -296,13 +296,13 @@ impl Macro { args: &FnArgs, db_lt: &Option, ) -> syn::Result<()> { - if let Some(return_ref) = &args.return_ref { + if let Some(returns) = &args.returns { if let syn::ReturnType::Type(_, t) = &mut sig.output { **t = parse_quote!(& #db_lt #t) } else { return Err(syn::Error::new_spanned( - return_ref, - "return_ref attribute requires explicit return type", + returns, + "returns attribute requires explicit return type", )); }; } diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs index ba4b33ad3..4970591e1 100644 --- a/components/salsa-macros/src/tracked_struct.rs +++ b/components/salsa-macros/src/tracked_struct.rs @@ -28,7 +28,7 @@ type TrackedArgs = Options; struct TrackedStruct; impl crate::options::AllowedOptions for TrackedStruct { - const RETURN_REF: bool = false; + const RETURNS: bool = false; const SPECIFY: bool = false; diff --git a/examples/calc/ir.rs b/examples/calc/ir.rs index 45b8ed54f..2c386564b 100644 --- a/examples/calc/ir.rs +++ b/examples/calc/ir.rs @@ -5,7 +5,7 @@ use ordered_float::OrderedFloat; // ANCHOR: input #[salsa::input(debug)] pub struct SourceProgram { - #[return_ref] + #[returns(as_ref)] pub text: String, } // ANCHOR_END: input @@ -13,13 +13,13 @@ pub struct SourceProgram { // ANCHOR: interned_ids #[salsa::interned(debug)] pub struct VariableId<'db> { - #[return_ref] + #[returns(as_ref)] pub text: String, } #[salsa::interned(debug)] pub struct FunctionId<'db> { - #[return_ref] + #[returns(as_ref)] pub text: String, } // ANCHOR_END: interned_ids @@ -28,7 +28,7 @@ pub struct FunctionId<'db> { #[salsa::tracked(debug)] pub struct Program<'db> { #[tracked] - #[return_ref] + #[returns(as_ref)] pub statements: Vec>, } // ANCHOR_END: program @@ -93,11 +93,11 @@ pub struct Function<'db> { name_span: Span<'db>, #[tracked] - #[return_ref] + #[returns(as_ref)] pub args: Vec>, #[tracked] - #[return_ref] + #[returns(as_ref)] pub body: Expression<'db>, } // ANCHOR_END: functions diff --git a/examples/lazy-input/main.rs b/examples/lazy-input/main.rs index 7183eb2ac..4b69b96f6 100644 --- a/examples/lazy-input/main.rs +++ b/examples/lazy-input/main.rs @@ -67,7 +67,7 @@ fn main() -> Result<()> { #[salsa::input] struct File { path: PathBuf, - #[return_ref] + #[returns(as_ref)] contents: String, } @@ -158,7 +158,7 @@ impl Diagnostic { #[salsa::tracked] struct ParsedFile<'db> { value: u32, - #[return_ref] + #[returns(as_ref)] links: Vec>, } diff --git a/src/lib.rs b/src/lib.rs index 1d59a3cf8..ccba905e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,7 +71,7 @@ pub mod plumbing { pub use std::option::Option::{self, None, Some}; pub use salsa_macro_rules::{ - macro_if, maybe_backdate, maybe_clone, maybe_cloned_ty, maybe_default, maybe_default_tt, + macro_if, maybe_backdate, return_mode, return_mode_ty, maybe_default, maybe_default_tt, setup_accumulator_impl, setup_input_struct, setup_interned_struct, setup_method_body, setup_tracked_fn, setup_tracked_struct, unexpected_cycle_initial, unexpected_cycle_recovery, diff --git a/tests/accumulate-reuse-workaround.rs b/tests/accumulate-reuse-workaround.rs index d363e118d..b2b3e81cb 100644 --- a/tests/accumulate-reuse-workaround.rs +++ b/tests/accumulate-reuse-workaround.rs @@ -37,7 +37,7 @@ fn compute(db: &dyn LogDatabase, input: List) -> u32 { result } -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(as_ref))] fn accumulated(db: &dyn LogDatabase, input: List) -> Vec { db.push_log(format!("accumulated({input:?})")); compute::accumulated::(db, input) diff --git a/tests/compile-fail/accumulator_incompatibles.rs b/tests/compile-fail/accumulator_incompatibles.rs index a0110a634..90db24567 100644 --- a/tests/compile-fail/accumulator_incompatibles.rs +++ b/tests/compile-fail/accumulator_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::accumulator(return_ref)] +#[salsa::accumulator(returns(as_ref))] struct AccWithRetRef(u32); #[salsa::accumulator(specify)] diff --git a/tests/compile-fail/accumulator_incompatibles.stderr b/tests/compile-fail/accumulator_incompatibles.stderr index 501896b40..d459de6f7 100644 --- a/tests/compile-fail/accumulator_incompatibles.stderr +++ b/tests/compile-fail/accumulator_incompatibles.stderr @@ -1,8 +1,8 @@ -error: `return_ref` option not allowed here +error: `returns` option not allowed here --> tests/compile-fail/accumulator_incompatibles.rs:1:22 | -1 | #[salsa::accumulator(return_ref)] - | ^^^^^^^^^^ +1 | #[salsa::accumulator(returns(as_ref))] + | ^^^^^^^ error: `specify` option not allowed here --> tests/compile-fail/accumulator_incompatibles.rs:4:22 diff --git a/tests/compile-fail/input_struct_incompatibles.rs b/tests/compile-fail/input_struct_incompatibles.rs index 854a4ad5a..6b79c61e7 100644 --- a/tests/compile-fail/input_struct_incompatibles.rs +++ b/tests/compile-fail/input_struct_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::input(return_ref)] +#[salsa::input(returns(as_ref))] struct InputWithRetRef(u32); #[salsa::input(specify)] diff --git a/tests/compile-fail/input_struct_incompatibles.stderr b/tests/compile-fail/input_struct_incompatibles.stderr index 0dbb8aa33..82cdb204a 100644 --- a/tests/compile-fail/input_struct_incompatibles.stderr +++ b/tests/compile-fail/input_struct_incompatibles.stderr @@ -1,8 +1,8 @@ -error: `return_ref` option not allowed here +error: `returns` option not allowed here --> tests/compile-fail/input_struct_incompatibles.rs:1:16 | -1 | #[salsa::input(return_ref)] - | ^^^^^^^^^^ +1 | #[salsa::input(returns(as_ref))] + | ^^^^^^^ error: `specify` option not allowed here --> tests/compile-fail/input_struct_incompatibles.rs:4:16 diff --git a/tests/compile-fail/interned_struct_incompatibles.rs b/tests/compile-fail/interned_struct_incompatibles.rs index 2ab050d6d..38ba79483 100644 --- a/tests/compile-fail/interned_struct_incompatibles.rs +++ b/tests/compile-fail/interned_struct_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::interned(return_ref)] +#[salsa::interned(returns(as_ref))] struct InternedWithRetRef { field: u32, } diff --git a/tests/compile-fail/interned_struct_incompatibles.stderr b/tests/compile-fail/interned_struct_incompatibles.stderr index d60890ede..c2747fcd5 100644 --- a/tests/compile-fail/interned_struct_incompatibles.stderr +++ b/tests/compile-fail/interned_struct_incompatibles.stderr @@ -1,8 +1,8 @@ -error: `return_ref` option not allowed here +error: `returns` option not allowed here --> tests/compile-fail/interned_struct_incompatibles.rs:1:19 | -1 | #[salsa::interned(return_ref)] - | ^^^^^^^^^^ +1 | #[salsa::interned(returns(as_ref))] + | ^^^^^^^ error: `specify` option not allowed here --> tests/compile-fail/interned_struct_incompatibles.rs:6:19 diff --git a/tests/compile-fail/tracked_fn_return_ref.rs b/tests/compile-fail/tracked_fn_return_ref.rs index 6ecd64e93..f741b8a92 100644 --- a/tests/compile-fail/tracked_fn_return_ref.rs +++ b/tests/compile-fail/tracked_fn_return_ref.rs @@ -2,7 +2,7 @@ use salsa::Database as Db; #[salsa::input] struct MyInput { - #[return_ref] + #[returns(as_ref)] text: String, } diff --git a/tests/compile-fail/tracked_impl_incompatibles.rs b/tests/compile-fail/tracked_impl_incompatibles.rs index a46819cba..92698687e 100644 --- a/tests/compile-fail/tracked_impl_incompatibles.rs +++ b/tests/compile-fail/tracked_impl_incompatibles.rs @@ -3,7 +3,7 @@ struct MyTracked<'db> { field: u32, } -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(as_ref))] impl<'db> std::default::Default for MyTracked<'db> { fn default() -> Self {} } diff --git a/tests/compile-fail/tracked_impl_incompatibles.stderr b/tests/compile-fail/tracked_impl_incompatibles.stderr index 59d9d64d2..2d3e76486 100644 --- a/tests/compile-fail/tracked_impl_incompatibles.stderr +++ b/tests/compile-fail/tracked_impl_incompatibles.stderr @@ -1,8 +1,8 @@ error: unexpected token --> tests/compile-fail/tracked_impl_incompatibles.rs:6:18 | -6 | #[salsa::tracked(return_ref)] - | ^^^^^^^^^^ +6 | #[salsa::tracked(returns(as_ref))] + | ^^^^^^^ error: unexpected token --> tests/compile-fail/tracked_impl_incompatibles.rs:11:18 diff --git a/tests/compile-fail/tracked_struct_incompatibles.rs b/tests/compile-fail/tracked_struct_incompatibles.rs index 69c125918..80f054d45 100644 --- a/tests/compile-fail/tracked_struct_incompatibles.rs +++ b/tests/compile-fail/tracked_struct_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(as_ref))] struct TrackedWithRetRef { field: u32, } diff --git a/tests/compile-fail/tracked_struct_incompatibles.stderr b/tests/compile-fail/tracked_struct_incompatibles.stderr index 8450d85ff..7f19d67ff 100644 --- a/tests/compile-fail/tracked_struct_incompatibles.stderr +++ b/tests/compile-fail/tracked_struct_incompatibles.stderr @@ -1,8 +1,8 @@ -error: `return_ref` option not allowed here +error: `returns` option not allowed here --> tests/compile-fail/tracked_struct_incompatibles.rs:1:18 | -1 | #[salsa::tracked(return_ref)] - | ^^^^^^^^^^ +1 | #[salsa::tracked(returns(as_ref))] + | ^^^^^^^ error: `specify` option not allowed here --> tests/compile-fail/tracked_struct_incompatibles.rs:6:18 diff --git a/tests/cycle.rs b/tests/cycle.rs index 55f6b97c8..e70d3cc46 100644 --- a/tests/cycle.rs +++ b/tests/cycle.rs @@ -32,7 +32,7 @@ impl Value { /// `max_iterate`, `min_panic`, `max_panic`) for testing cycle behaviors. #[salsa::input] struct Inputs { - #[return_ref] + #[returns(as_ref)] inputs: Vec, } diff --git a/tests/cycle_maybe_changed_after.rs b/tests/cycle_maybe_changed_after.rs index cfc271bbc..a739ced70 100644 --- a/tests/cycle_maybe_changed_after.rs +++ b/tests/cycle_maybe_changed_after.rs @@ -12,7 +12,7 @@ struct Input { #[salsa::interned(debug)] struct Output<'db> { - #[return_ref] + #[returns(as_ref)] value: u32, } @@ -170,7 +170,7 @@ fn nested_cycle_fewer_dependencies_in_first_iteration() { } } - #[salsa::tracked(return_ref)] + #[salsa::tracked(returns(as_ref))] fn index<'db>(db: &'db dyn salsa::Database, input: Input) -> Index<'db> { Index { scope: Scope::new(db, input.value(db) * 2), diff --git a/tests/cycle_tracked.rs b/tests/cycle_tracked.rs index fdf42cad4..8d7193ec8 100644 --- a/tests/cycle_tracked.rs +++ b/tests/cycle_tracked.rs @@ -30,10 +30,10 @@ struct Edge { #[salsa::tracked(debug)] struct Node<'db> { - #[return_ref] + #[returns(as_ref)] name: String, - #[return_ref] + #[returns(as_ref)] #[tracked] edges: Vec, @@ -45,7 +45,7 @@ struct GraphInput { simple: bool, } -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(as_ref))] fn create_graph(db: &dyn salsa::Database, input: GraphInput) -> Graph<'_> { if input.simple(db) { let a = Node::new(db, "a".to_string(), vec![], input); diff --git a/tests/deletion-drops.rs b/tests/deletion-drops.rs index 2410aa346..a2ddc73c3 100644 --- a/tests/deletion-drops.rs +++ b/tests/deletion-drops.rs @@ -17,7 +17,7 @@ struct MyTracked<'db> { identifier: u32, #[tracked] - #[return_ref] + #[returns(as_ref)] field: Bomb, } diff --git a/tests/override_new_get_set.rs b/tests/override_new_get_set.rs index 9f3a87528..4454751d3 100644 --- a/tests/override_new_get_set.rs +++ b/tests/override_new_get_set.rs @@ -34,7 +34,7 @@ impl MyInput { #[salsa::interned(constructor = from_string)] struct MyInterned<'db> { #[get(text)] - #[return_ref] + #[returns(as_ref)] field: String, } diff --git a/tests/tracked_fn_return_ref.rs b/tests/tracked_fn_return_ref.rs index 1ff111f51..dd7fc1ce3 100644 --- a/tests/tracked_fn_return_ref.rs +++ b/tests/tracked_fn_return_ref.rs @@ -5,7 +5,7 @@ struct Input { number: usize, } -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(as_ref))] fn test(db: &dyn salsa::Database, input: Input) -> Vec { (0..input.number(db)).map(|i| format!("test {i}")).collect() } diff --git a/tests/tracked_method.rs b/tests/tracked_method.rs index b31e54d2a..932736c01 100644 --- a/tests/tracked_method.rs +++ b/tests/tracked_method.rs @@ -23,7 +23,7 @@ impl MyInput { self.field(db) * 2 } - #[salsa::tracked(return_ref)] + #[salsa::tracked(returns(as_ref))] fn tracked_fn_ref(self, db: &dyn salsa::Database) -> u32 { self.field(db) * 3 } diff --git a/tests/tracked_method_inherent_return_ref.rs b/tests/tracked_method_inherent_return_ref.rs index 92ce22392..cb1da06b0 100644 --- a/tests/tracked_method_inherent_return_ref.rs +++ b/tests/tracked_method_inherent_return_ref.rs @@ -7,7 +7,7 @@ struct Input { #[salsa::tracked] impl Input { - #[salsa::tracked(return_ref)] + #[salsa::tracked(returns(as_ref))] fn test(self, db: &dyn salsa::Database) -> Vec { (0..self.number(db)).map(|i| format!("test {i}")).collect() } diff --git a/tests/tracked_method_on_tracked_struct.rs b/tests/tracked_method_on_tracked_struct.rs index 1febcfd36..96212efd4 100644 --- a/tests/tracked_method_on_tracked_struct.rs +++ b/tests/tracked_method_on_tracked_struct.rs @@ -23,7 +23,7 @@ pub struct SourceTree<'db> { #[salsa::tracked] impl<'db1> SourceTree<'db1> { - #[salsa::tracked(return_ref)] + #[salsa::tracked(returns(as_ref))] pub fn inherent_item_name(self, db: &'db1 dyn Database) -> String { self.name(db) } @@ -35,7 +35,7 @@ trait ItemName<'db1> { #[salsa::tracked] impl<'db1> ItemName<'db1> for SourceTree<'db1> { - #[salsa::tracked(return_ref)] + #[salsa::tracked(returns(as_ref))] fn trait_item_name(self, db: &'db1 dyn Database) -> String { self.name(db) } diff --git a/tests/tracked_method_trait_return_ref.rs b/tests/tracked_method_trait_return_ref.rs index 950e682c7..8cd29f19d 100644 --- a/tests/tracked_method_trait_return_ref.rs +++ b/tests/tracked_method_trait_return_ref.rs @@ -11,7 +11,7 @@ trait Trait { #[salsa::tracked] impl Trait for Input { - #[salsa::tracked(return_ref)] + #[salsa::tracked(returns(as_ref))] fn test(self, db: &dyn salsa::Database) -> Vec { (0..self.number(db)).map(|i| format!("test {i}")).collect() } diff --git a/tests/warnings/needless_borrow.rs b/tests/warnings/needless_borrow.rs index f27a4b6b7..982ef939f 100644 --- a/tests/warnings/needless_borrow.rs +++ b/tests/warnings/needless_borrow.rs @@ -3,6 +3,6 @@ enum Token {} #[salsa::tracked] struct TokenTree<'db> { - #[return_ref] + #[returns(as_ref)] tokens: Vec, } diff --git a/tests/warnings/needless_lifetimes.rs b/tests/warnings/needless_lifetimes.rs index 0eb9198d0..b1c912819 100644 --- a/tests/warnings/needless_lifetimes.rs +++ b/tests/warnings/needless_lifetimes.rs @@ -9,13 +9,13 @@ pub struct SourceTree<'db> {} #[salsa::tracked] impl<'db> SourceTree<'db> { - #[salsa::tracked(return_ref)] + #[salsa::tracked(returns(as_ref))] pub fn all_items(self, _db: &'db dyn Db) -> Vec { todo!() } } -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(as_ref))] fn use_tree<'db>(_db: &'db dyn Db, _tree: SourceTree<'db>) {} #[allow(unused)] From 6570a1ecd2ec5073dffe296c6c6d944c19c6feef Mon Sep 17 00:00:00 2001 From: CheaterCodes Date: Sat, 29 Mar 2025 13:06:31 +0100 Subject: [PATCH 2/5] Implement --- benches/compare.rs | 4 +- benches/incremental.rs | 2 +- book/src/overview.md | 14 ++-- book/src/reference/algorithm.md | 2 +- book/src/tutorial/ir.md | 2 +- book/src/tutorial/parser.md | 8 +- .../salsa-macro-rules/src/return_mode.rs | 76 +++++++++++++++++-- .../salsa-macro-rules/src/setup_tracked_fn.rs | 2 +- .../src/setup_tracked_struct.rs | 4 +- components/salsa-macros/src/options.rs | 7 +- components/salsa-macros/src/salsa_struct.rs | 6 +- components/salsa-macros/src/tracked_fn.rs | 2 +- examples/calc/ir.rs | 12 +-- examples/lazy-input/main.rs | 4 +- src/lib.rs | 3 + src/traits.rs | 47 ++++++++++++ tests/accumulate-reuse-workaround.rs | 2 +- .../compile-fail/accumulator_incompatibles.rs | 2 +- .../accumulator_incompatibles.stderr | 2 +- .../input_struct_incompatibles.rs | 2 +- .../input_struct_incompatibles.stderr | 2 +- .../interned_struct_incompatibles.rs | 2 +- .../interned_struct_incompatibles.stderr | 2 +- tests/compile-fail/tracked_fn_return_ref.rs | 2 +- .../tracked_impl_incompatibles.rs | 2 +- .../tracked_impl_incompatibles.stderr | 2 +- .../tracked_struct_incompatibles.rs | 2 +- .../tracked_struct_incompatibles.stderr | 2 +- tests/cycle.rs | 2 +- tests/cycle_maybe_changed_after.rs | 3 +- tests/cycle_tracked.rs | 6 +- tests/deletion-drops.rs | 2 +- tests/override_new_get_set.rs | 2 +- tests/tracked_fn_return_ref.rs | 2 +- tests/tracked_method.rs | 2 +- tests/tracked_method_inherent_return_ref.rs | 2 +- tests/tracked_method_on_tracked_struct.rs | 4 +- tests/tracked_method_trait_return_ref.rs | 2 +- tests/warnings/needless_borrow.rs | 2 +- tests/warnings/needless_lifetimes.rs | 4 +- 40 files changed, 183 insertions(+), 69 deletions(-) create mode 100644 src/traits.rs diff --git a/benches/compare.rs b/benches/compare.rs index 26ee36866..c4e6b36f8 100644 --- a/benches/compare.rs +++ b/benches/compare.rs @@ -10,7 +10,7 @@ include!("shims/global_alloc_overwrite.rs"); #[salsa::input] pub struct Input { - #[returns(as_ref)] + #[returns(ref)] pub text: String, } @@ -22,7 +22,7 @@ pub fn length(db: &dyn salsa::Database, input: Input) -> usize { #[salsa::interned] pub struct InternedInput<'db> { - #[returns(as_ref)] + #[returns(ref)] pub text: String, } diff --git a/benches/incremental.rs b/benches/incremental.rs index 432c958b1..872d9fa1a 100644 --- a/benches/incremental.rs +++ b/benches/incremental.rs @@ -15,7 +15,7 @@ struct Tracked<'db> { number: usize, } -#[salsa::tracked(returns(as_ref))] +#[salsa::tracked(returns(ref))] #[inline(never)] fn index<'db>(db: &'db dyn salsa::Database, input: Input) -> Vec> { (0..input.field(db)).map(|i| Tracked::new(db, i)).collect() diff --git a/book/src/overview.md b/book/src/overview.md index 3c0c00f54..df468d2fa 100644 --- a/book/src/overview.md +++ b/book/src/overview.md @@ -79,7 +79,7 @@ pub struct ProgramFile(salsa::Id); This means that, when you have a `ProgramFile`, you can easily copy it around and put it wherever you like. To actually read any of its fields, however, you will need to use the database and a getter method. -### Reading fields and `return_ref` +### Reading fields and `returns(ref)` You can access the value of an input's fields by using the getter method. As this is only reading the field, it just needs a `&`-reference to the database: @@ -89,13 +89,13 @@ let contents: String = file.contents(&db); ``` Invoking the accessor clones the value from the database. -Sometimes this is not what you want, so you can annotate fields with `#[returns(as_ref)]` to indicate that they should return a reference into the database instead: +Sometimes this is not what you want, so you can annotate fields with `#[returns(ref)]` to indicate that they should return a reference into the database instead: ```rust #[salsa::input] pub struct ProgramFile { pub path: PathBuf, - #[returns(as_ref)] + #[returns(ref)] pub contents: String, } ``` @@ -145,7 +145,7 @@ Tracked functions have to follow a particular structure: - They must take a "Salsa struct" as the second argument -- in our example, this is an input struct, but there are other kinds of Salsa structs we'll describe shortly. - They _can_ take additional arguments, but it's faster and better if they don't. -Tracked functions can return any clone-able type. A clone is required since, when the value is cached, the result will be cloned out of the database. Tracked functions can also be annotated with `#[returns(as_ref)]` if you would prefer to return a reference into the database instead (if `parse_file` were so annotated, then callers would actually get back an `&Ast`, for example). +Tracked functions can return any clone-able type. A clone is required since, when the value is cached, the result will be cloned out of the database. Tracked functions can also be annotated with `#[returns(ref)]` if you would prefer to return a reference into the database instead (if `parse_file` were so annotated, then callers would actually get back an `&Ast`, for example). ## Tracked structs @@ -158,7 +158,7 @@ Example: ```rust #[salsa::tracked] struct Ast<'db> { - #[returns(as_ref)] + #[returns(ref)] top_level_items: Vec, } ``` @@ -252,7 +252,7 @@ Most compilers, for example, will define a type to represent a user identifier: ```rust #[salsa::interned] struct Word { - #[returns(as_ref)] + #[returns(ref)] pub text: String, } ``` @@ -269,7 +269,7 @@ let w3 = Word::new(db, "foo".to_string()); When you create two interned structs with the same field values, you are guaranteed to get back the same integer id. So here, we know that `assert_eq!(w1, w3)` is true and `assert_ne!(w1, w2)`. -You can access the fields of an interned struct using a getter, like `word.text(db)`. These getters respect the `#[returns(as_ref)]` annotation. Like tracked structs, the fields of interned structs are immutable. +You can access the fields of an interned struct using a getter, like `word.text(db)`. These getters respect the `#[returns(ref)]` annotation. Like tracked structs, the fields of interned structs are immutable. ## Accumulators diff --git a/book/src/reference/algorithm.md b/book/src/reference/algorithm.md index 003437685..3dc190974 100644 --- a/book/src/reference/algorithm.md +++ b/book/src/reference/algorithm.md @@ -20,7 +20,7 @@ fn parse_module(db: &dyn Db, module: Module) -> Ast { Ast::parse_text(module_text) } -#[salsa::tracked(return_ref)] +#[salsa::tracked(returns(ref))] fn module_text(db: &dyn Db, module: Module) -> String { panic!("text for module `{module:?}` not set") } diff --git a/book/src/tutorial/ir.md b/book/src/tutorial/ir.md index 56c8ab6df..6b6c8b671 100644 --- a/book/src/tutorial/ir.md +++ b/book/src/tutorial/ir.md @@ -92,7 +92,7 @@ Apart from the fields being immutable, the API for working with a tracked struct - You can create a new value by using `new`: e.g., `Program::new(&db, some_statements)` - You use a getter to read the value of a field, just like with an input (e.g., `my_func.statements(db)` to read the `statements` field). - - In this case, the field is tagged as `#[returns(as_ref)]`, which means that the getter will return a `&Vec`, instead of cloning the vector. + - In this case, the field is tagged as `#[returns(ref)]`, which means that the getter will return a `&Vec`, instead of cloning the vector. ### The `'db` lifetime diff --git a/book/src/tutorial/parser.md b/book/src/tutorial/parser.md index 43e86f51d..271d77a19 100644 --- a/book/src/tutorial/parser.md +++ b/book/src/tutorial/parser.md @@ -61,12 +61,12 @@ Tracked functions may take other arguments as well, though our examples here do Functions that take additional arguments are less efficient and flexible. It's generally better to structure tracked functions as functions of a single Salsa struct if possible. -### The `return_ref` annotation +### The `returns(ref)` annotation -You may have noticed that `parse_statements` is tagged with `#[salsa::tracked(return_ref)]`. +You may have noticed that `parse_statements` is tagged with `#[salsa::tracked(returns(ref))]`. Ordinarily, when you call a tracked function, the result you get back is cloned out of the database. -The `return_ref` attribute means that a reference into the database is returned instead. +The `returns(ref)` attribute means that a reference into the database is returned instead. So, when called, `parse_statements` will return an `&Vec` rather than cloning the `Vec`. This is useful as a performance optimization. -(You may recall the `return_ref` annotation from the [ir](./ir.md) section of the tutorial, +(You may recall the `returns(ref)` annotation from the [ir](./ir.md) section of the tutorial, where it was placed on struct fields, with roughly the same meaning.) diff --git a/components/salsa-macro-rules/src/return_mode.rs b/components/salsa-macro-rules/src/return_mode.rs index 9fdfa1959..0d39fd57d 100644 --- a/components/salsa-macro-rules/src/return_mode.rs +++ b/components/salsa-macro-rules/src/return_mode.rs @@ -4,7 +4,23 @@ #[macro_export] macro_rules! return_mode { ( - (as_ref, $maybe_backdate:ident, $maybe_default:ident), + (copy, $maybe_backdate:ident, $maybe_default:ident), + $field_ty:ty, + $field_ref_expr:expr, + ) => { + *$field_ref_expr + }; + + ( + (clone, $maybe_backdate:ident, $maybe_default:ident), + $field_ty:ty, + $field_ref_expr:expr, + ) => { + ::core::clone::Clone::clone($field_ref_expr) + }; + + ( + (ref, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, $field_ref_expr:expr, ) => { @@ -12,29 +28,77 @@ macro_rules! return_mode { }; ( - (cloned, $maybe_backdate:ident, $maybe_default:ident), + (deref, $maybe_backdate:ident, $maybe_default:ident), + $field_ty:ty, + $field_ref_expr:expr, + ) => { + ::core::ops::Deref::deref($field_ref_expr) + }; + + ( + (as_ref, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, $field_ref_expr:expr, ) => { - std::clone::Clone::clone($field_ref_expr) + ::salsa::SalsaAsRef::as_ref($field_ref_expr) + }; + + ( + (as_deref, $maybe_backdate:ident, $maybe_default:ident), + $field_ty:ty, + $field_ref_expr:expr, + ) => { + ::salsa::SalsaAsDeref::as_deref($field_ref_expr) }; } #[macro_export] macro_rules! return_mode_ty { ( - (as_ref, $maybe_backdate:ident, $maybe_default:ident), + (copy, $maybe_backdate:ident, $maybe_default:ident), $db_lt:lifetime, $field_ty:ty ) => { - & $db_lt $field_ty + $field_ty }; ( - (cloned, $maybe_backdate:ident, $maybe_default:ident), + (clone, $maybe_backdate:ident, $maybe_default:ident), $db_lt:lifetime, $field_ty:ty ) => { $field_ty }; + + ( + (ref, $maybe_backdate:ident, $maybe_default:ident), + $db_lt:lifetime, + $field_ty:ty + ) => { + & $db_lt $field_ty + }; + + ( + (deref, $maybe_backdate:ident, $maybe_default:ident), + $db_lt:lifetime, + $field_ty:ty + ) => { + & $db_lt <$field_ty as ::core::ops::Deref>::Target + }; + + ( + (as_ref, $maybe_backdate:ident, $maybe_default:ident), + $db_lt:lifetime, + $field_ty:ty + ) => { + <$field_ty as ::salsa::SalsaAsRef>::AsRef<$db_lt> + }; + + ( + (as_deref, $maybe_backdate:ident, $maybe_default:ident), + $db_lt:lifetime, + $field_ty:ty + ) => { + <$field_ty as ::salsa::SalsaAsDeref>::AsDeref<$db_lt> + }; } diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index 6d920032b..6d4e2f50c 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/components/salsa-macro-rules/src/setup_tracked_fn.rs @@ -55,7 +55,7 @@ macro_rules! setup_tracked_fn { // LRU capacity (a literal, maybe 0) lru: $lru:tt, - // The return mode for the function, either `as_ref` or `cloned` + // The return mode for the function, see `salsa_macros::options::Option::returns` return_mode: $return_mode:tt, assert_return_type_is_update: {$($assert_return_type_is_update:tt)*}, diff --git a/components/salsa-macro-rules/src/setup_tracked_struct.rs b/components/salsa-macro-rules/src/setup_tracked_struct.rs index eafe46801..41da9c5be 100644 --- a/components/salsa-macro-rules/src/setup_tracked_struct.rs +++ b/components/salsa-macro-rules/src/setup_tracked_struct.rs @@ -54,7 +54,7 @@ macro_rules! setup_tracked_struct { // // Each field option is a tuple `(return_mode, maybe_backdate)` where: // - // * `return_mode` is either the identifier `as_ref` or `cloned` + // * `return_mode` is an indentiefier as specified in `salsa_macros::options::Option::returns` // * `maybe_backdate` is either the identifier `backdate` or `no_backdate` // // These are used to drive conditional logic for each field via recursive macro invocation @@ -65,7 +65,7 @@ macro_rules! setup_tracked_struct { // // Each field option is a tuple `(return_mode, maybe_backdate)` where: // - // * `return_mode` is either the identifier `as_ref` or `cloned` + // * `return_mode` is an indentiefier as specified in `salsa_macros::options::Option::returns` // * `maybe_backdate` is either the identifier `backdate` or `no_backdate` // // These are used to drive conditional logic for each field via recursive macro invocation diff --git a/components/salsa-macros/src/options.rs b/components/salsa-macros/src/options.rs index 56a15842f..b4d9e08c0 100644 --- a/components/salsa-macros/src/options.rs +++ b/components/salsa-macros/src/options.rs @@ -11,9 +11,10 @@ use syn::spanned::Spanned; /// trait. #[derive(Debug)] pub(crate) struct Options { - /// The `returns` option is used to signal whether the field/return type is cloned or returned as a reference + /// The `returns` option is used to configure the "return mode" for the field/function. + /// This may be one of `copy`, `clone`, `ref`, `as_ref`, `as_deref`. /// - /// If this is `Some`, the value is the return mode (`as_ref`, `cloned`). + /// If this is `Some`, the value is the ident representing the selected mode. pub returns: Option, /// The `no_eq` option is used to signal that a given field does not implement @@ -148,7 +149,7 @@ impl syn::parse::Parse for Options { if ident == "returns" { let content; parenthesized!(content in input); - let mode = content.parse()?; + let mode = syn::Ident::parse_any(&content)?; if A::RETURNS { if let Some(old) = options.returns.replace(mode) { return Err(syn::Error::new( diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs index 12171d8b8..8cbdb7df4 100644 --- a/components/salsa-macros/src/salsa_struct.rs +++ b/components/salsa-macros/src/salsa_struct.rs @@ -26,7 +26,7 @@ //! * this could be optimized, particularly for interned fields use proc_macro2::{Ident, Literal, Span, TokenStream}; -use syn::spanned::Spanned; +use syn::{ext::IdentExt, spanned::Spanned}; use crate::db_lifetime; use crate::options::{AllowedOptions, Options}; @@ -72,7 +72,7 @@ pub(crate) const FIELD_OPTION_ATTRIBUTES: &[(&str, fn(&syn::Attribute, &mut Sals ("tracked", |_, ef| ef.has_tracked_attr = true), ("default", |_, ef| ef.has_default_attr = true), ("returns", |attr, ef| { - ef.returns = attr.parse_args().unwrap(); + ef.returns = attr.parse_args_with(syn::Ident::parse_any).unwrap(); }), ("no_eq", |_, ef| ef.has_no_eq_attr = true), ("get", |attr, ef| { @@ -367,7 +367,7 @@ impl<'s> SalsaField<'s> { let get_name = Ident::new(&field_name_str, field_name.span()); let set_name = Ident::new(&format!("set_{field_name_str}",), field_name.span()); - let returns = Ident::new("cloned", field.span()); + let returns = Ident::new("clone", field.span()); let mut result = SalsaField { field, has_tracked_attr: false, diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index 58686144c..0fba8b142 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/components/salsa-macros/src/tracked_fn.rs @@ -150,7 +150,7 @@ impl Macro { .args .returns .clone() - .unwrap_or(Ident::new("cloned", Span::call_site())); + .unwrap_or(Ident::new("clone", Span::call_site())); // 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 diff --git a/examples/calc/ir.rs b/examples/calc/ir.rs index 2c386564b..7909b2642 100644 --- a/examples/calc/ir.rs +++ b/examples/calc/ir.rs @@ -5,7 +5,7 @@ use ordered_float::OrderedFloat; // ANCHOR: input #[salsa::input(debug)] pub struct SourceProgram { - #[returns(as_ref)] + #[returns(ref)] pub text: String, } // ANCHOR_END: input @@ -13,13 +13,13 @@ pub struct SourceProgram { // ANCHOR: interned_ids #[salsa::interned(debug)] pub struct VariableId<'db> { - #[returns(as_ref)] + #[returns(ref)] pub text: String, } #[salsa::interned(debug)] pub struct FunctionId<'db> { - #[returns(as_ref)] + #[returns(ref)] pub text: String, } // ANCHOR_END: interned_ids @@ -28,7 +28,7 @@ pub struct FunctionId<'db> { #[salsa::tracked(debug)] pub struct Program<'db> { #[tracked] - #[returns(as_ref)] + #[returns(ref)] pub statements: Vec>, } // ANCHOR_END: program @@ -93,11 +93,11 @@ pub struct Function<'db> { name_span: Span<'db>, #[tracked] - #[returns(as_ref)] + #[returns(ref)] pub args: Vec>, #[tracked] - #[returns(as_ref)] + #[returns(ref)] pub body: Expression<'db>, } // ANCHOR_END: functions diff --git a/examples/lazy-input/main.rs b/examples/lazy-input/main.rs index 4b69b96f6..233d933de 100644 --- a/examples/lazy-input/main.rs +++ b/examples/lazy-input/main.rs @@ -67,7 +67,7 @@ fn main() -> Result<()> { #[salsa::input] struct File { path: PathBuf, - #[returns(as_ref)] + #[returns(ref)] contents: String, } @@ -158,7 +158,7 @@ impl Diagnostic { #[salsa::tracked] struct ParsedFile<'db> { value: u32, - #[returns(as_ref)] + #[returns(ref)] links: Vec>, } diff --git a/src/lib.rs b/src/lib.rs index ccba905e1..7e8f422a7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,7 @@ mod salsa_struct; mod storage; mod table; mod tracked_struct; +mod traits; mod update; mod views; mod zalsa; @@ -52,6 +53,8 @@ pub use self::key::DatabaseKeyIndex; pub use self::revision::Revision; pub use self::runtime::Runtime; pub use self::storage::{Storage, StorageHandle}; +pub use self::traits::SalsaAsRef; +pub use self::traits::SalsaAsDeref; pub use self::update::Update; pub use self::zalsa::IngredientIndex; pub use crate::attach::{attach, with_attached_database}; diff --git a/src/traits.rs b/src/traits.rs new file mode 100644 index 000000000..994f15a8f --- /dev/null +++ b/src/traits.rs @@ -0,0 +1,47 @@ +//! User-implementable salsa traits + +use std::ops::Deref; + +pub trait SalsaAsRef { + type AsRef<'a> where Self: 'a; + + fn as_ref(&self) -> Self::AsRef<'_>; +} + +impl SalsaAsRef for Option { + type AsRef<'a> = Option<&'a T> where Self: 'a; + + fn as_ref(&self) -> Self::AsRef<'_> { + self.as_ref() + } +} + +impl SalsaAsRef for Result { + type AsRef<'a> = Result<&'a T, &'a E> where Self: 'a; + + fn as_ref(&self) -> Self::AsRef<'_> { + self.as_ref() + } +} + +pub trait SalsaAsDeref { + type AsDeref<'a> where Self: 'a; + + fn as_ref(&self) -> Self::AsDeref<'_>; +} + +impl SalsaAsDeref for Option { + type AsDeref<'a> = Option<&'a T::Target> where Self: 'a; + + fn as_ref(&self) -> Self::AsDeref<'_> { + self.as_deref() + } +} + +impl SalsaAsDeref for Result { + type AsDeref<'a> = Result<&'a T::Target, &'a E> where Self: 'a; + + fn as_ref(&self) -> Self::AsDeref<'_> { + self.as_deref() + } +} diff --git a/tests/accumulate-reuse-workaround.rs b/tests/accumulate-reuse-workaround.rs index b2b3e81cb..915a14c1c 100644 --- a/tests/accumulate-reuse-workaround.rs +++ b/tests/accumulate-reuse-workaround.rs @@ -37,7 +37,7 @@ fn compute(db: &dyn LogDatabase, input: List) -> u32 { result } -#[salsa::tracked(returns(as_ref))] +#[salsa::tracked(returns(ref))] fn accumulated(db: &dyn LogDatabase, input: List) -> Vec { db.push_log(format!("accumulated({input:?})")); compute::accumulated::(db, input) diff --git a/tests/compile-fail/accumulator_incompatibles.rs b/tests/compile-fail/accumulator_incompatibles.rs index 90db24567..d453ab6f5 100644 --- a/tests/compile-fail/accumulator_incompatibles.rs +++ b/tests/compile-fail/accumulator_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::accumulator(returns(as_ref))] +#[salsa::accumulator(returns(ref))] struct AccWithRetRef(u32); #[salsa::accumulator(specify)] diff --git a/tests/compile-fail/accumulator_incompatibles.stderr b/tests/compile-fail/accumulator_incompatibles.stderr index d459de6f7..8853f4f65 100644 --- a/tests/compile-fail/accumulator_incompatibles.stderr +++ b/tests/compile-fail/accumulator_incompatibles.stderr @@ -1,7 +1,7 @@ error: `returns` option not allowed here --> tests/compile-fail/accumulator_incompatibles.rs:1:22 | -1 | #[salsa::accumulator(returns(as_ref))] +1 | #[salsa::accumulator(returns(ref))] | ^^^^^^^ error: `specify` option not allowed here diff --git a/tests/compile-fail/input_struct_incompatibles.rs b/tests/compile-fail/input_struct_incompatibles.rs index 6b79c61e7..8bc3a7e3a 100644 --- a/tests/compile-fail/input_struct_incompatibles.rs +++ b/tests/compile-fail/input_struct_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::input(returns(as_ref))] +#[salsa::input(returns(ref))] struct InputWithRetRef(u32); #[salsa::input(specify)] diff --git a/tests/compile-fail/input_struct_incompatibles.stderr b/tests/compile-fail/input_struct_incompatibles.stderr index 82cdb204a..8a868c146 100644 --- a/tests/compile-fail/input_struct_incompatibles.stderr +++ b/tests/compile-fail/input_struct_incompatibles.stderr @@ -1,7 +1,7 @@ error: `returns` option not allowed here --> tests/compile-fail/input_struct_incompatibles.rs:1:16 | -1 | #[salsa::input(returns(as_ref))] +1 | #[salsa::input(returns(ref))] | ^^^^^^^ error: `specify` option not allowed here diff --git a/tests/compile-fail/interned_struct_incompatibles.rs b/tests/compile-fail/interned_struct_incompatibles.rs index 38ba79483..65dbb3e0f 100644 --- a/tests/compile-fail/interned_struct_incompatibles.rs +++ b/tests/compile-fail/interned_struct_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::interned(returns(as_ref))] +#[salsa::interned(returns(ref))] struct InternedWithRetRef { field: u32, } diff --git a/tests/compile-fail/interned_struct_incompatibles.stderr b/tests/compile-fail/interned_struct_incompatibles.stderr index c2747fcd5..482e38b46 100644 --- a/tests/compile-fail/interned_struct_incompatibles.stderr +++ b/tests/compile-fail/interned_struct_incompatibles.stderr @@ -1,7 +1,7 @@ error: `returns` option not allowed here --> tests/compile-fail/interned_struct_incompatibles.rs:1:19 | -1 | #[salsa::interned(returns(as_ref))] +1 | #[salsa::interned(returns(ref))] | ^^^^^^^ error: `specify` option not allowed here diff --git a/tests/compile-fail/tracked_fn_return_ref.rs b/tests/compile-fail/tracked_fn_return_ref.rs index f741b8a92..1f4f61392 100644 --- a/tests/compile-fail/tracked_fn_return_ref.rs +++ b/tests/compile-fail/tracked_fn_return_ref.rs @@ -2,7 +2,7 @@ use salsa::Database as Db; #[salsa::input] struct MyInput { - #[returns(as_ref)] + #[returns(ref)] text: String, } diff --git a/tests/compile-fail/tracked_impl_incompatibles.rs b/tests/compile-fail/tracked_impl_incompatibles.rs index 92698687e..d3b66590b 100644 --- a/tests/compile-fail/tracked_impl_incompatibles.rs +++ b/tests/compile-fail/tracked_impl_incompatibles.rs @@ -3,7 +3,7 @@ struct MyTracked<'db> { field: u32, } -#[salsa::tracked(returns(as_ref))] +#[salsa::tracked(returns(ref))] impl<'db> std::default::Default for MyTracked<'db> { fn default() -> Self {} } diff --git a/tests/compile-fail/tracked_impl_incompatibles.stderr b/tests/compile-fail/tracked_impl_incompatibles.stderr index 2d3e76486..fa4227011 100644 --- a/tests/compile-fail/tracked_impl_incompatibles.stderr +++ b/tests/compile-fail/tracked_impl_incompatibles.stderr @@ -1,7 +1,7 @@ error: unexpected token --> tests/compile-fail/tracked_impl_incompatibles.rs:6:18 | -6 | #[salsa::tracked(returns(as_ref))] +6 | #[salsa::tracked(returns(ref))] | ^^^^^^^ error: unexpected token diff --git a/tests/compile-fail/tracked_struct_incompatibles.rs b/tests/compile-fail/tracked_struct_incompatibles.rs index 80f054d45..a128226c0 100644 --- a/tests/compile-fail/tracked_struct_incompatibles.rs +++ b/tests/compile-fail/tracked_struct_incompatibles.rs @@ -1,4 +1,4 @@ -#[salsa::tracked(returns(as_ref))] +#[salsa::tracked(returns(ref))] struct TrackedWithRetRef { field: u32, } diff --git a/tests/compile-fail/tracked_struct_incompatibles.stderr b/tests/compile-fail/tracked_struct_incompatibles.stderr index 7f19d67ff..c0fb9c30b 100644 --- a/tests/compile-fail/tracked_struct_incompatibles.stderr +++ b/tests/compile-fail/tracked_struct_incompatibles.stderr @@ -1,7 +1,7 @@ error: `returns` option not allowed here --> tests/compile-fail/tracked_struct_incompatibles.rs:1:18 | -1 | #[salsa::tracked(returns(as_ref))] +1 | #[salsa::tracked(returns(ref))] | ^^^^^^^ error: `specify` option not allowed here diff --git a/tests/cycle.rs b/tests/cycle.rs index e70d3cc46..cb1faba11 100644 --- a/tests/cycle.rs +++ b/tests/cycle.rs @@ -32,7 +32,7 @@ impl Value { /// `max_iterate`, `min_panic`, `max_panic`) for testing cycle behaviors. #[salsa::input] struct Inputs { - #[returns(as_ref)] + #[returns(ref)] inputs: Vec, } diff --git a/tests/cycle_maybe_changed_after.rs b/tests/cycle_maybe_changed_after.rs index a739ced70..2759c65ff 100644 --- a/tests/cycle_maybe_changed_after.rs +++ b/tests/cycle_maybe_changed_after.rs @@ -12,7 +12,6 @@ struct Input { #[salsa::interned(debug)] struct Output<'db> { - #[returns(as_ref)] value: u32, } @@ -170,7 +169,7 @@ fn nested_cycle_fewer_dependencies_in_first_iteration() { } } - #[salsa::tracked(returns(as_ref))] + #[salsa::tracked(returns(ref))] fn index<'db>(db: &'db dyn salsa::Database, input: Input) -> Index<'db> { Index { scope: Scope::new(db, input.value(db) * 2), diff --git a/tests/cycle_tracked.rs b/tests/cycle_tracked.rs index 8d7193ec8..7a653f81e 100644 --- a/tests/cycle_tracked.rs +++ b/tests/cycle_tracked.rs @@ -30,10 +30,10 @@ struct Edge { #[salsa::tracked(debug)] struct Node<'db> { - #[returns(as_ref)] + #[returns(ref)] name: String, - #[returns(as_ref)] + #[returns(deref)] #[tracked] edges: Vec, @@ -45,7 +45,7 @@ struct GraphInput { simple: bool, } -#[salsa::tracked(returns(as_ref))] +#[salsa::tracked(returns(ref))] fn create_graph(db: &dyn salsa::Database, input: GraphInput) -> Graph<'_> { if input.simple(db) { let a = Node::new(db, "a".to_string(), vec![], input); diff --git a/tests/deletion-drops.rs b/tests/deletion-drops.rs index a2ddc73c3..52b0b5120 100644 --- a/tests/deletion-drops.rs +++ b/tests/deletion-drops.rs @@ -17,7 +17,7 @@ struct MyTracked<'db> { identifier: u32, #[tracked] - #[returns(as_ref)] + #[returns(ref)] field: Bomb, } diff --git a/tests/override_new_get_set.rs b/tests/override_new_get_set.rs index 4454751d3..367decf04 100644 --- a/tests/override_new_get_set.rs +++ b/tests/override_new_get_set.rs @@ -34,7 +34,7 @@ impl MyInput { #[salsa::interned(constructor = from_string)] struct MyInterned<'db> { #[get(text)] - #[returns(as_ref)] + #[returns(ref)] field: String, } diff --git a/tests/tracked_fn_return_ref.rs b/tests/tracked_fn_return_ref.rs index dd7fc1ce3..918f33b37 100644 --- a/tests/tracked_fn_return_ref.rs +++ b/tests/tracked_fn_return_ref.rs @@ -5,7 +5,7 @@ struct Input { number: usize, } -#[salsa::tracked(returns(as_ref))] +#[salsa::tracked(returns(ref))] fn test(db: &dyn salsa::Database, input: Input) -> Vec { (0..input.number(db)).map(|i| format!("test {i}")).collect() } diff --git a/tests/tracked_method.rs b/tests/tracked_method.rs index 932736c01..4fc283a64 100644 --- a/tests/tracked_method.rs +++ b/tests/tracked_method.rs @@ -23,7 +23,7 @@ impl MyInput { self.field(db) * 2 } - #[salsa::tracked(returns(as_ref))] + #[salsa::tracked(returns(ref))] fn tracked_fn_ref(self, db: &dyn salsa::Database) -> u32 { self.field(db) * 3 } diff --git a/tests/tracked_method_inherent_return_ref.rs b/tests/tracked_method_inherent_return_ref.rs index cb1da06b0..564bb31ff 100644 --- a/tests/tracked_method_inherent_return_ref.rs +++ b/tests/tracked_method_inherent_return_ref.rs @@ -7,7 +7,7 @@ struct Input { #[salsa::tracked] impl Input { - #[salsa::tracked(returns(as_ref))] + #[salsa::tracked(returns(ref))] fn test(self, db: &dyn salsa::Database) -> Vec { (0..self.number(db)).map(|i| format!("test {i}")).collect() } diff --git a/tests/tracked_method_on_tracked_struct.rs b/tests/tracked_method_on_tracked_struct.rs index 96212efd4..f7cb9f6da 100644 --- a/tests/tracked_method_on_tracked_struct.rs +++ b/tests/tracked_method_on_tracked_struct.rs @@ -23,7 +23,7 @@ pub struct SourceTree<'db> { #[salsa::tracked] impl<'db1> SourceTree<'db1> { - #[salsa::tracked(returns(as_ref))] + #[salsa::tracked(returns(ref))] pub fn inherent_item_name(self, db: &'db1 dyn Database) -> String { self.name(db) } @@ -35,7 +35,7 @@ trait ItemName<'db1> { #[salsa::tracked] impl<'db1> ItemName<'db1> for SourceTree<'db1> { - #[salsa::tracked(returns(as_ref))] + #[salsa::tracked(returns(ref))] fn trait_item_name(self, db: &'db1 dyn Database) -> String { self.name(db) } diff --git a/tests/tracked_method_trait_return_ref.rs b/tests/tracked_method_trait_return_ref.rs index 8cd29f19d..ec74cf3ae 100644 --- a/tests/tracked_method_trait_return_ref.rs +++ b/tests/tracked_method_trait_return_ref.rs @@ -11,7 +11,7 @@ trait Trait { #[salsa::tracked] impl Trait for Input { - #[salsa::tracked(returns(as_ref))] + #[salsa::tracked(returns(ref))] fn test(self, db: &dyn salsa::Database) -> Vec { (0..self.number(db)).map(|i| format!("test {i}")).collect() } diff --git a/tests/warnings/needless_borrow.rs b/tests/warnings/needless_borrow.rs index 982ef939f..605f4c778 100644 --- a/tests/warnings/needless_borrow.rs +++ b/tests/warnings/needless_borrow.rs @@ -3,6 +3,6 @@ enum Token {} #[salsa::tracked] struct TokenTree<'db> { - #[returns(as_ref)] + #[returns(ref)] tokens: Vec, } diff --git a/tests/warnings/needless_lifetimes.rs b/tests/warnings/needless_lifetimes.rs index b1c912819..8976464a1 100644 --- a/tests/warnings/needless_lifetimes.rs +++ b/tests/warnings/needless_lifetimes.rs @@ -9,13 +9,13 @@ pub struct SourceTree<'db> {} #[salsa::tracked] impl<'db> SourceTree<'db> { - #[salsa::tracked(returns(as_ref))] + #[salsa::tracked(returns(ref))] pub fn all_items(self, _db: &'db dyn Db) -> Vec { todo!() } } -#[salsa::tracked(returns(as_ref))] +#[salsa::tracked(returns(ref))] fn use_tree<'db>(_db: &'db dyn Db, _tree: SourceTree<'db>) {} #[allow(unused)] From 0cc7db7513f01671d0a1b297ac0d0aafaa9a1f6b Mon Sep 17 00:00:00 2001 From: CheaterCodes Date: Wed, 7 May 2025 20:08:43 +0200 Subject: [PATCH 3/5] renamed module for return_mode --- src/lib.rs | 6 +++--- src/{traits.rs => return_mode.rs} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename src/{traits.rs => return_mode.rs} (100%) diff --git a/src/lib.rs b/src/lib.rs index 7e8f422a7..68a861feb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,7 +28,7 @@ mod salsa_struct; mod storage; mod table; mod tracked_struct; -mod traits; +mod return_mode; mod update; mod views; mod zalsa; @@ -53,8 +53,8 @@ pub use self::key::DatabaseKeyIndex; pub use self::revision::Revision; pub use self::runtime::Runtime; pub use self::storage::{Storage, StorageHandle}; -pub use self::traits::SalsaAsRef; -pub use self::traits::SalsaAsDeref; +pub use self::return_mode::SalsaAsRef; +pub use self::return_mode::SalsaAsDeref; pub use self::update::Update; pub use self::zalsa::IngredientIndex; pub use crate::attach::{attach, with_attached_database}; diff --git a/src/traits.rs b/src/return_mode.rs similarity index 100% rename from src/traits.rs rename to src/return_mode.rs From e7d480523041a0806c2fec513f75073551c97e94 Mon Sep 17 00:00:00 2001 From: CheaterCodes Date: Wed, 7 May 2025 21:42:43 +0200 Subject: [PATCH 4/5] Rename macro, fix docs, add tests, validate return modes --- .../salsa-macro-rules/src/return_mode.rs | 4 +- .../src/setup_input_struct.rs | 2 +- .../src/setup_interned_struct.rs | 2 +- .../salsa-macro-rules/src/setup_tracked_fn.rs | 2 +- .../src/setup_tracked_struct.rs | 8 +- components/salsa-macros/src/salsa_struct.rs | 6 + components/salsa-macros/src/tracked_fn.rs | 7 + src/lib.rs | 12 +- src/return_mode.rs | 14 +- tests/compile-fail/invalid_return_mode.rs | 20 ++ tests/compile-fail/invalid_return_mode.stderr | 17 ++ tests/return_mode.rs | 172 ++++++++++++++++++ 12 files changed, 247 insertions(+), 19 deletions(-) create mode 100644 tests/compile-fail/invalid_return_mode.rs create mode 100644 tests/compile-fail/invalid_return_mode.stderr create mode 100644 tests/return_mode.rs diff --git a/components/salsa-macro-rules/src/return_mode.rs b/components/salsa-macro-rules/src/return_mode.rs index 0d39fd57d..e5a3338a3 100644 --- a/components/salsa-macro-rules/src/return_mode.rs +++ b/components/salsa-macro-rules/src/return_mode.rs @@ -1,8 +1,8 @@ -/// Generate either `field_ref_expr` or a clone of that expr. +/// Generate the expression for the return type, depending on the return mode defined in [`salsa-macros::options::Options::returns`] /// /// Used when generating field getters. #[macro_export] -macro_rules! return_mode { +macro_rules! return_mode_expression { ( (copy, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, diff --git a/components/salsa-macro-rules/src/setup_input_struct.rs b/components/salsa-macro-rules/src/setup_input_struct.rs index e44c8c9d8..02ee01c09 100644 --- a/components/salsa-macro-rules/src/setup_input_struct.rs +++ b/components/salsa-macro-rules/src/setup_input_struct.rs @@ -192,7 +192,7 @@ macro_rules! setup_input_struct { self, $field_index, ); - $zalsa::return_mode!( + $zalsa::return_mode_expression!( $field_option, $field_ty, &fields.$field_index, diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs index 07cf35cf6..e000e8560 100644 --- a/components/salsa-macro-rules/src/setup_interned_struct.rs +++ b/components/salsa-macro-rules/src/setup_interned_struct.rs @@ -221,7 +221,7 @@ macro_rules! setup_interned_struct { $Db: ?Sized + $zalsa::Database, { let fields = $Configuration::ingredient(db).fields(db.as_dyn_database(), self); - $zalsa::return_mode!( + $zalsa::return_mode_expression!( $field_option, $field_ty, &fields.$field_index, diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs index 6d4e2f50c..ac51848d1 100644 --- a/components/salsa-macro-rules/src/setup_tracked_fn.rs +++ b/components/salsa-macro-rules/src/setup_tracked_fn.rs @@ -366,7 +366,7 @@ macro_rules! setup_tracked_fn { } }; - $zalsa::return_mode!(($return_mode, __, __), $output_ty, result,) + $zalsa::return_mode_expression!(($return_mode, __, __), $output_ty, result,) }) } // The struct needs be last in the macro expansion in order to make the tracked diff --git a/components/salsa-macro-rules/src/setup_tracked_struct.rs b/components/salsa-macro-rules/src/setup_tracked_struct.rs index 41da9c5be..fe06f06d4 100644 --- a/components/salsa-macro-rules/src/setup_tracked_struct.rs +++ b/components/salsa-macro-rules/src/setup_tracked_struct.rs @@ -54,7 +54,7 @@ macro_rules! setup_tracked_struct { // // Each field option is a tuple `(return_mode, maybe_backdate)` where: // - // * `return_mode` is an indentiefier as specified in `salsa_macros::options::Option::returns` + // * `return_mode` is an identifier as specified in `salsa_macros::options::Option::returns` // * `maybe_backdate` is either the identifier `backdate` or `no_backdate` // // These are used to drive conditional logic for each field via recursive macro invocation @@ -65,7 +65,7 @@ macro_rules! setup_tracked_struct { // // Each field option is a tuple `(return_mode, maybe_backdate)` where: // - // * `return_mode` is an indentiefier as specified in `salsa_macros::options::Option::returns` + // * `return_mode` is an identifier as specified in `salsa_macros::options::Option::returns` // * `maybe_backdate` is either the identifier `backdate` or `no_backdate` // // These are used to drive conditional logic for each field via recursive macro invocation @@ -267,7 +267,7 @@ macro_rules! setup_tracked_struct { { let db = db.as_dyn_database(); let fields = $Configuration::ingredient(db).tracked_field(db, self, $relative_tracked_index); - $crate::return_mode!( + $crate::return_mode_expression!( $tracked_option, $tracked_ty, &fields.$absolute_tracked_index, @@ -283,7 +283,7 @@ macro_rules! setup_tracked_struct { { let db = db.as_dyn_database(); let fields = $Configuration::ingredient(db).untracked_field(db, self); - $crate::return_mode!( + $crate::return_mode_expression!( $untracked_option, $untracked_ty, &fields.$absolute_untracked_index, diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs index 8cbdb7df4..5f965978c 100644 --- a/components/salsa-macros/src/salsa_struct.rs +++ b/components/salsa-macros/src/salsa_struct.rs @@ -66,6 +66,7 @@ pub(crate) struct SalsaField<'s> { } const BANNED_FIELD_NAMES: &[&str] = &["from", "new"]; +const ALLOWED_RETURN_MODES: &[&str] = &["copy", "clone", "ref", "deref", "as_ref", "as_deref"]; #[allow(clippy::type_complexity)] pub(crate) const FIELD_OPTION_ATTRIBUTES: &[(&str, fn(&syn::Attribute, &mut SalsaField))] = &[ @@ -387,6 +388,11 @@ impl<'s> SalsaField<'s> { } } + // Validate return mode + if !ALLOWED_RETURN_MODES.iter().any(|mode| mode == &result.returns.to_string()) { + return Err(syn::Error::new(result.returns.span(), format!("Invalid return mode. Allowed modes are: {ALLOWED_RETURN_MODES:?}"))); + } + Ok(result) } diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index 0fba8b142..d39de28bf 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/components/salsa-macros/src/tracked_fn.rs @@ -67,6 +67,8 @@ struct ValidFn<'item> { db_path: &'item syn::Path, } +const ALLOWED_RETURN_MODES: &[&str] = &["copy", "clone", "ref", "deref", "as_ref", "as_deref"]; + #[allow(non_snake_case)] impl Macro { fn try_fn(&self, item: syn::ItemFn) -> syn::Result { @@ -152,6 +154,11 @@ impl Macro { .clone() .unwrap_or(Ident::new("clone", Span::call_site())); + // Validate return mode + if !ALLOWED_RETURN_MODES.iter().any(|mode| mode == &return_mode.to_string()) { + return Err(syn::Error::new(return_mode.span(), format!("Invalid return mode. Allowed modes are: {ALLOWED_RETURN_MODES:?}"))); + } + // 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. diff --git a/src/lib.rs b/src/lib.rs index 68a861feb..1b5cbf6a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,13 +22,13 @@ mod memo_ingredient_indices; mod nonce; #[cfg(feature = "rayon")] mod parallel; +mod return_mode; mod revision; mod runtime; mod salsa_struct; mod storage; mod table; mod tracked_struct; -mod return_mode; mod update; mod views; mod zalsa; @@ -50,11 +50,11 @@ pub use self::event::{Event, EventKind}; pub use self::id::Id; pub use self::input::setter::Setter; pub use self::key::DatabaseKeyIndex; +pub use self::return_mode::SalsaAsDeref; +pub use self::return_mode::SalsaAsRef; pub use self::revision::Revision; pub use self::runtime::Runtime; pub use self::storage::{Storage, StorageHandle}; -pub use self::return_mode::SalsaAsRef; -pub use self::return_mode::SalsaAsDeref; pub use self::update::Update; pub use self::zalsa::IngredientIndex; pub use crate::attach::{attach, with_attached_database}; @@ -74,9 +74,9 @@ pub mod plumbing { pub use std::option::Option::{self, None, Some}; pub use salsa_macro_rules::{ - macro_if, maybe_backdate, return_mode, return_mode_ty, maybe_default, maybe_default_tt, - setup_accumulator_impl, setup_input_struct, setup_interned_struct, setup_method_body, - setup_tracked_fn, setup_tracked_struct, unexpected_cycle_initial, + macro_if, maybe_backdate, maybe_default, maybe_default_tt, return_mode_expression, + return_mode_ty, setup_accumulator_impl, setup_input_struct, setup_interned_struct, + setup_method_body, setup_tracked_fn, setup_tracked_struct, unexpected_cycle_initial, unexpected_cycle_recovery, }; diff --git a/src/return_mode.rs b/src/return_mode.rs index 994f15a8f..ff59fc23b 100644 --- a/src/return_mode.rs +++ b/src/return_mode.rs @@ -1,10 +1,13 @@ -//! User-implementable salsa traits +//! User-implementable salsa traits for refining the return type via `returns(as_ref)` and `returns(as_deref)`. use std::ops::Deref; +/// Used to determine the return type and value for tracked fields and functions annotated with `returns(as_ref)`. pub trait SalsaAsRef { + // The type returned by tracked fields and functions annotated with `returns(as_ref)`. type AsRef<'a> where Self: 'a; + // The value returned by tracked fields and functions annotated with `returns(as_ref)`. fn as_ref(&self) -> Self::AsRef<'_>; } @@ -24,16 +27,19 @@ impl SalsaAsRef for Result { } } +/// Used to determine the return type and value for tracked fields and functions annotated with `returns(as_deref)`. pub trait SalsaAsDeref { + // The type returned by tracked fields and functions annotated with `returns(as_deref)`. type AsDeref<'a> where Self: 'a; - fn as_ref(&self) -> Self::AsDeref<'_>; + // The value returned by tracked fields and functions annotated with `returns(as_deref)`. + fn as_deref(&self) -> Self::AsDeref<'_>; } impl SalsaAsDeref for Option { type AsDeref<'a> = Option<&'a T::Target> where Self: 'a; - fn as_ref(&self) -> Self::AsDeref<'_> { + fn as_deref(&self) -> Self::AsDeref<'_> { self.as_deref() } } @@ -41,7 +47,7 @@ impl SalsaAsDeref for Option { impl SalsaAsDeref for Result { type AsDeref<'a> = Result<&'a T::Target, &'a E> where Self: 'a; - fn as_ref(&self) -> Self::AsDeref<'_> { + fn as_deref(&self) -> Self::AsDeref<'_> { self.as_deref() } } diff --git a/tests/compile-fail/invalid_return_mode.rs b/tests/compile-fail/invalid_return_mode.rs new file mode 100644 index 000000000..c69287f2a --- /dev/null +++ b/tests/compile-fail/invalid_return_mode.rs @@ -0,0 +1,20 @@ +use salsa::Database as Db; + +#[salsa::input] +struct MyInput { + #[returns(clone)] + text: String, +} + +#[salsa::tracked(returns(not_a_return_mode))] +fn tracked_fn_invalid_return_mode(db: &dyn Db, input: MyInput) -> String { + input.text(db) +} + +#[salsa::input] +struct MyInvalidInput { + #[returns(not_a_return_mode)] + text: String, +} + +fn main() { } \ No newline at end of file diff --git a/tests/compile-fail/invalid_return_mode.stderr b/tests/compile-fail/invalid_return_mode.stderr new file mode 100644 index 000000000..fd3390909 --- /dev/null +++ b/tests/compile-fail/invalid_return_mode.stderr @@ -0,0 +1,17 @@ +error: Invalid return mode. Allowed modes are: ["copy", "clone", "ref", "deref", "as_ref", "as_deref"] + --> tests/compile-fail/invalid_return_mode.rs:9:26 + | +9 | #[salsa::tracked(returns(not_a_return_mode))] + | ^^^^^^^^^^^^^^^^^ + +error: Invalid return mode. Allowed modes are: ["copy", "clone", "ref", "deref", "as_ref", "as_deref"] + --> tests/compile-fail/invalid_return_mode.rs:16:15 + | +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/return_mode.rs b/tests/return_mode.rs new file mode 100644 index 000000000..09b4c8e19 --- /dev/null +++ b/tests/return_mode.rs @@ -0,0 +1,172 @@ +use salsa::Database; + +#[salsa::input] +struct DefaultInput { + text: String, +} + +#[salsa::tracked] +fn default_fn(db: &dyn Database, input: DefaultInput) -> String { + let input: String = input.text(db); + input +} + +#[test] +fn default_test() { + salsa::DatabaseImpl::new().attach(|db| { + let input = DefaultInput::new(db, "Test".into()); + let x: String = default_fn(db, input); + expect_test::expect![[r#" + "Test" + "#]] + .assert_debug_eq(&x); + }) +} + +#[salsa::input] +struct CopyInput { + #[returns(copy)] + text: &'static str, +} + +#[salsa::tracked(returns(copy))] +fn copy_fn(db: &dyn Database, input: CopyInput) -> &'static str { + let input: &'static str = input.text(db); + input +} + +#[test] +fn copy_test() { + salsa::DatabaseImpl::new().attach(|db| { + let input = CopyInput::new(db, "Test"); + let x: &str = copy_fn(db, input); + expect_test::expect![[r#" + "Test" + "#]] + .assert_debug_eq(&x); + }) +} + +#[salsa::input] +struct CloneInput { + #[returns(clone)] + text: String, +} + +#[salsa::tracked(returns(clone))] +fn clone_fn(db: &dyn Database, input: CloneInput) -> String { + let input: String = input.text(db); + input +} + +#[test] +fn clone_test() { + salsa::DatabaseImpl::new().attach(|db| { + let input = CloneInput::new(db, "Test".into()); + let x: String = clone_fn(db, input); + expect_test::expect![[r#" + "Test" + "#]] + .assert_debug_eq(&x); + }) +} + +#[salsa::input] +struct RefInput { + #[returns(ref)] + text: String, +} + +#[salsa::tracked(returns(ref))] +fn ref_fn(db: &dyn Database, input: RefInput) -> String { + let input: &String = input.text(db); + input.to_owned() +} + +#[test] +fn ref_test() { + salsa::DatabaseImpl::new().attach(|db| { + let input = RefInput::new(db, "Test".into()); + let x: &String = ref_fn(db, input); + expect_test::expect![[r#" + "Test" + "#]] + .assert_debug_eq(&x); + }) +} + +#[salsa::input] +struct DerefInput { + #[returns(deref)] + text: String, +} + +#[salsa::tracked(returns(deref))] +fn deref_fn(db: &dyn Database, input: DerefInput) -> String { + let input: &str = input.text(db); + input.to_owned() +} + +#[test] +fn deref_test() { + salsa::DatabaseImpl::new().attach(|db| { + let input = DerefInput::new(db, "Test".into()); + let x: &str = deref_fn(db, input); + expect_test::expect![[r#" + "Test" + "#]] + .assert_debug_eq(&x); + }) +} + +#[salsa::input] +struct AsRefInput { + #[returns(as_ref)] + text: Option, +} + +#[salsa::tracked(returns(as_ref))] +fn as_ref_fn(db: &dyn Database, input: AsRefInput) -> Option { + let input: Option<&String> = input.text(db); + input.cloned() +} + +#[test] +fn as_ref_test() { + salsa::DatabaseImpl::new().attach(|db| { + let input = AsRefInput::new(db, Some("Test".into())); + let x: Option<&String> = as_ref_fn(db, input); + expect_test::expect![[r#" + Some( + "Test", + ) + "#]] + .assert_debug_eq(&x); + }) +} + +#[salsa::input] +struct AsDerefInput { + #[returns(as_deref)] + text: Option, +} + +#[salsa::tracked(returns(as_deref))] +fn as_deref_fn(db: &dyn Database, input: AsDerefInput) -> Option { + let input: Option<&str> = input.text(db); + input.map(|s| s.to_owned()) +} + +#[test] +fn as_deref_test() { + salsa::DatabaseImpl::new().attach(|db| { + let input = AsDerefInput::new(db, Some("Test".into())); + let x: Option<&str> = as_deref_fn(db, input); + expect_test::expect![[r#" + Some( + "Test", + ) + "#]] + .assert_debug_eq(&x); + }) +} \ No newline at end of file From e162825a96d1c81092fbaf0a0da9a6c16cd3ce80 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 9 May 2025 09:13:58 +0200 Subject: [PATCH 5/5] Cargo fmt --- .../salsa-macro-rules/src/return_mode.rs | 6 ++-- components/salsa-macros/src/salsa_struct.rs | 10 +++++-- components/salsa-macros/src/tracked_fn.rs | 10 +++++-- src/return_mode.rs | 28 +++++++++++++++---- tests/return_mode.rs | 2 +- 5 files changed, 42 insertions(+), 14 deletions(-) diff --git a/components/salsa-macro-rules/src/return_mode.rs b/components/salsa-macro-rules/src/return_mode.rs index e5a3338a3..32d0e9950 100644 --- a/components/salsa-macro-rules/src/return_mode.rs +++ b/components/salsa-macro-rules/src/return_mode.rs @@ -18,7 +18,7 @@ macro_rules! return_mode_expression { ) => { ::core::clone::Clone::clone($field_ref_expr) }; - + ( (ref, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, @@ -34,7 +34,7 @@ macro_rules! return_mode_expression { ) => { ::core::ops::Deref::deref($field_ref_expr) }; - + ( (as_ref, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, @@ -42,7 +42,7 @@ macro_rules! return_mode_expression { ) => { ::salsa::SalsaAsRef::as_ref($field_ref_expr) }; - + ( (as_deref, $maybe_backdate:ident, $maybe_default:ident), $field_ty:ty, diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs index 5f965978c..bec4121db 100644 --- a/components/salsa-macros/src/salsa_struct.rs +++ b/components/salsa-macros/src/salsa_struct.rs @@ -389,8 +389,14 @@ impl<'s> SalsaField<'s> { } // Validate return mode - if !ALLOWED_RETURN_MODES.iter().any(|mode| mode == &result.returns.to_string()) { - return Err(syn::Error::new(result.returns.span(), format!("Invalid return mode. Allowed modes are: {ALLOWED_RETURN_MODES:?}"))); + if !ALLOWED_RETURN_MODES + .iter() + .any(|mode| mode == &result.returns.to_string()) + { + return Err(syn::Error::new( + result.returns.span(), + format!("Invalid return mode. Allowed modes are: {ALLOWED_RETURN_MODES:?}"), + )); } Ok(result) diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index d39de28bf..5022a18a2 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/components/salsa-macros/src/tracked_fn.rs @@ -155,8 +155,14 @@ impl Macro { .unwrap_or(Ident::new("clone", Span::call_site())); // Validate return mode - if !ALLOWED_RETURN_MODES.iter().any(|mode| mode == &return_mode.to_string()) { - return Err(syn::Error::new(return_mode.span(), format!("Invalid return mode. Allowed modes are: {ALLOWED_RETURN_MODES:?}"))); + if !ALLOWED_RETURN_MODES + .iter() + .any(|mode| mode == &return_mode.to_string()) + { + return Err(syn::Error::new( + return_mode.span(), + format!("Invalid return mode. Allowed modes are: {ALLOWED_RETURN_MODES:?}"), + )); } // The path expression is responsible for emitting the primary span in the diagnostic we diff --git a/src/return_mode.rs b/src/return_mode.rs index ff59fc23b..d5dcca79c 100644 --- a/src/return_mode.rs +++ b/src/return_mode.rs @@ -5,14 +5,19 @@ use std::ops::Deref; /// Used to determine the return type and value for tracked fields and functions annotated with `returns(as_ref)`. pub trait SalsaAsRef { // The type returned by tracked fields and functions annotated with `returns(as_ref)`. - type AsRef<'a> where Self: 'a; + type AsRef<'a> + where + Self: 'a; // The value returned by tracked fields and functions annotated with `returns(as_ref)`. fn as_ref(&self) -> Self::AsRef<'_>; } impl SalsaAsRef for Option { - type AsRef<'a> = Option<&'a T> where Self: 'a; + type AsRef<'a> + = Option<&'a T> + where + Self: 'a; fn as_ref(&self) -> Self::AsRef<'_> { self.as_ref() @@ -20,7 +25,10 @@ impl SalsaAsRef for Option { } impl SalsaAsRef for Result { - type AsRef<'a> = Result<&'a T, &'a E> where Self: 'a; + type AsRef<'a> + = Result<&'a T, &'a E> + where + Self: 'a; fn as_ref(&self) -> Self::AsRef<'_> { self.as_ref() @@ -30,14 +38,19 @@ impl SalsaAsRef for Result { /// Used to determine the return type and value for tracked fields and functions annotated with `returns(as_deref)`. pub trait SalsaAsDeref { // The type returned by tracked fields and functions annotated with `returns(as_deref)`. - type AsDeref<'a> where Self: 'a; + type AsDeref<'a> + where + Self: 'a; // The value returned by tracked fields and functions annotated with `returns(as_deref)`. fn as_deref(&self) -> Self::AsDeref<'_>; } impl SalsaAsDeref for Option { - type AsDeref<'a> = Option<&'a T::Target> where Self: 'a; + type AsDeref<'a> + = Option<&'a T::Target> + where + Self: 'a; fn as_deref(&self) -> Self::AsDeref<'_> { self.as_deref() @@ -45,7 +58,10 @@ impl SalsaAsDeref for Option { } impl SalsaAsDeref for Result { - type AsDeref<'a> = Result<&'a T::Target, &'a E> where Self: 'a; + type AsDeref<'a> + = Result<&'a T::Target, &'a E> + where + Self: 'a; fn as_deref(&self) -> Self::AsDeref<'_> { self.as_deref() diff --git a/tests/return_mode.rs b/tests/return_mode.rs index 09b4c8e19..34f676288 100644 --- a/tests/return_mode.rs +++ b/tests/return_mode.rs @@ -169,4 +169,4 @@ fn as_deref_test() { "#]] .assert_debug_eq(&x); }) -} \ No newline at end of file +}