Skip to content

feature: allow skipping the lifetime on #[salsa::interned] structs #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 45 additions & 23 deletions components/salsa-macro-rules/src/setup_interned_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,28 @@ macro_rules! setup_interned_struct {
// Name of the struct
Struct: $Struct:ident,

// Name of the struct data. This is a parameter because `std::concat_idents`
// is unstable and taking an additional dependency is unnecessary.
StructData: $StructDataIdent:ident,

// Name of the struct type with a `'static` argument (unless this type has no db lifetime,
// in which case this is the same as `$Struct`)
StructWithStatic: $StructWithStatic:ty,

// Name of the `'db` lifetime that the user gave
db_lt: $db_lt:lifetime,

// optional db lifetime argument.
db_lt_arg: $($db_lt_arg:lifetime)?,

// the salsa ID
id: $Id:path,

// the lifetime used in the desugared interned struct.
// if the `db_lt_arg`, is present, this is `db_lt_arg`, but otherwise,
// it is `'static`.
interior_lt: $interior_lt:lifetime,

// Name user gave for `new`
new_fn: $new_fn:ident,

Expand Down Expand Up @@ -54,18 +73,18 @@ macro_rules! setup_interned_struct {
) => {
$(#[$attr])*
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
$vis struct $Struct<$db_lt>(
salsa::Id,
std::marker::PhantomData < & $db_lt salsa::plumbing::interned::Value < $Struct<'static> > >
$vis struct $Struct< $($db_lt_arg)? >(
$Id,
std::marker::PhantomData < & $interior_lt salsa::plumbing::interned::Value <$StructWithStatic> >
);

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

type $Configuration = $Struct<'static>;
type $Configuration = $StructWithStatic;

type StructData<$db_lt> = ($($field_ty,)*);
type $StructDataIdent<$db_lt> = ($($field_ty,)*);

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

impl<$db_lt, $($indexed_ty,)*> $zalsa::interned::HashEqLike<StructKey<$db_lt, $($indexed_ty),*>>
for StructData<$db_lt>
for $StructDataIdent<$db_lt>
where
$($field_ty: $zalsa::interned::HashEqLike<$indexed_ty>),*
{
Expand All @@ -90,24 +109,27 @@ macro_rules! setup_interned_struct {
}
}

impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<StructData<$db_lt>>
impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<$StructDataIdent<$db_lt>>
for StructKey<$db_lt, $($indexed_ty),*> {

#[allow(unused_unit)]
fn into_owned(self) -> StructData<$db_lt> {
fn into_owned(self) -> $StructDataIdent<$db_lt> {
($($zalsa::interned::Lookup::into_owned(self.$field_index),)*)
}
}

impl $zalsa_struct::Configuration for $Configuration {
impl salsa::plumbing::interned::Configuration for $StructWithStatic {
const DEBUG_NAME: &'static str = stringify!($Struct);
type Data<'a> = StructData<'a>;
type Struct<'a> = $Struct<'a>;
type Data<'a> = $StructDataIdent<'a>;
type Struct<'db> = $Struct< $($db_lt_arg)? >;
fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
$Struct(id, std::marker::PhantomData)
use salsa::plumbing::FromId;
$Struct(<$Id>::from_id(id), std::marker::PhantomData)
}

fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
s.0
use salsa::plumbing::AsId;
s.0.as_id()
}
}

Expand All @@ -124,37 +146,37 @@ macro_rules! setup_interned_struct {
}
}

impl $zalsa::AsId for $Struct<'_> {
impl< $($db_lt_arg)? > $zalsa::AsId for $Struct< $($db_lt_arg)? > {
fn as_id(&self) -> salsa::Id {
self.0
self.0.as_id()
}
}

impl $zalsa::FromId for $Struct<'_> {
impl< $($db_lt_arg)? > $zalsa::FromId for $Struct< $($db_lt_arg)? > {
fn from_id(id: salsa::Id) -> Self {
Self(id, std::marker::PhantomData)
Self(<$Id>::from_id(id), std::marker::PhantomData)
}
}

unsafe impl Send for $Struct<'_> {}
unsafe impl< $($db_lt_arg)? > Send for $Struct< $($db_lt_arg)? > {}

unsafe impl Sync for $Struct<'_> {}
unsafe impl< $($db_lt_arg)? > Sync for $Struct< $($db_lt_arg)? > {}

$zalsa::macro_if! { $generate_debug_impl =>
impl std::fmt::Debug for $Struct<'_> {
impl< $($db_lt_arg)? > std::fmt::Debug for $Struct< $($db_lt_arg)? > {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Self::default_debug_fmt(*self, f)
}
}
}

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

unsafe impl $zalsa::Update for $Struct<'_> {
unsafe impl< $($db_lt_arg)? > $zalsa::Update for $Struct< $($db_lt_arg)? > {
unsafe fn maybe_update(old_pointer: *mut Self, new_value: Self) -> bool {
if unsafe { *old_pointer } != new_value {
unsafe { *old_pointer = new_value };
Expand All @@ -165,7 +187,7 @@ macro_rules! setup_interned_struct {
}
}

impl<$db_lt> $Struct<$db_lt> {
impl<$db_lt> $Struct< $($db_lt_arg)? > {
pub fn $new_fn<$Db, $($indexed_ty: $zalsa::interned::Lookup<$field_ty> + std::hash::Hash,)*>(db: &$db_lt $Db, $($field_id: $indexed_ty),*) -> Self
where
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
Expand Down
2 changes: 2 additions & 0 deletions components/salsa-macros/src/accumulator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ impl AllowedOptions for Accumulator {
const NO_EQ: bool = false;
const NO_DEBUG: bool = true;
const NO_CLONE: bool = true;
const NO_LIFETIME: bool = false;
const SINGLETON: bool = false;
const DATA: bool = false;
const DB: bool = false;
const RECOVERY_FN: bool = false;
const LRU: bool = false;
const CONSTRUCTOR_NAME: bool = false;
const ID: bool = false;
}

struct StructMacro {
Expand Down
6 changes: 6 additions & 0 deletions components/salsa-macros/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ impl crate::options::AllowedOptions for InputStruct {

const NO_DEBUG: bool = true;

const NO_LIFETIME: bool = false;

const NO_CLONE: bool = false;

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

const CONSTRUCTOR_NAME: bool = true;

const ID: bool = false;
}

impl SalsaStructAllowedOptions for InputStruct {
Expand All @@ -64,6 +68,8 @@ impl SalsaStructAllowedOptions for InputStruct {

const HAS_LIFETIME: bool = false;

const ELIDABLE_LIFETIME: bool = false;

const ALLOW_DEFAULT: bool = true;
}

Expand Down
30 changes: 30 additions & 0 deletions components/salsa-macros/src/interned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ impl crate::options::AllowedOptions for InternedStruct {

const NO_DEBUG: bool = true;

const NO_LIFETIME: bool = true;

const NO_CLONE: bool = false;

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

const CONSTRUCTOR_NAME: bool = true;

const ID: bool = true;
}

impl SalsaStructAllowedOptions for InternedStruct {
Expand All @@ -65,6 +69,8 @@ impl SalsaStructAllowedOptions for InternedStruct {

const HAS_LIFETIME: bool = true;

const ELIDABLE_LIFETIME: bool = true;

const ALLOW_DEFAULT: bool = false;
}

Expand All @@ -82,6 +88,7 @@ impl Macro {
let attrs = &self.struct_item.attrs;
let vis = &self.struct_item.vis;
let struct_ident = &self.struct_item.ident;
let struct_data_ident = format_ident!("{}Data", struct_ident);
let db_lt = db_lifetime::db_lifetime(&self.struct_item.generics);
let new_fn = salsa_struct.constructor_name();
let field_ids = salsa_struct.field_ids();
Expand All @@ -93,6 +100,24 @@ impl Macro {
let field_tys = salsa_struct.field_tys();
let field_indexed_tys = salsa_struct.field_indexed_tys();
let generate_debug_impl = salsa_struct.generate_debug_impl();
let has_lifetime = salsa_struct.generate_lifetime();
let id = salsa_struct.id();

let (db_lt_arg, cfg, interior_lt) = if has_lifetime {
(
Some(db_lt.clone()),
quote!(#struct_ident<'static>),
db_lt.clone(),
)
} else {
let span = syn::spanned::Spanned::span(&self.struct_item.generics);
let static_lifetime = syn::Lifetime {
apostrophe: span,
ident: syn::Ident::new("static", span),
};

(None, quote!(#struct_ident), static_lifetime)
};

let zalsa = self.hygiene.ident("zalsa");
let zalsa_struct = self.hygiene.ident("zalsa_struct");
Expand All @@ -107,7 +132,12 @@ impl Macro {
attrs: [#(#attrs),*],
vis: #vis,
Struct: #struct_ident,
StructData: #struct_data_ident,
StructWithStatic: #cfg,
db_lt: #db_lt,
db_lt_arg: #db_lt_arg,
id: #id,
interior_lt: #interior_lt,
new_fn: #new_fn,
field_options: [#(#field_options),*],
field_ids: [#(#field_ids),*],
Expand Down
41 changes: 41 additions & 0 deletions components/salsa-macros/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use syn::{ext::IdentExt, spanned::Spanned};
/// are required and trailing commas are permitted. The options accepted
/// for any particular location are configured via the `AllowedOptions`
/// trait.
#[derive(Debug)]
pub(crate) struct Options<A: AllowedOptions> {
/// The `return_ref` option is used to signal that field/return type is "by ref"
///
Expand All @@ -24,6 +25,11 @@ pub(crate) struct Options<A: AllowedOptions> {
/// If this is `Some`, the value is the `no_debug` identifier.
pub no_debug: Option<syn::Ident>,

/// Signal we should not include the `'db` lifetime.
///
/// If this is `Some`, the value is the `no_lifetime` identifier.
pub no_lifetime: Option<syn::Ident>,

/// Signal we should not generate a `Clone` impl.
///
/// If this is `Some`, the value is the `no_clone` identifier.
Expand Down Expand Up @@ -66,6 +72,12 @@ pub(crate) struct Options<A: AllowedOptions> {
/// If this is `Some`, the value is the `<ident>`.
pub constructor_name: Option<syn::Ident>,

/// The `id = <path>` option is used to set a custom ID for interrned structs.
///
/// The ID must implement `salsa::plumbing::AsId` and `salsa::plumbing::FromId`.
/// If this is `Some`, the value is the `<ident>`.
pub id: Option<syn::Path>,

/// Remember the `A` parameter, which plays no role after parsing.
phantom: PhantomData<A>,
}
Expand All @@ -77,6 +89,7 @@ impl<A: AllowedOptions> Default for Options<A> {
specify: Default::default(),
no_eq: Default::default(),
no_debug: Default::default(),
no_lifetime: Default::default(),
no_clone: Default::default(),
db_path: Default::default(),
recovery_fn: Default::default(),
Expand All @@ -85,6 +98,7 @@ impl<A: AllowedOptions> Default for Options<A> {
phantom: Default::default(),
lru: Default::default(),
singleton: Default::default(),
id: Default::default(),
}
}
}
Expand All @@ -95,13 +109,15 @@ pub(crate) trait AllowedOptions {
const SPECIFY: bool;
const NO_EQ: bool;
const NO_DEBUG: bool;
const NO_LIFETIME: bool;
const NO_CLONE: bool;
const SINGLETON: bool;
const DATA: bool;
const DB: bool;
const RECOVERY_FN: bool;
const LRU: bool;
const CONSTRUCTOR_NAME: bool;
const ID: bool;
}

type Equals = syn::Token![=];
Expand Down Expand Up @@ -152,6 +168,20 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
"`no_debug` option not allowed here",
));
}
} else if ident == "no_lifetime" {
if A::NO_LIFETIME {
if let Some(old) = std::mem::replace(&mut options.no_lifetime, Some(ident)) {
return Err(syn::Error::new(
old.span(),
"option `no_lifetime` provided twice",
));
}
} else {
return Err(syn::Error::new(
ident.span(),
"`no_lifetime` option not allowed here",
));
}
} else if ident == "no_clone" {
if A::NO_CLONE {
if let Some(old) = std::mem::replace(&mut options.no_clone, Some(ident)) {
Expand Down Expand Up @@ -267,6 +297,17 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
"`constructor` option not allowed here",
));
}
} else if ident == "id" {
if A::ID {
let _eq = Equals::parse(input)?;
let path = syn::Path::parse(input)?;
options.id = Some(path);
} else {
return Err(syn::Error::new(
ident.span(),
"`id` option not allowed here",
));
}
} else {
return Err(syn::Error::new(
ident.span(),
Expand Down
Loading
Loading