Skip to content

Commit

Permalink
feature: allow calling ingredient on some interned structs
Browse files Browse the repository at this point in the history
  • Loading branch information
davidbarsky committed Dec 18, 2024
1 parent 42d556f commit 6d23f3b
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ macro_rules! setup_interned_struct_sans_lifetime {
// 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 addition dependency is unnecessary.
StructData: $StructDataIdent:ident,

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

Expand Down Expand Up @@ -62,52 +66,52 @@ macro_rules! setup_interned_struct_sans_lifetime {
std::marker::PhantomData < &'static salsa::plumbing::interned::Value < $Struct > >
);

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

impl salsa::plumbing::interned::Configuration for $Struct {
const DEBUG_NAME: &'static str = stringify!($Struct);
type Data<'a> = $StructDataIdent<'a>;
type Struct<'a> = $Struct;
fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
use salsa::plumbing::FromId;
$Struct(<$Id>::from_id(id), std::marker::PhantomData)
}
fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
use salsa::plumbing::AsId;
s.0.as_id()
}
}

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

type $Configuration = $Struct;

type StructData<$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.
struct StructKey<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*>(
$($indexed_ty,)*
std::marker::PhantomData<&$db_lt ()>,
);

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),*> {

fn hash<H: std::hash::Hasher>(&self, h: &mut H) {
$($zalsa::interned::Lookup::hash(&self.$field_index, &mut *h);)*
}

fn eq(&self, data: &StructData<$db_lt>) -> bool {
fn eq(&self, data: &$StructDataIdent<$db_lt>) -> bool {
($($zalsa::interned::Lookup::eq(&self.$field_index, &data.$field_index) && )* true)
}

#[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 {
const DEBUG_NAME: &'static str = stringify!($Struct);
type Data<'a> = StructData<'a>;
type Struct<'a> = $Struct;
fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
use $zalsa::FromId;
$Struct(<$Id>::from_id(id), std::marker::PhantomData)
}
fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
use $zalsa::AsId;
s.0.as_id()
}
}

