From 1e1833467260ce7538c8540d51805a545cfb5f20 Mon Sep 17 00:00:00 2001
From: puuuuh <54703480+puuuuh@users.noreply.github.com>
Date: Fri, 4 Oct 2024 07:41:20 +0300
Subject: [PATCH 1/9] Add way to intern structs from references

---
 Cargo.toml                                    |  2 +-
 .../src/setup_interned_struct.rs              | 32 ++++++-
 components/salsa-macros/src/interned.rs       |  2 +
 components/salsa-macros/src/salsa_struct.rs   |  8 ++
 src/interned.rs                               | 83 +++++++++++++++++--
 src/lib.rs                                    |  1 +
 tests/interned-struct-with-lifetime.rs        | 31 ++++++-
 7 files changed, 148 insertions(+), 11 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index d3815b1..8a8e906 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -10,7 +10,7 @@ description = "A generic framework for on-demand, incrementalized computation (e
 [dependencies]
 arc-swap = "1"
 crossbeam = "0.8"
-dashmap = "6"
+dashmap = { version = "6", features = ["raw-api"] }
 hashlink = "0.9"
 indexmap = "2"
 append-only-vec = "0.1.5"
diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs
index 2b7f8d8..a5e21e0 100644
--- a/components/salsa-macro-rules/src/setup_interned_struct.rs
+++ b/components/salsa-macro-rules/src/setup_interned_struct.rs
@@ -32,6 +32,9 @@ macro_rules! setup_interned_struct {
         // Indices for each field from 0..N -- must be unsuffixed (e.g., `0`, `1`).
         field_indices: [$($field_index:tt),*],
 
+        // Indexed types for each field (T0, T1, ...)
+        field_indexed_tys: [$($indexed_ty:ident),*],
+
         // Number of fields
         num_fields: $N:literal,
 
@@ -62,10 +65,30 @@ macro_rules! setup_interned_struct {
 
             type $Configuration = $Struct<'static>;
 
+            type StructData<$db_lt> = ($($field_ty,)*);
+
+            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>>
+                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 {
+                    ($($zalsa::interned::Lookup::eq(&self.$field_index, &data.$field_index) && )* true)
+                }
+
+                fn into_owned(self) -> StructData<$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<$db_lt> = ($($field_ty,)*);
-                type Struct<$db_lt> = $Struct<$db_lt>;
+                type Data<'a> = StructData<'a>;
+                type Struct<'a> = $Struct<'a>;
                 fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
                     $Struct(id, std::marker::PhantomData)
                 }
@@ -126,13 +149,14 @@ macro_rules! setup_interned_struct {
             }
 
             impl<$db_lt> $Struct<$db_lt> {
-                pub fn $new_fn<$Db>(db: &$db_lt $Db, $($field_id: $field_ty),*) -> Self
+                pub fn $new_fn<$Db>(db: &$db_lt $Db,  $($field_id: impl $zalsa::interned::Lookup<$field_ty>),*) -> Self
                 where
                     // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
                     $Db: ?Sized + salsa::Database,
                 {
                     let current_revision = $zalsa::current_revision(db);
-                    $Configuration::ingredient(db).intern(db.as_dyn_database(), ($($field_id,)*))
+                    $Configuration::ingredient(db).intern(db.as_dyn_database(),
+                        StructKey::<$db_lt>($($field_id,)* std::marker::PhantomData::default()))
                 }
 
                 $(
diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs
index b0fdb32..aea9a4e 100644
--- a/components/salsa-macros/src/interned.rs
+++ b/components/salsa-macros/src/interned.rs
@@ -90,6 +90,7 @@ impl Macro {
         let field_getter_ids = salsa_struct.field_getter_ids();
         let field_options = salsa_struct.field_options();
         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 zalsa = self.hygiene.ident("zalsa");
@@ -112,6 +113,7 @@ impl Macro {
                     field_getters: [#(#field_vis #field_getter_ids),*],
                     field_tys: [#(#field_tys),*],
                     field_indices: [#(#field_indices),*],
+                    field_indexed_tys: [#(#field_indexed_tys),*],
                     num_fields: #num_fields,
                     generate_debug_impl: #generate_debug_impl,
                     unused_names: [
diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs
index 5480824..b93470a 100644
--- a/components/salsa-macros/src/salsa_struct.rs
+++ b/components/salsa-macros/src/salsa_struct.rs
@@ -242,6 +242,14 @@ where
         self.fields.iter().map(|f| &f.field.ty).collect()
     }
 
+    pub(crate) fn field_indexed_tys(&self) -> Vec<syn::Ident> {
+        self.fields
+            .iter()
+            .enumerate()
+            .map(|(i, _)| quote::format_ident!("T{i}"))
+            .collect()
+    }
+
     pub(crate) fn field_options(&self) -> Vec<TokenStream> {
         self.fields
             .iter()
diff --git a/src/interned.rs b/src/interned.rs
index 2003520..2b2442c 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -1,5 +1,5 @@
 use std::fmt;
-use std::hash::Hash;
+use std::hash::{BuildHasher, Hash, Hasher};
 use std::marker::PhantomData;
 
 use crate::durability::Durability;
@@ -114,19 +114,19 @@ where
         unsafe { std::mem::transmute(data) }
     }
 
-    pub fn intern_id<'db>(
+    pub fn intern_id<'db, T: Lookup<C::Data<'db>>>(
         &'db self,
         db: &'db dyn crate::Database,
-        data: C::Data<'db>,
+        data: T,
     ) -> crate::Id {
         C::deref_struct(self.intern(db, data)).as_id()
     }
 
     /// Intern data to a unique reference.
-    pub fn intern<'db>(
+    pub fn intern<'db, T: Lookup<C::Data<'db>>>(
         &'db self,
         db: &'db dyn crate::Database,
-        data: C::Data<'db>,
+        data: T,
     ) -> C::Struct<'db> {
         let zalsa_local = db.zalsa_local();
         zalsa_local.report_tracked_read(
@@ -137,7 +137,25 @@ where
 
         // Optimisation to only get read lock on the map if the data has already
         // been interned.
+        let mut hasher = self.key_map.hasher().build_hasher();
+        data.hash(&mut hasher);
+        let data_hash = hasher.finish();
+        let shard = self.key_map.determine_shard(data_hash as _);
+        {
+            let lock = self.key_map.shards()[shard].read();
+            if let Some(bucket) = lock.find(data_hash, |(a, _)| {
+                // lifetime shrink
+                let a: &C::Data<'db> = unsafe { std::mem::transmute(a) };
+                Lookup::eq(&data, a)
+            }) {
+                return C::struct_from_id(unsafe { *bucket.as_ref().1.get() });
+            }
+        };
+
+        let data = data.into_owned();
+
         let internal_data = unsafe { self.to_internal_data(data) };
+
         if let Some(guard) = self.key_map.get(&internal_data) {
             let id = *guard;
             drop(guard);
@@ -288,3 +306,58 @@ where
         &self.syncs
     }
 }
+
+pub trait Lookup<O>
+{
+    fn hash<H: Hasher>(&self, h: &mut H);
+    fn eq(&self, data: &O) -> bool;
+    fn into_owned(self) -> O;
+}
+
+impl<T> Lookup<T> for T
+where
+    T: Hash + Eq
+{
+    fn hash<H: Hasher>(&self, h: &mut H) {
+        Hash::hash(self, &mut *h);
+    }
+
+    fn eq(&self, data: &T) -> bool {
+        self == data
+    }
+
+    fn into_owned(self) -> T {
+        self
+    }
+}
+
+impl<T> Lookup<T> for &T
+where
+    T: Clone + Eq + Hash,
+{
+    fn hash<H: Hasher>(&self, h: &mut H) {
+        Hash::hash(self, &mut *h);
+    }
+
+    fn eq(&self, data: &T) -> bool {
+        *self == data
+    }
+
+    fn into_owned(self) -> T {
+        Clone::clone(self)
+    }
+}
+
+impl Lookup<String> for &str {
+    fn hash<H: Hasher>(&self, h: &mut H) {
+        Hash::hash(self, &mut *h)
+    }
+
+    fn eq(&self, data: &String) -> bool {
+        self == data
+    }
+
+    fn into_owned(self) -> String {
+        self.to_owned()
+    }
+}
\ No newline at end of file
diff --git a/src/lib.rs b/src/lib.rs
index c23d9c2..d3befc2 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -133,6 +133,7 @@ pub mod plumbing {
         pub use crate::interned::IngredientImpl;
         pub use crate::interned::JarImpl;
         pub use crate::interned::Value;
+        pub use crate::interned::Lookup;
     }
 
     pub mod function {
diff --git a/tests/interned-struct-with-lifetime.rs b/tests/interned-struct-with-lifetime.rs
index a74d2c4..8c60292 100644
--- a/tests/interned-struct-with-lifetime.rs
+++ b/tests/interned-struct-with-lifetime.rs
@@ -9,16 +9,24 @@ struct InternedString<'db> {
     data: String,
 }
 
+
 #[salsa::interned]
 struct InternedPair<'db> {
     data: (InternedString<'db>, InternedString<'db>),
 }
 
+#[salsa::interned]
+struct InternedTwoFields<'db> {
+    data1: String,
+    data2: String,
+}
+
 #[salsa::tracked]
 fn intern_stuff(db: &dyn salsa::Database) -> String {
     let s1 = InternedString::new(db, "Hello, ".to_string());
-    let s2 = InternedString::new(db, "World, ".to_string());
+    let s2 = InternedString::new(db, "World, ");
     let s3 = InternedPair::new(db, (s1, s2));
+
     format!("{s3:?}")
 }
 
@@ -29,3 +37,24 @@ fn execute() {
         "InternedPair { data: (InternedString { data: \"Hello, \" }, InternedString { data: \"World, \" }) }"
     "#]].assert_debug_eq(&intern_stuff(&db));
 }
+
+#[test]
+fn interning_returns_equal_keys_for_equal_data() {
+    let db = salsa::DatabaseImpl::new();
+    let s1 = InternedString::new(&db, "Hello, ".to_string());
+    let s2 = InternedString::new(&db, "World, ".to_string());
+    let s1_2 = InternedString::new(&db, "Hello, ");
+    let s2_2 = InternedString::new(&db, "World, ");
+    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();
+    let s1 = InternedTwoFields::new(&db, "Hello, ".to_string(), "World");
+    let s2 = InternedTwoFields::new(&db, "World, ", "Hello".to_string());
+    let s1_2 = InternedTwoFields::new(&db, "Hello, ", "World");
+    let s2_2 = InternedTwoFields::new(&db, "World, ", "Hello");
+    assert_eq!(s1, s1_2);
+    assert_eq!(s2, s2_2);
+}

From dcdf4d774d595764fa1a77afec01e820607228cf Mon Sep 17 00:00:00 2001
From: puuuuh <54703480+puuuuh@users.noreply.github.com>
Date: Fri, 4 Oct 2024 13:17:15 +0300
Subject: [PATCH 2/9] Update
 components/salsa-macro-rules/src/setup_interned_struct.rs

Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
---
 components/salsa-macro-rules/src/setup_interned_struct.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs
index a5e21e0..4f52119 100644
--- a/components/salsa-macro-rules/src/setup_interned_struct.rs
+++ b/components/salsa-macro-rules/src/setup_interned_struct.rs
@@ -67,7 +67,12 @@ macro_rules! setup_interned_struct {
 
             type StructData<$db_lt> = ($($field_ty,)*);
 
-            struct StructKey<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*>($($indexed_ty,)* std::marker::PhantomData<&$db_lt ()>,);
+            /// 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>>
                 for StructKey<$db_lt, $($indexed_ty),*> {

From 60b921f153f8e35c4cc6d8490b4bb55d75338e0b Mon Sep 17 00:00:00 2001
From: puuuuh <54703480+puuuuh@users.noreply.github.com>
Date: Fri, 4 Oct 2024 13:17:45 +0300
Subject: [PATCH 3/9] Update src/interned.rs

Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
---
 src/interned.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/interned.rs b/src/interned.rs
index 2b2442c..a15808e 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -137,9 +137,12 @@ where
 
         // Optimisation to only get read lock on the map if the data has already
         // been interned.
-        let mut hasher = self.key_map.hasher().build_hasher();
-        data.hash(&mut hasher);
-        let data_hash = hasher.finish();
+        // We need to use the raw API for this lookup. See the [`Lookup`][] trait definition for an explanation of why.
+        let data_hash = {
+            let mut hasher = self.key_map.hasher().build_hasher();
+            data.hash(&mut hasher);
+            hasher.finish()
+        };
         let shard = self.key_map.determine_shard(data_hash as _);
         {
             let lock = self.key_map.shards()[shard].read();

From 22eafc213e9de5987f75bc5df8c06dd19c08dbd5 Mon Sep 17 00:00:00 2001
From: puuuuh <54703480+puuuuh@users.noreply.github.com>
Date: Fri, 4 Oct 2024 13:33:26 +0300
Subject: [PATCH 4/9] Apply suggestions from code review

Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
---
 src/interned.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/interned.rs b/src/interned.rs
index a15808e..ff828de 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -151,6 +151,7 @@ where
                 let a: &C::Data<'db> = unsafe { std::mem::transmute(a) };
                 Lookup::eq(&data, a)
             }) {
+                // SAFETY: Read lock on map is held during this block
                 return C::struct_from_id(unsafe { *bucket.as_ref().1.get() });
             }
         };
@@ -310,6 +311,19 @@ where
     }
 }
 
+/// The `Lookup` trait is a more flexible variant on [`std::borrow::Borrow`]
+/// and [`std::borrow::ToOwned`]. It is implemented by "some type that can
+/// be used as the lookup key for `O`". This means that `self` 
+/// can be hashed and compared for equality with values of type `O`
+/// without actually creating an owned value. It `self` needs to be interned,
+/// it can be converted into an equivalent value of type `O`.
+///
+/// The canonical example is `&str: Lookup<String>`. However, this example
+/// alone can be handled by [`std::borrow::Borrow`][]. In our case, we may have
+/// multiple keys accumulated into a struct, like `ViewStruct: Lookup<(K1, ...)>`,
+/// where `struct ViewStruct<L1: Lookup<K1>...>(K1...)`. The `Borrow` trait
+/// requires that `&(K1...)` be convertible to `&ViewStruct` which just isn't
+/// possible. `Lookup` instead offers direct `hash` and `eq` methods.
 pub trait Lookup<O>
 {
     fn hash<H: Hasher>(&self, h: &mut H);

From 552b3ae9792f9c40203067a5eb6d67743f96b584 Mon Sep 17 00:00:00 2001
From: puuuuh <54703480+puuuuh@users.noreply.github.com>
Date: Fri, 4 Oct 2024 14:32:10 +0300
Subject: [PATCH 5/9] Add impl Lookup for Path, Vec

---
 src/interned.rs | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/interned.rs b/src/interned.rs
index ff828de..2f3b891 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -1,7 +1,7 @@
 use std::fmt;
 use std::hash::{BuildHasher, Hash, Hasher};
 use std::marker::PhantomData;
-
+use std::path::{Path, PathBuf};
 use crate::durability::Durability;
 use crate::id::AsId;
 use crate::ingredient::fmt_index;
@@ -147,7 +147,8 @@ where
         {
             let lock = self.key_map.shards()[shard].read();
             if let Some(bucket) = lock.find(data_hash, |(a, _)| {
-                // lifetime shrink
+                // SAFETY: it's safe to go from Data<'static> to Data<'db>
+                // shrink lifetime here to use a single lifetime in Lookup::eq(&StructKey<'db>, &C::Data<'db>)
                 let a: &C::Data<'db> = unsafe { std::mem::transmute(a) };
                 Lookup::eq(&data, a)
             }) {
@@ -160,12 +161,6 @@ where
 
         let internal_data = unsafe { self.to_internal_data(data) };
 
-        if let Some(guard) = self.key_map.get(&internal_data) {
-            let id = *guard;
-            drop(guard);
-            return C::struct_from_id(id);
-        }
-
         match self.key_map.entry(internal_data.clone()) {
             // Data has been interned by a racing call, use that ID instead
             dashmap::mapref::entry::Entry::Occupied(entry) => {
@@ -377,4 +372,35 @@ impl Lookup<String> for &str {
     fn into_owned(self) -> String {
         self.to_owned()
     }
+}
+
+impl<A: Hash + Eq + PartialEq<T> + Clone + Lookup<T>, T> Lookup<Vec<T>> for &[A] {
+    fn hash<H: Hasher>(&self, h: &mut H) {
+        for a in *self {
+            Hash::hash(a, h);
+        }
+    }
+
+    fn eq(&self, data: &Vec<T>) -> bool {
+        self.len() == data.len() &&
+            data.iter().enumerate().all(|(i, a)| &self[i] == a)
+    }
+
+    fn into_owned(self) -> Vec<T> {
+        self.into_iter().map(|a| Lookup::into_owned(a.clone())).collect()
+    }
+}
+
+impl Lookup<PathBuf> for &Path {
+    fn hash<H: Hasher>(&self, h: &mut H) {
+        Hash::hash(self, h);
+    }
+
+    fn eq(&self, data: &PathBuf) -> bool {
+        self == data
+    }
+
+    fn into_owned(self) -> PathBuf {
+        self.to_owned()
+    }
 }
\ No newline at end of file

From e9ee280c10763bd4acdc2f6ef011a962568eaea7 Mon Sep 17 00:00:00 2001
From: puuuuh <54703480+puuuuh@users.noreply.github.com>
Date: Fri, 4 Oct 2024 14:37:40 +0300
Subject: [PATCH 6/9] Add impl for Lookup Vec via [A; N]

---
 src/interned.rs                        | 17 ++++++++++
 tests/interned-struct-with-lifetime.rs | 46 ++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/src/interned.rs b/src/interned.rs
index 2f3b891..b6f4579 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -391,6 +391,23 @@ impl<A: Hash + Eq + PartialEq<T> + Clone + Lookup<T>, T> Lookup<Vec<T>> for &[A]
     }
 }
 
+impl<const N: usize, A: Hash + Eq + PartialEq<T> + Clone + Lookup<T>, T> Lookup<Vec<T>> for [A; N] {
+    fn hash<H: Hasher>(&self, h: &mut H) {
+        for a in self {
+            Hash::hash(a, h);
+        }
+    }
+
+    fn eq(&self, data: &Vec<T>) -> bool {
+        self.len() == data.len() &&
+            data.iter().enumerate().all(|(i, a)| &self[i] == a)
+    }
+
+    fn into_owned(self) -> Vec<T> {
+        self.into_iter().map(|a| Lookup::into_owned(a.clone())).collect()
+    }
+}
+
 impl Lookup<PathBuf> for &Path {
     fn hash<H: Hasher>(&self, h: &mut H) {
         Hash::hash(self, h);
diff --git a/tests/interned-struct-with-lifetime.rs b/tests/interned-struct-with-lifetime.rs
index 8c60292..aa2306e 100644
--- a/tests/interned-struct-with-lifetime.rs
+++ b/tests/interned-struct-with-lifetime.rs
@@ -1,6 +1,7 @@
 //! Test that a `tracked` fn on a `salsa::input`
 //! compiles and executes successfully.
 
+use std::path::{Path, PathBuf};
 use expect_test::expect;
 use test_log::test;
 
@@ -21,6 +22,16 @@ struct InternedTwoFields<'db> {
     data2: String,
 }
 
+#[salsa::interned]
+struct InternedVec<'db> {
+    data1: Vec<String>,
+}
+
+#[salsa::interned]
+struct InternedPathBuf<'db> {
+    data1: PathBuf,
+}
+
 #[salsa::tracked]
 fn intern_stuff(db: &dyn salsa::Database) -> String {
     let s1 = InternedString::new(db, "Hello, ".to_string());
@@ -55,6 +66,41 @@ fn interning_returns_equal_keys_for_equal_data_multi_field() {
     let s2 = InternedTwoFields::new(&db, "World, ", "Hello".to_string());
     let s1_2 = InternedTwoFields::new(&db, "Hello, ", "World");
     let s2_2 = InternedTwoFields::new(&db, "World, ", "Hello");
+    let new = InternedTwoFields::new(&db, "Hello, World", "");
+
     assert_eq!(s1, s1_2);
     assert_eq!(s2, s2_2);
+    assert_ne!(s1, s2_2);
+    assert_ne!(s1, new);
+}
+
+#[test]
+fn interning_vec() {
+    let db = salsa::DatabaseImpl::new();
+    let s1 = InternedVec::new(&db, ["Hello, ".to_string(), "World".to_string()].as_slice());
+    let s2 = InternedVec::new(&db, ["Hello, ", "World"].as_slice());
+    let s3 = InternedVec::new(&db, vec!["Hello, ".to_string(), "World".to_string()]);
+    let s4 = InternedVec::new(&db, ["Hello, ", "World"].as_slice());
+    let s5 = InternedVec::new(&db, ["Hello, ", "World", "Test"].as_slice());
+    let s6 = InternedVec::new(&db, ["Hello, ", "World", ""].as_slice());
+    let s7 = InternedVec::new(&db, ["Hello, "].as_slice());
+    assert_eq!(s1, s2);
+    assert_eq!(s1, s3);
+    assert_eq!(s1, s4);
+    assert_ne!(s1, s5);
+    assert_ne!(s1, s6);
+    assert_ne!(s5, s6);
+    assert_ne!(s6, s7);
+}
+
+#[test]
+fn interning_path_buf() {
+    let db = salsa::DatabaseImpl::new();
+    let s1 = InternedPathBuf::new(&db, PathBuf::from("test_path".to_string()));
+    let s2 = InternedPathBuf::new(&db, Path::new("test_path"));
+    let s3 = InternedPathBuf::new(&db, Path::new("test_path/"));
+    let s4 = InternedPathBuf::new(&db, Path::new("test_path/a"));
+    assert_eq!(s1, s2);
+    assert_eq!(s1, s3);
+    assert_ne!(s1, s4);
 }

From 514a31331b0135639483cdfa9a9900d6ec19e0e9 Mon Sep 17 00:00:00 2001
From: puuuuh <54703480+puuuuh@users.noreply.github.com>
Date: Fri, 4 Oct 2024 14:42:58 +0300
Subject: [PATCH 7/9] Apply suggestions from code review

Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
---
 src/interned.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/interned.rs b/src/interned.rs
index b6f4579..f276d05 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -117,7 +117,7 @@ where
     pub fn intern_id<'db, T: Lookup<C::Data<'db>>>(
         &'db self,
         db: &'db dyn crate::Database,
-        data: T,
+        data: impl Lookup<C::Data<'db>>,
     ) -> crate::Id {
         C::deref_struct(self.intern(db, data)).as_id()
     }
@@ -126,7 +126,7 @@ where
     pub fn intern<'db, T: Lookup<C::Data<'db>>>(
         &'db self,
         db: &'db dyn crate::Database,
-        data: T,
+        data: impl Lookup<C::Data<'db>>,
     ) -> C::Struct<'db> {
         let zalsa_local = db.zalsa_local();
         zalsa_local.report_tracked_read(

From 01d7aab603f1a613363f51dcba89da0d85da0942 Mon Sep 17 00:00:00 2001
From: puuuuh <puhovik@protonmail.com>
Date: Sun, 6 Oct 2024 20:05:04 +0300
Subject: [PATCH 8/9] Remove obsolete type argument

---
 src/interned.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/interned.rs b/src/interned.rs
index f276d05..a73ef9b 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -114,7 +114,7 @@ where
         unsafe { std::mem::transmute(data) }
     }
 
-    pub fn intern_id<'db, T: Lookup<C::Data<'db>>>(
+    pub fn intern_id<'db>(
         &'db self,
         db: &'db dyn crate::Database,
         data: impl Lookup<C::Data<'db>>,
@@ -123,7 +123,7 @@ where
     }
 
     /// Intern data to a unique reference.
-    pub fn intern<'db, T: Lookup<C::Data<'db>>>(
+    pub fn intern<'db>(
         &'db self,
         db: &'db dyn crate::Database,
         data: impl Lookup<C::Data<'db>>,

From de95f497ce3ed457f1784a7e3b4690542a3c2130 Mon Sep 17 00:00:00 2001
From: puuuuh <puhovik@protonmail.com>
Date: Thu, 10 Oct 2024 03:45:33 +0300
Subject: [PATCH 9/9] fix fmt

Fix clippy warnings
---
 .../src/setup_interned_struct.rs              |  1 +
 src/interned.rs                               | 36 +++++++++----------
 src/lib.rs                                    |  2 +-
 ...t-with-lifetime.rs => interned-structs.rs} |  3 +-
 4 files changed, 21 insertions(+), 21 deletions(-)
 rename tests/{interned-struct-with-lifetime.rs => interned-structs.rs} (99%)

diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs
index 4f52119..bf9d98f 100644
--- a/components/salsa-macro-rules/src/setup_interned_struct.rs
+++ b/components/salsa-macro-rules/src/setup_interned_struct.rs
@@ -85,6 +85,7 @@ macro_rules! setup_interned_struct {
                     ($($zalsa::interned::Lookup::eq(&self.$field_index, &data.$field_index) && )* true)
                 }
 
+                #[allow(unused_unit)]
                 fn into_owned(self) -> StructData<$db_lt> {
                     ($($zalsa::interned::Lookup::into_owned(self.$field_index),)*)
                 }
diff --git a/src/interned.rs b/src/interned.rs
index a73ef9b..94a6d89 100644
--- a/src/interned.rs
+++ b/src/interned.rs
@@ -1,7 +1,3 @@
-use std::fmt;
-use std::hash::{BuildHasher, Hash, Hasher};
-use std::marker::PhantomData;
-use std::path::{Path, PathBuf};
 use crate::durability::Durability;
 use crate::id::AsId;
 use crate::ingredient::fmt_index;
@@ -13,6 +9,10 @@ use crate::table::Slot;
 use crate::zalsa::IngredientIndex;
 use crate::zalsa_local::QueryOrigin;
 use crate::{Database, DatabaseKeyIndex, Id};
+use std::fmt;
+use std::hash::{BuildHasher, Hash, Hasher};
+use std::marker::PhantomData;
+use std::path::{Path, PathBuf};
 
 use super::hash::FxDashMap;
 use super::ingredient::Ingredient;
@@ -307,10 +307,11 @@ where
 }
 
 /// The `Lookup` trait is a more flexible variant on [`std::borrow::Borrow`]
-/// and [`std::borrow::ToOwned`]. It is implemented by "some type that can
-/// be used as the lookup key for `O`". This means that `self` 
-/// can be hashed and compared for equality with values of type `O`
-/// without actually creating an owned value. It `self` needs to be interned,
+/// and [`std::borrow::ToOwned`].
+///
+/// It is implemented by "some type that can be used as the lookup key for `O`".
+/// This means that `self` can be hashed and compared for equality with values
+/// of type `O` without actually creating an owned value. It `self` needs to be interned,
 /// it can be converted into an equivalent value of type `O`.
 ///
 /// The canonical example is `&str: Lookup<String>`. However, this example
@@ -319,8 +320,7 @@ where
 /// where `struct ViewStruct<L1: Lookup<K1>...>(K1...)`. The `Borrow` trait
 /// requires that `&(K1...)` be convertible to `&ViewStruct` which just isn't
 /// possible. `Lookup` instead offers direct `hash` and `eq` methods.
-pub trait Lookup<O>
-{
+pub trait Lookup<O> {
     fn hash<H: Hasher>(&self, h: &mut H);
     fn eq(&self, data: &O) -> bool;
     fn into_owned(self) -> O;
@@ -328,7 +328,7 @@ pub trait Lookup<O>
 
 impl<T> Lookup<T> for T
 where
-    T: Hash + Eq
+    T: Hash + Eq,
 {
     fn hash<H: Hasher>(&self, h: &mut H) {
         Hash::hash(self, &mut *h);
@@ -382,12 +382,11 @@ impl<A: Hash + Eq + PartialEq<T> + Clone + Lookup<T>, T> Lookup<Vec<T>> for &[A]
     }
 
     fn eq(&self, data: &Vec<T>) -> bool {
-        self.len() == data.len() &&
-            data.iter().enumerate().all(|(i, a)| &self[i] == a)
+        self.len() == data.len() && data.iter().enumerate().all(|(i, a)| &self[i] == a)
     }
 
     fn into_owned(self) -> Vec<T> {
-        self.into_iter().map(|a| Lookup::into_owned(a.clone())).collect()
+        self.iter().map(|a| Lookup::into_owned(a.clone())).collect()
     }
 }
 
@@ -399,12 +398,13 @@ impl<const N: usize, A: Hash + Eq + PartialEq<T> + Clone + Lookup<T>, T> Lookup<
     }
 
     fn eq(&self, data: &Vec<T>) -> bool {
-        self.len() == data.len() &&
-            data.iter().enumerate().all(|(i, a)| &self[i] == a)
+        self.len() == data.len() && data.iter().enumerate().all(|(i, a)| &self[i] == a)
     }
 
     fn into_owned(self) -> Vec<T> {
-        self.into_iter().map(|a| Lookup::into_owned(a.clone())).collect()
+        self.into_iter()
+            .map(|a| Lookup::into_owned(a.clone()))
+            .collect()
     }
 }
 
@@ -420,4 +420,4 @@ impl Lookup<PathBuf> for &Path {
     fn into_owned(self) -> PathBuf {
         self.to_owned()
     }
-}
\ No newline at end of file
+}
diff --git a/src/lib.rs b/src/lib.rs
index d3befc2..0ee7d3e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -132,8 +132,8 @@ pub mod plumbing {
         pub use crate::interned::Configuration;
         pub use crate::interned::IngredientImpl;
         pub use crate::interned::JarImpl;
-        pub use crate::interned::Value;
         pub use crate::interned::Lookup;
+        pub use crate::interned::Value;
     }
 
     pub mod function {
diff --git a/tests/interned-struct-with-lifetime.rs b/tests/interned-structs.rs
similarity index 99%
rename from tests/interned-struct-with-lifetime.rs
rename to tests/interned-structs.rs
index aa2306e..70dbc26 100644
--- a/tests/interned-struct-with-lifetime.rs
+++ b/tests/interned-structs.rs
@@ -1,8 +1,8 @@
 //! Test that a `tracked` fn on a `salsa::input`
 //! compiles and executes successfully.
 
-use std::path::{Path, PathBuf};
 use expect_test::expect;
+use std::path::{Path, PathBuf};
 use test_log::test;
 
 #[salsa::interned]
@@ -10,7 +10,6 @@ struct InternedString<'db> {
     data: String,
 }
 
-
 #[salsa::interned]
 struct InternedPair<'db> {
     data: (InternedString<'db>, InternedString<'db>),