Skip to content

Commit 8c49bb0

Browse files
nikomatsakisnikomatsakis
authored
Merge pull request #661 from davidbarsky/davidbarsky/interned-structs-without-lifetimes-take-two
feature: allow skipping the lifetime on `#[salsa::interned]` structs
2 parents 917cfa9 + a1f61d0 commit 8c49bb0

File tree

10 files changed

+236
-25
lines changed

10 files changed

+236
-25
lines changed

‎components/salsa-macro-rules/src/setup_interned_struct.rs

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,28 @@ macro_rules! setup_interned_struct {
1111
// Name of the struct
1212
Struct: $Struct:ident,
1313

14+
// Name of the struct data. This is a parameter because `std::concat_idents`
15+
// is unstable and taking an additional dependency is unnecessary.
16+
StructData: $StructDataIdent:ident,
17+
18+
// Name of the struct type with a `'static` argument (unless this type has no db lifetime,
19+
// in which case this is the same as `$Struct`)
20+
StructWithStatic: $StructWithStatic:ty,
21+
1422
// Name of the `'db` lifetime that the user gave
1523
db_lt: $db_lt:lifetime,
1624

25+
// optional db lifetime argument.
26+
db_lt_arg: $($db_lt_arg:lifetime)?,
27+
28+
// the salsa ID
29+
id: $Id:path,
30+
31+
// the lifetime used in the desugared interned struct.
32+
// if the `db_lt_arg`, is present, this is `db_lt_arg`, but otherwise,
33+
// it is `'static`.
34+
interior_lt: $interior_lt:lifetime,
35+
1736
// Name user gave for `new`
1837
new_fn: $new_fn:ident,
1938

@@ -54,18 +73,18 @@ macro_rules! setup_interned_struct {
5473
) => {
5574
$(#[$attr])*
5675
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
57-
$vis struct $Struct<$db_lt>(
58-
salsa::Id,
59-
std::marker::PhantomData < & $db_lt salsa::plumbing::interned::Value < $Struct<'static> > >
76+
$vis struct $Struct< $($db_lt_arg)? >(
77+
$Id,
78+
std::marker::PhantomData < & $interior_lt salsa::plumbing::interned::Value <$StructWithStatic> >
6079
);
6180

6281
const _: () = {
6382
use salsa::plumbing as $zalsa;
6483
use $zalsa::interned as $zalsa_struct;
6584

66-
type $Configuration = $Struct<'static>;
85+
type $Configuration = $StructWithStatic;
6786

68-
type StructData<$db_lt> = ($($field_ty,)*);
87+
type $StructDataIdent<$db_lt> = ($($field_ty,)*);
6988

7089
/// Key to use during hash lookups. Each field is some type that implements `Lookup<T>`
7190
/// for the owned type. This permits interning with an `&str` when a `String` is required and so forth.
@@ -76,7 +95,7 @@ macro_rules! setup_interned_struct {
7695
);
7796

7897
impl<$db_lt, $($indexed_ty,)*> $zalsa::interned::HashEqLike<StructKey<$db_lt, $($indexed_ty),*>>
79-
for StructData<$db_lt>
98+
for $StructDataIdent<$db_lt>
8099
where
81100
$($field_ty: $zalsa::interned::HashEqLike<$indexed_ty>),*
82101
{
@@ -90,24 +109,27 @@ macro_rules! setup_interned_struct {
90109
}
91110
}
92111

93-
impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<StructData<$db_lt>>
112+
impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<$StructDataIdent<$db_lt>>
94113
for StructKey<$db_lt, $($indexed_ty),*> {
95114

96115
#[allow(unused_unit)]
97-
fn into_owned(self) -> StructData<$db_lt> {
116+
fn into_owned(self) -> $StructDataIdent<$db_lt> {
98117
($($zalsa::interned::Lookup::into_owned(self.$field_index),)*)
99118
}
100119
}
101120

102-
impl $zalsa_struct::Configuration for $Configuration {
121+
impl salsa::plumbing::interned::Configuration for $StructWithStatic {
103122
const DEBUG_NAME: &'static str = stringify!($Struct);
104-
type Data<'a> = StructData<'a>;
105-
type Struct<'a> = $Struct<'a>;
123+
type Data<'a> = $StructDataIdent<'a>;
124+
type Struct<'db> = $Struct< $($db_lt_arg)? >;
106125
fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
107-
$Struct(id, std::marker::PhantomData)
126+
use salsa::plumbing::FromId;
127+
$Struct(<$Id>::from_id(id), std::marker::PhantomData)
108128
}
129+
109130
fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
110-
s.0
131+
use salsa::plumbing::AsId;
132+
s.0.as_id()
111133
}
112134
}
113135

@@ -124,37 +146,37 @@ macro_rules! setup_interned_struct {
124146
}
125147
}
126148

127-
impl $zalsa::AsId for $Struct<'_> {
149+
impl< $($db_lt_arg)? > $zalsa::AsId for $Struct< $($db_lt_arg)? > {
128150
fn as_id(&self) -> salsa::Id {
129-
self.0
151+
self.0.as_id()
130152
}
131153
}
132154

133-
impl $zalsa::FromId for $Struct<'_> {
155+
impl< $($db_lt_arg)? > $zalsa::FromId for $Struct< $($db_lt_arg)? > {
134156
fn from_id(id: salsa::Id) -> Self {
135-
Self(id, std::marker::PhantomData)
157+
Self(<$Id>::from_id(id), std::marker::PhantomData)
136158
}
137159
}
138160

139-
unsafe impl Send for $Struct<'_> {}
161+
unsafe impl< $($db_lt_arg)? > Send for $Struct< $($db_lt_arg)? > {}
140162

141-
unsafe impl Sync for $Struct<'_> {}
163+
unsafe impl< $($db_lt_arg)? > Sync for $Struct< $($db_lt_arg)? > {}
142164

143165
$zalsa::macro_if! { $generate_debug_impl =>
144-
impl std::fmt::Debug for $Struct<'_> {
166+
impl< $($db_lt_arg)? > std::fmt::Debug for $Struct< $($db_lt_arg)? > {
145167
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
146168
Self::default_debug_fmt(*self, f)
147169
}
148170
}
149171
}
150172

151-
impl $zalsa::SalsaStructInDb for $Struct<'_> {
173+
impl< $($db_lt_arg)? > $zalsa::SalsaStructInDb for $Struct< $($db_lt_arg)? > {
152174
fn lookup_ingredient_index(aux: &dyn $zalsa::JarAux) -> core::option::Option<$zalsa::IngredientIndex> {
153175
aux.lookup_jar_by_type(&<$zalsa_struct::JarImpl<$Configuration>>::default())
154176
}
155177
}
156178

157-
unsafe impl $zalsa::Update for $Struct<'_> {
179+
unsafe impl< $($db_lt_arg)? > $zalsa::Update for $Struct< $($db_lt_arg)? > {
158180
unsafe fn maybe_update(old_pointer: *mut Self, new_value: Self) -> bool {
159181
if unsafe { *old_pointer } != new_value {
160182
unsafe { *old_pointer = new_value };
@@ -165,7 +187,7 @@ macro_rules! setup_interned_struct {
165187
}
166188
}
167189

168-
impl<$db_lt> $Struct<$db_lt> {
190+
impl<$db_lt> $Struct< $($db_lt_arg)? > {
169191
pub fn $new_fn<$Db, $($indexed_ty: $zalsa::interned::Lookup<$field_ty> + std::hash::Hash,)*>(db: &$db_lt $Db, $($field_id: $indexed_ty),*) -> Self
170192
where
171193
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`

‎components/salsa-macros/src/accumulator.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,14 @@ impl AllowedOptions for Accumulator {
3636
const NO_EQ: bool = false;
3737
const NO_DEBUG: bool = true;
3838
const NO_CLONE: bool = true;
39+
const NO_LIFETIME: bool = false;
3940
const SINGLETON: bool = false;
4041
const DATA: bool = false;
4142
const DB: bool = false;
4243
const RECOVERY_FN: bool = false;
4344
const LRU: bool = false;
4445
const CONSTRUCTOR_NAME: bool = false;
46+
const ID: bool = false;
4547
}
4648

4749
struct StructMacro {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ impl crate::options::AllowedOptions for InputStruct {
4242

4343
const NO_DEBUG: bool = true;
4444

45+
const NO_LIFETIME: bool = false;
46+
4547
const NO_CLONE: bool = false;
4648

4749
const SINGLETON: bool = true;
@@ -55,6 +57,8 @@ impl crate::options::AllowedOptions for InputStruct {
5557
const LRU: bool = false;
5658

5759
const CONSTRUCTOR_NAME: bool = true;
60+
61+
const ID: bool = false;
5862
}
5963

6064
impl SalsaStructAllowedOptions for InputStruct {
@@ -64,6 +68,8 @@ impl SalsaStructAllowedOptions for InputStruct {
6468

6569
const HAS_LIFETIME: bool = false;
6670

71+
const ELIDABLE_LIFETIME: bool = false;
72+
6773
const ALLOW_DEFAULT: bool = true;
6874
}
6975

‎components/salsa-macros/src/interned.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ impl crate::options::AllowedOptions for InternedStruct {
4343

4444
const NO_DEBUG: bool = true;
4545

46+
const NO_LIFETIME: bool = true;
47+
4648
const NO_CLONE: bool = false;
4749

4850
const SINGLETON: bool = true;
@@ -56,6 +58,8 @@ impl crate::options::AllowedOptions for InternedStruct {
5658
const LRU: bool = false;
5759

5860
const CONSTRUCTOR_NAME: bool = true;
61+
62+
const ID: bool = true;
5963
}
6064

6165
impl SalsaStructAllowedOptions for InternedStruct {
@@ -65,6 +69,8 @@ impl SalsaStructAllowedOptions for InternedStruct {
6569

6670
const HAS_LIFETIME: bool = true;
6771

72+
const ELIDABLE_LIFETIME: bool = true;
73+
6874
const ALLOW_DEFAULT: bool = false;
6975
}
7076

@@ -82,6 +88,7 @@ impl Macro {
8288
let attrs = &self.struct_item.attrs;
8389
let vis = &self.struct_item.vis;
8490
let struct_ident = &self.struct_item.ident;
91+
let struct_data_ident = format_ident!("{}Data", struct_ident);
8592
let db_lt = db_lifetime::db_lifetime(&self.struct_item.generics);
8693
let new_fn = salsa_struct.constructor_name();
8794
let field_ids = salsa_struct.field_ids();
@@ -93,6 +100,24 @@ impl Macro {
93100
let field_tys = salsa_struct.field_tys();
94101
let field_indexed_tys = salsa_struct.field_indexed_tys();
95102
let generate_debug_impl = salsa_struct.generate_debug_impl();
103+
let has_lifetime = salsa_struct.generate_lifetime();
104+
let id = salsa_struct.id();
105+
106+
let (db_lt_arg, cfg, interior_lt) = if has_lifetime {
107+
(
108+
Some(db_lt.clone()),
109+
quote!(#struct_ident<'static>),
110+
db_lt.clone(),
111+
)
112+
} else {
113+
let span = syn::spanned::Spanned::span(&self.struct_item.generics);
114+
let static_lifetime = syn::Lifetime {
115+
apostrophe: span,
116+
ident: syn::Ident::new("static", span),
117+
};
118+
119+
(None, quote!(#struct_ident), static_lifetime)
120+
};
96121

97122
let zalsa = self.hygiene.ident("zalsa");
98123
let zalsa_struct = self.hygiene.ident("zalsa_struct");
@@ -107,7 +132,12 @@ impl Macro {
107132
attrs: [#(#attrs),*],
108133
vis: #vis,
109134
Struct: #struct_ident,
135+
StructData: #struct_data_ident,
136+
StructWithStatic: #cfg,
110137
db_lt: #db_lt,
138+
db_lt_arg: #db_lt_arg,
139+
id: #id,
140+
interior_lt: #interior_lt,
111141
new_fn: #new_fn,
112142
field_options: [#(#field_options),*],
113143
field_ids: [#(#field_ids),*],

‎components/salsa-macros/src/options.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use syn::{ext::IdentExt, spanned::Spanned};
77
/// are required and trailing commas are permitted. The options accepted
88
/// for any particular location are configured via the `AllowedOptions`
99
/// trait.
10+
#[derive(Debug)]
1011
pub(crate) struct Options<A: AllowedOptions> {
1112
/// The `return_ref` option is used to signal that field/return type is "by ref"
1213
///
@@ -24,6 +25,11 @@ pub(crate) struct Options<A: AllowedOptions> {
2425
/// If this is `Some`, the value is the `no_debug` identifier.
2526
pub no_debug: Option<syn::Ident>,
2627

28+
/// Signal we should not include the `'db` lifetime.
29+
///
30+
/// If this is `Some`, the value is the `no_lifetime` identifier.
31+
pub no_lifetime: Option<syn::Ident>,
32+
2733
/// Signal we should not generate a `Clone` impl.
2834
///
2935
/// If this is `Some`, the value is the `no_clone` identifier.
@@ -66,6 +72,12 @@ pub(crate) struct Options<A: AllowedOptions> {
6672
/// If this is `Some`, the value is the `<ident>`.
6773
pub constructor_name: Option<syn::Ident>,
6874

75+
/// The `id = <path>` option is used to set a custom ID for interrned structs.
76+
///
77+
/// The ID must implement `salsa::plumbing::AsId` and `salsa::plumbing::FromId`.
78+
/// If this is `Some`, the value is the `<ident>`.
79+
pub id: Option<syn::Path>,
80+
6981
/// Remember the `A` parameter, which plays no role after parsing.
7082
phantom: PhantomData<A>,
7183
}
@@ -77,6 +89,7 @@ impl<A: AllowedOptions> Default for Options<A> {
7789
specify: Default::default(),
7890
no_eq: Default::default(),
7991
no_debug: Default::default(),
92+
no_lifetime: Default::default(),
8093
no_clone: Default::default(),
8194
db_path: Default::default(),
8295
recovery_fn: Default::default(),
@@ -85,6 +98,7 @@ impl<A: AllowedOptions> Default for Options<A> {
8598
phantom: Default::default(),
8699
lru: Default::default(),
87100
singleton: Default::default(),
101+
id: Default::default(),
88102
}
89103
}
90104
}
@@ -95,13 +109,15 @@ pub(crate) trait AllowedOptions {
95109
const SPECIFY: bool;
96110
const NO_EQ: bool;
97111
const NO_DEBUG: bool;
112+
const NO_LIFETIME: bool;
98113
const NO_CLONE: bool;
99114
const SINGLETON: bool;
100115
const DATA: bool;
101116
const DB: bool;
102117
const RECOVERY_FN: bool;
103118
const LRU: bool;
104119
const CONSTRUCTOR_NAME: bool;
120+
const ID: bool;
105121
}
106122

107123
type Equals = syn::Token![=];
@@ -152,6 +168,20 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
152168
"`no_debug` option not allowed here",
153169
));
154170
}
171+
} else if ident == "no_lifetime" {
172+
if A::NO_LIFETIME {
173+
if let Some(old) = std::mem::replace(&mut options.no_lifetime, Some(ident)) {
174+
return Err(syn::Error::new(
175+
old.span(),
176+
"option `no_lifetime` provided twice",
177+
));
178+
}
179+
} else {
180+
return Err(syn::Error::new(
181+
ident.span(),
182+
"`no_lifetime` option not allowed here",
183+
));
184+
}
155185
} else if ident == "no_clone" {
156186
if A::NO_CLONE {
157187
if let Some(old) = std::mem::replace(&mut options.no_clone, Some(ident)) {
@@ -267,6 +297,17 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
267297
"`constructor` option not allowed here",
268298
));
269299
}
300+
} else if ident == "id" {
301+
if A::ID {
302+
let _eq = Equals::parse(input)?;
303+
let path = syn::Path::parse(input)?;
304+
options.id = Some(path);
305+
} else {
306+
return Err(syn::Error::new(
307+
ident.span(),
308+
"`id` option not allowed here",
309+
));
310+
}
270311
} else {
271312
return Err(syn::Error::new(
272313
ident.span(),

0 commit comments

Comments
 (0)