impl $Configuration {
pub fn ingredient<Db>(db: &Db) -> &$zalsa_struct::IngredientImpl<Self>
where
Expand Down Expand Up @@ -146,6 +150,9 @@ macro_rules! setup_interned_struct_sans_lifetime {
}

impl $zalsa::SalsaStructInDb for $Struct {
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 {
Expand Down
2 changes: 2 additions & 0 deletions components/salsa-macros/src/interned_sans_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,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 @@ -111,6 +112,7 @@ impl Macro {
attrs: [#(#attrs),*],
vis: #vis,
Struct: #struct_ident,
StructData: #struct_data_ident,
db_lt: #db_lt,
id: #id,
new_fn: #new_fn,
Expand Down
44 changes: 44 additions & 0 deletions components/salsa-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,50 @@ pub fn interned(args: TokenStream, input: TokenStream) -> TokenStream {
interned::interned(args, input)
}

/// A discouraged variant of `#[salsa::interned]`.
///
/// `#[salsa::interned_sans_lifetime]` is intended to be used in codebases that are migrating from
/// the original Salsa to the current version of Salsa. New codebases that are just starting to use
/// Salsa should avoid using this macro and prefer `#[salsa::interned]` instead.
///
/// `#[salsa::interned_sans_lifetime]` differs from `#[salsa::interned]` in a two key ways:
/// 1. As the name suggests, it removes the `'db` lifetime from the interned struct. This lifetime is
/// designed to meant to certain values as "salsa structs", but it also adds the desirable property
/// of misuse resistance: it is difficult to embed an `#[salsa::interned]` struct into an auxiliary
/// structures or collections collection, which can lead to subtle invalidation bugs. However, old
/// Salsa encouraged storing keys to interned values in auxiliary structures and collections, so
/// so converting all usage to Salsa's current API guidelines might not be desirable or feasible.
/// 2. `#[salsa::interned_sans_lifetime]` requires specifiying the ID. In most cases, `salsa::Id`
/// is sufficent, but in rare, performance-sensitive circumstances, it might be desireable to
/// set the Id to a type that implements `salsa::plumbing::AsId` and `salsa::plumbing::FromId`.
///
/// ## Example
///
/// Below is an example of a struct using `#[salsa::interned_sans_lifetime]` with a custom Id:
///
/// ```no_compile
/// use salsa::plumbing::{AsId, FromId};
///
/// #[derive(Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)]
/// struct CustomSalsaIdWrapper(salsa::Id);
///
/// impl AsId for CustomSalsaIdWrapper {
/// fn as_id(&self) -> salsa::Id {
/// self.0
/// }
/// }
///
/// impl FromId for CustomSalsaIdWrapper {
/// fn from_id(id: salsa::Id) -> Self {
/// CustomSalsaIdWrapper(id)
/// }
/// }
///
/// #[salsa::interned_sans_lifetime(id = CustomSalsaIdWrapper)]
/// struct InternedString {
/// data: String,
/// }
/// ```
#[proc_macro_attribute]
pub fn interned_sans_lifetime(args: TokenStream, input: TokenStream) -> TokenStream {
interned_sans_lifetime::interned_sans_lifetime(args, input)
Expand Down
2 changes: 1 addition & 1 deletion src/table/memo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{zalsa::MemoIngredientIndex, zalsa_local::QueryOrigin};
/// Every tracked function must take a salsa struct as its first argument
/// and memo tables are attached to those salsa structs as auxiliary data.
#[derive(Default)]
pub(crate) struct MemoTable {
pub struct MemoTable {
memos: RwLock<Vec<MemoEntry>>,
}

Expand Down
2 changes: 1 addition & 1 deletion src/table/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::util;
/// Tracks the keys that are currently being processed; used to coordinate between
/// worker threads.
#[derive(Default)]
pub(crate) struct SyncTable {
pub struct SyncTable {
syncs: RwLock<Vec<Option<SyncState>>>,
}

Expand Down
37 changes: 34 additions & 3 deletions tests/interned-sans-lifetime.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
use expect_test::expect;
use salsa::plumbing::{AsId, FromId};
use std::path::{Path, PathBuf};
use test_log::test;

#[derive(Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)]
struct CustomSalsaIdWrapper(salsa::Id);

impl salsa::plumbing::AsId for CustomSalsaIdWrapper {
impl AsId for CustomSalsaIdWrapper {
fn as_id(&self) -> salsa::Id {
self.0
}
}

impl salsa::plumbing::FromId for CustomSalsaIdWrapper {
impl FromId for CustomSalsaIdWrapper {
fn from_id(id: salsa::Id) -> Self {
CustomSalsaIdWrapper(id)
}
Expand Down Expand Up @@ -45,7 +46,7 @@ struct InternedPathBuf {

#[salsa::tracked]
fn intern_stuff(db: &dyn salsa::Database) -> String {
let s1 = InternedString::new(db, "Hello, ".to_string());
let s1 = InternedString::new(db, "Hello, ");
let s2 = InternedString::new(db, "World, ");
let s3 = InternedPair::new(db, (s1, s2));

Expand All @@ -70,6 +71,7 @@ fn interning_returns_equal_keys_for_equal_data() {
assert_eq!(s1, s1_2);
assert_eq!(s2, s2_2);
}

#[test]
fn interning_returns_equal_keys_for_equal_data_multi_field() {
let db = salsa::DatabaseImpl::new();
Expand Down Expand Up @@ -127,3 +129,32 @@ fn tracked_static_query_works() {
let s1 = InternedString::new(&db, "Hello, World!".to_string());
assert_eq!(length(&db, s1), 13);
}

#[test]
fn public_ingredient() {
let db = salsa::DatabaseImpl::new();
let s = InternedString::new(&db, String::from("Hello, world!"));
let underlying_id = s.0;

let data = InternedString::ingredient(&db).data(&db, underlying_id.as_id());
assert_eq!(data, &(String::from("Hello, world!"),));
}

#[salsa::tracked]
fn intern_more_stuff(db: &dyn salsa::Database) -> (InternedString, InternedString, InternedPair) {
let s1 = InternedString::new(db, "Hello, ");
let s2 = InternedString::new(db, "World, ");
let pair = InternedPair::new(db, (s1, s2));
(s1, s2, pair)
}

#[test]
fn public_ingredients() {
let db = salsa::DatabaseImpl::new();

let (_, _, pair) = intern_more_stuff(&db);
let (interned_s1, interned_s2) = InternedPair::ingredient(&db).fields(&db, pair).0;

assert_eq!(interned_s1.data(&db), "Hello, ".to_owned());
assert_eq!(interned_s2.data(&db), "World, ".to_owned());
}

0 comments on commit 6d23f3b

Please sign in to comment.