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

Conversation

davidbarsky
Copy link
Contributor

@davidbarsky davidbarsky commented Jan 23, 2025

This PR adds support for three features:

  1. Writing #[salsa::interned(no_lifetime)] allows skipping the 'db lifetime.
  2. #[salsa::interned] structs now have public ingredients, which allows accessing the underlying, interned data through the salsa::Id alone.
  3. Adds support for custom Salsa IDs. While rust-analyzer doesn't need this, their presence makes migration easier. I'd like to jettison this feature before the 1.0 release.

In my view, the use of the above features should be discouraged, but they are necessary for rust-analyzer to migrate to the new version of Salsa.

This PR supersedes #632.

Copy link

netlify bot commented Jan 23, 2025

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit a1f61d0
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/6797f242283abc000881c0df

Copy link

codspeed-hq bot commented Jan 23, 2025

CodSpeed Performance Report

Merging #661 will not alter performance

Comparing davidbarsky:davidbarsky/interned-structs-without-lifetimes-take-two (a1f61d0) with master (917cfa9)

Summary

✅ 9 untouched benchmarks

@davidbarsky davidbarsky force-pushed the davidbarsky/interned-structs-without-lifetimes-take-two branch from 231806c to 8ff97d7 Compare January 23, 2025 18:32
Comment on lines 76 to 90
impl salsa::plumbing::interned::Configuration for $StructWithStatic {
const DEBUG_NAME: &'static str = stringify!($Struct);
type Data<'a> = $StructDataIdent<'a>;
type Struct<'db> = $Struct< $($db_lt_arg)? >;
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()
}
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this got moved up out of the const _ scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it'd be necessary to get completions from rust-analyzer, but it wasn't. reverted.

@davidbarsky davidbarsky force-pushed the davidbarsky/interned-structs-without-lifetimes-take-two branch from f7bb8c6 to bd81aa4 Compare January 27, 2025 18:39
@nikomatsakis nikomatsakis added this pull request to the merge queue Jan 27, 2025
Merged via the queue into salsa-rs:master with commit 8c49bb0 Jan 27, 2025
5 checks passed
@davidbarsky davidbarsky deleted the davidbarsky/interned-structs-without-lifetimes-take-two branch January 27, 2025 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants