Skip to content

Commit 2d4321e

Browse files
authored
Implement an !Update bound escape hatch for tracked fn (#867)
1 parent 4818b15 commit 2d4321e

File tree

7 files changed

+65
-23
lines changed

7 files changed

+65
-23
lines changed

components/salsa-macros/src/accumulator.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ impl AllowedOptions for Accumulator {
3333
const SPECIFY: bool = false;
3434
const NO_EQ: bool = false;
3535
const DEBUG: bool = false;
36-
const NO_CLONE: bool = false;
36+
const NON_UPDATE_RETURN_TYPE: bool = false;
3737
const NO_LIFETIME: bool = false;
3838
const SINGLETON: bool = false;
3939
const DATA: bool = false;

components/salsa-macros/src/input.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ impl crate::options::AllowedOptions for InputStruct {
4343

4444
const NO_LIFETIME: bool = false;
4545

46-
const NO_CLONE: bool = false;
46+
const NON_UPDATE_RETURN_TYPE: bool = false;
4747

4848
const SINGLETON: bool = true;
4949

components/salsa-macros/src/interned.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ impl crate::options::AllowedOptions for InternedStruct {
4343

4444
const NO_LIFETIME: bool = true;
4545

46-
const NO_CLONE: bool = false;
46+
const NON_UPDATE_RETURN_TYPE: bool = false;
4747

4848
const SINGLETON: bool = true;
4949

components/salsa-macros/src/options.rs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ pub(crate) struct Options<A: AllowedOptions> {
3333
/// If this is `Some`, the value is the `no_lifetime` identifier.
3434
pub no_lifetime: Option<syn::Ident>,
3535

36-
/// Signal we should not generate a `Clone` impl.
37-
///
38-
/// If this is `Some`, the value is the `no_clone` identifier.
39-
pub no_clone: Option<syn::Ident>,
40-
4136
/// The `singleton` option is used on input with only one field
4237
/// It allows the creation of convenient methods
4338
pub singleton: Option<syn::Ident>,
@@ -48,6 +43,13 @@ pub(crate) struct Options<A: AllowedOptions> {
4843
/// If this is `Some`, the value is the `specify` identifier.
4944
pub specify: Option<syn::Ident>,
5045

46+
/// The `non_update_return_type` option is used to signal that a tracked function's
47+
/// return type does not require `Update` to be implemented. This is unsafe and
48+
/// generally discouraged as it allows for dangling references.
49+
///
50+
/// If this is `Some`, the value is the `non_update_return_type` identifier.
51+
pub non_update_return_type: Option<syn::Ident>,
52+
5153
/// The `db = <path>` option is used to indicate the db.
5254
///
5355
/// If this is `Some`, the value is the `<path>`.
@@ -100,10 +102,10 @@ impl<A: AllowedOptions> Default for Options<A> {
100102
Self {
101103
returns: Default::default(),
102104
specify: Default::default(),
105+
non_update_return_type: Default::default(),
103106
no_eq: Default::default(),
104107
debug: Default::default(),
105108
no_lifetime: Default::default(),
106-
no_clone: Default::default(),
107109
db_path: Default::default(),
108110
cycle_fn: Default::default(),
109111
cycle_initial: Default::default(),
@@ -125,7 +127,7 @@ pub(crate) trait AllowedOptions {
125127
const NO_EQ: bool;
126128
const DEBUG: bool;
127129
const NO_LIFETIME: bool;
128-
const NO_CLONE: bool;
130+
const NON_UPDATE_RETURN_TYPE: bool;
129131
const SINGLETON: bool;
130132
const DATA: bool;
131133
const DB: bool;
@@ -199,18 +201,28 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
199201
"`no_lifetime` option not allowed here",
200202
));
201203
}
202-
} else if ident == "no_clone" {
203-
if A::NO_CLONE {
204-
if let Some(old) = options.no_clone.replace(ident) {
204+
} else if ident == "unsafe" {
205+
if A::NON_UPDATE_RETURN_TYPE {
206+
let content;
207+
parenthesized!(content in input);
208+
let ident = syn::Ident::parse_any(&content)?;
209+
if ident == "non_update_return_type" {
210+
if let Some(old) = options.non_update_return_type.replace(ident) {
211+
return Err(syn::Error::new(
212+
old.span(),
213+
"option `non_update_return_type` provided twice",
214+
));
215+
}
216+
} else {
205217
return Err(syn::Error::new(
206-
old.span(),
207-
"option `no_clone` provided twice",
218+
ident.span(),
219+
"expected `non_update_return_type`",
208220
));
209221
}
210222
} else {
211223
return Err(syn::Error::new(
212224
ident.span(),
213-
"`no_clone` option not allowed here",
225+
"`unsafe` options not allowed here",
214226
));
215227
}
216228
} else if ident == "singleton" {

components/salsa-macros/src/tracked_fn.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl crate::options::AllowedOptions for TrackedFn {
3636

3737
const NO_LIFETIME: bool = false;
3838

39-
const NO_CLONE: bool = false;
39+
const NON_UPDATE_RETURN_TYPE: bool = true;
4040

4141
const SINGLETON: bool = false;
4242

@@ -84,6 +84,7 @@ impl Macro {
8484
let (cycle_recovery_fn, cycle_recovery_initial, cycle_recovery_strategy) =
8585
self.cycle_recovery()?;
8686
let is_specifiable = self.args.specify.is_some();
87+
let requires_update = self.args.non_update_return_type.is_none();
8788
let eq = if let Some(token) = &self.args.no_eq {
8889
if self.args.cycle_fn.is_some() {
8990
return Err(syn::Error::new_spanned(
@@ -172,12 +173,16 @@ impl Macro {
172173
let maybe_update_path = quote_spanned! {output_ty.span() =>
173174
UpdateDispatch::<#output_ty>::maybe_update
174175
};
175-
let assert_return_type_is_update = quote! {
176-
#[allow(clippy::all, warnings)]
177-
fn _assert_return_type_is_update<#db_lt>() {
178-
use #zalsa::{UpdateFallback, UpdateDispatch};
179-
#maybe_update_path;
176+
let assert_return_type_is_update = if requires_update {
177+
quote! {
178+
#[allow(clippy::all, warnings)]
179+
fn _assert_return_type_is_update<#db_lt>() {
180+
use #zalsa::{UpdateFallback, UpdateDispatch};
181+
#maybe_update_path;
182+
}
180183
}
184+
} else {
185+
quote! {}
181186
};
182187

183188
Ok(crate::debug::dump_tokens(

components/salsa-macros/src/tracked_struct.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ impl crate::options::AllowedOptions for TrackedStruct {
3838

3939
const NO_LIFETIME: bool = false;
4040

41-
const NO_CLONE: bool = false;
41+
const NON_UPDATE_RETURN_TYPE: bool = false;
4242

4343
const SINGLETON: bool = true;
4444

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//! Test that a `tracked` fn on a `salsa::input`
2+
//! compiles and executes successfully.
3+
#![allow(warnings)]
4+
5+
use std::marker::PhantomData;
6+
7+
#[salsa::input]
8+
struct MyInput {
9+
field: u32,
10+
}
11+
12+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
13+
struct NotUpdate<'a>(PhantomData<fn() -> &'a ()>);
14+
15+
#[salsa::tracked(unsafe(non_update_return_type))]
16+
fn tracked_fn(db: &dyn salsa::Database, input: MyInput) -> NotUpdate<'_> {
17+
NotUpdate(PhantomData)
18+
}
19+
20+
#[test]
21+
fn execute() {
22+
let mut db = salsa::DatabaseImpl::new();
23+
let input = MyInput::new(&db, 22);
24+
tracked_fn(&db, input);
25+
}

0 commit comments

Comments
 (0)