Skip to content

Do manual trait casting #922

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.

Sign up for GitHub

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
merged 4 commits into from
Jul 31, 2025
Merged

Do manual trait casting #922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
523d4c8
Do manual trait upcasting instead of downcasting
Veykril Jun 20, 2025
70bdaef
Remove another dynamic `zalsa` call
Veykril Jul 30, 2025
bde56cb
Rename UpCaster back to DownCaster
Veykril Jul 31, 2025
8c0ec58
Address reviews
Veykril Jul 31, 2025
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
19 changes: 11 additions & 8 deletions components/salsa-macro-rules/src/setup_input_struct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ macro_rules! setup_input_struct {
}
}

pub fn ingredient_mut(db: &mut dyn $zalsa::Database) -> (&mut $zalsa_struct::IngredientImpl<Self>, &mut $zalsa::Runtime) {
let zalsa_mut = db.zalsa_mut();
pub fn ingredient_mut(zalsa_mut: &mut $zalsa::Zalsa) -> (&mut $zalsa_struct::IngredientImpl<Self>, &mut $zalsa::Runtime) {
zalsa_mut.new_revision();
let index = zalsa_mut.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Configuration>>();
let (ingredient, runtime) = zalsa_mut.lookup_ingredient_mut(index);
Expand Down Expand Up @@ -208,8 +207,10 @@ macro_rules! setup_input_struct {
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
{
let fields = $Configuration::ingredient_(db.zalsa()).field(
db.as_dyn_database(),
let (zalsa, zalsa_local) = db.zalsas();
let fields = $Configuration::ingredient_(zalsa).field(
zalsa,
zalsa_local,
self,
$field_index,
);
Expand All @@ -228,7 +229,8 @@ macro_rules! setup_input_struct {
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
{
let (ingredient, revision) = $Configuration::ingredient_mut(db.as_dyn_database_mut());
let zalsa = db.zalsa_mut();
let (ingredient, revision) = $Configuration::ingredient_mut(zalsa);
$zalsa::input::SetterImpl::new(
revision,
self,
Expand Down Expand Up @@ -267,7 +269,8 @@ macro_rules! setup_input_struct {
$(for<'__trivial_bounds> $field_ty: std::fmt::Debug),*
{
$zalsa::with_attached_database(|db| {
let fields = $Configuration::ingredient(db).leak_fields(db, this);
let zalsa = db.zalsa();
let fields = $Configuration::ingredient_(zalsa).leak_fields(zalsa, this);
let mut f = f.debug_struct(stringify!($Struct));
let f = f.field("[salsa id]", &$zalsa::AsId::as_id(&this));
$(
Expand Down Expand Up @@ -296,11 +299,11 @@ macro_rules! setup_input_struct {
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + salsa::Database
{
let zalsa = db.zalsa();
let (zalsa, zalsa_local) = db.zalsas();
let current_revision = zalsa.current_revision();
let ingredient = $Configuration::ingredient_(zalsa);
let (fields, revision, durabilities) = builder::builder_into_inner(self, current_revision);
ingredient.new_input(db.as_dyn_database(), fields, revision, durabilities)
ingredient.new_input(zalsa, zalsa_local, fields, revision, durabilities)
}
}

Expand Down
15 changes: 7 additions & 8 deletions components/salsa-macro-rules/src/setup_interned_struct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,11 @@ macro_rules! setup_interned_struct {
}

impl $Configuration {
pub fn ingredient<Db>(db: &Db) -> &$zalsa_struct::IngredientImpl<Self>
where
Db: ?Sized + $zalsa::Database,
pub fn ingredient(zalsa: &$zalsa::Zalsa) -> &$zalsa_struct::IngredientImpl<Self>
{
static CACHE: $zalsa::IngredientCache<$zalsa_struct::IngredientImpl<$Configuration>> =
$zalsa::IngredientCache::new();

let zalsa = db.zalsa();

// SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the only
// ingredient created by our jar is the struct ingredient.
unsafe {
Expand Down Expand Up @@ -239,7 +235,8 @@ macro_rules! setup_interned_struct {
$field_ty: $zalsa::interned::HashEqLike<$indexed_ty>,
)*
{
$Configuration::ingredient(db).intern(db.as_dyn_database(),
let (zalsa, zalsa_local) = db.zalsas();
$Configuration::ingredient(zalsa).intern(zalsa, zalsa_local,
StructKey::<$db_lt>($($field_id,)* std::marker::PhantomData::default()), |_, data| ($($zalsa::interned::Lookup::into_owned(data.$field_index),)*))
}

Expand All @@ -250,7 +247,8 @@ macro_rules! setup_interned_struct {
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
{
let fields = $Configuration::ingredient(db).fields(db.as_dyn_database(), self);
let zalsa = db.zalsa();
let fields = $Configuration::ingredient(zalsa).fields(zalsa, self);
$zalsa::return_mode_expression!(
$field_option,
$field_ty,
Expand All @@ -262,7 +260,8 @@ macro_rules! setup_interned_struct {
/// Default debug formatting for this struct (may be useful if you define your own `Debug` impl)
pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
$zalsa::with_attached_database(|db| {
let fields = $Configuration::ingredient(db).fields(db.as_dyn_database(), this);
let zalsa = db.zalsa();
let fields = $Configuration::ingredient(zalsa).fields(zalsa, this);
let mut f = f.debug_struct(stringify!($Struct));
$(
let f = f.field(stringify!($field_id), &fields.$field_index);
Expand Down
37 changes: 25 additions & 12 deletions components/salsa-macro-rules/src/setup_tracked_fn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,17 +175,21 @@ macro_rules! setup_tracked_fn {
impl $Configuration {
fn fn_ingredient(db: &dyn $Db) -> &$zalsa::function::IngredientImpl<$Configuration> {
let zalsa = db.zalsa();
Self::fn_ingredient_(db, zalsa)
}

#[inline]
fn fn_ingredient_<'z>(db: &dyn $Db, zalsa: &'z $zalsa::Zalsa) -> &'z $zalsa::function::IngredientImpl<$Configuration> {
// SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the first
// ingredient created by our jar is the function ingredient.
unsafe {
$FN_CACHE.get_or_create(zalsa, || zalsa.lookup_jar_by_type::<$fn_name>())
}
.get_or_init(|| <dyn $Db as $Db>::zalsa_register_downcaster(db))
.get_or_init(|| *<dyn $Db as $Db>::zalsa_register_downcaster(db))
}

pub fn fn_ingredient_mut(db: &mut dyn $Db) -> &mut $zalsa::function::IngredientImpl<Self> {
let view = <dyn $Db as $Db>::zalsa_register_downcaster(db);
let view = *<dyn $Db as $Db>::zalsa_register_downcaster(db);
let zalsa_mut = db.zalsa_mut();
let index = zalsa_mut.lookup_jar_by_type::<$fn_name>();
let (ingredient, _) = zalsa_mut.lookup_ingredient_mut(index);
Expand All @@ -199,7 +203,12 @@ macro_rules! setup_tracked_fn {
db: &dyn $Db,
) -> &$zalsa::interned::IngredientImpl<$Configuration> {
let zalsa = db.zalsa();

Self::intern_ingredient_(zalsa)
}
#[inline]
fn intern_ingredient_<'z>(
zalsa: &'z $zalsa::Zalsa
) -> &'z $zalsa::interned::IngredientImpl<$Configuration> {
// SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the second
// ingredient created by our jar is the interned ingredient (given `needs_interner`).
unsafe {
Expand Down Expand Up @@ -257,12 +266,12 @@ macro_rules! setup_tracked_fn {
$($cycle_recovery_fn)*(db, value, count, $($input_id),*)
}

fn id_to_input<$db_lt>(db: &$db_lt Self::DbView, key: salsa::Id) -> Self::Input<$db_lt> {
fn id_to_input<$db_lt>(zalsa: &$db_lt $zalsa::Zalsa, key: salsa::Id) -> Self::Input<$db_lt> {
$zalsa::macro_if! {
if $needs_interner {
$Configuration::intern_ingredient(db).data(db.as_dyn_database(), key).clone()
$Configuration::intern_ingredient_(zalsa).data(zalsa, key).clone()
} else {
$zalsa::FromIdWithDb::from_id(key, db.zalsa())
$zalsa::FromIdWithDb::from_id(key, zalsa)
}
}
}
Expand Down Expand Up @@ -340,9 +349,10 @@ macro_rules! setup_tracked_fn {
) -> Vec<&$db_lt A> {
use salsa::plumbing as $zalsa;
let key = $zalsa::macro_if! {
if $needs_interner {
$Configuration::intern_ingredient($db).intern_id($db.as_dyn_database(), ($($input_id),*), |_, data| data)
} else {
if $needs_interner {{
let (zalsa, zalsa_local) = $db.zalsas();
$Configuration::intern_ingredient($db).intern_id(zalsa, zalsa_local, ($($input_id),*), |_, data| data)
}} else {
$zalsa::AsId::as_id(&($($input_id),*))
}
};
Expand Down Expand Up @@ -380,14 +390,17 @@ macro_rules! setup_tracked_fn {
}

$zalsa::attach($db, || {
let (zalsa, zalsa_local) = $db.zalsas();
let result = $zalsa::macro_if! {
if $needs_interner {
{
let key = $Configuration::intern_ingredient($db).intern_id($db.as_dyn_database(), ($($input_id),*), |_, data| data);
$Configuration::fn_ingredient($db).fetch($db, key)
let key = $Configuration::intern_ingredient_(zalsa).intern_id(zalsa, zalsa_local, ($($input_id),*), |_, data| data);
$Configuration::fn_ingredient_($db, zalsa).fetch($db, zalsa, zalsa_local, key)
}
} else {
$Configuration::fn_ingredient($db).fetch($db, $zalsa::AsId::as_id(&($($input_id),*)))
{
$Configuration::fn_ingredient_($db, zalsa).fetch($db, zalsa, zalsa_local, $zalsa::AsId::as_id(&($($input_id),*)))
}
}
};

Expand Down
16 changes: 9 additions & 7 deletions components/salsa-macro-rules/src/setup_tracked_struct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,9 @@ macro_rules! setup_tracked_struct {
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
{
$Configuration::ingredient(db.as_dyn_database()).new_struct(
db.as_dyn_database(),
let (zalsa, zalsa_local) = db.zalsas();
$Configuration::ingredient_(zalsa).new_struct(
zalsa,zalsa_local,
($($field_id,)*)
)
}
Expand All @@ -295,8 +296,8 @@ macro_rules! setup_tracked_struct {
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
{
let db = db.as_dyn_database();
let fields = $Configuration::ingredient(db).tracked_field(db, self, $relative_tracked_index);
let (zalsa, zalsa_local) = db.zalsas();
let fields = $Configuration::ingredient_(zalsa).tracked_field(zalsa, zalsa_local, self, $relative_tracked_index);
$crate::return_mode_expression!(
$tracked_option,
$tracked_ty,
Expand All @@ -312,8 +313,8 @@ macro_rules! setup_tracked_struct {
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
{
let db = db.as_dyn_database();
let fields = $Configuration::ingredient(db).untracked_field(db, self);
let zalsa = db.zalsa();
let fields = $Configuration::ingredient_(zalsa).untracked_field(zalsa, self);
$crate::return_mode_expression!(
$untracked_option,
$untracked_ty,
Expand All @@ -335,7 +336,8 @@ macro_rules! setup_tracked_struct {
$(for<$db_lt> $field_ty: std::fmt::Debug),*
{
$zalsa::with_attached_database(|db| {
let fields = $Configuration::ingredient(db).leak_fields(db, this);
let zalsa = db.zalsa();
let fields = $Configuration::ingredient_(zalsa).leak_fields(zalsa, this);
let mut f = f.debug_struct(stringify!($Struct));
let f = f.field("[salsa id]", &$zalsa::AsId::as_id(&this));
$(
Expand Down
22 changes: 9 additions & 13 deletions components/salsa-macros/src/db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,14 @@ impl DbMacro {
let trait_name = &input.ident;
input.items.push(parse_quote! {
#[doc(hidden)]
fn zalsa_register_downcaster(&self) -> salsa::plumbing::DatabaseDownCaster<dyn #trait_name>;
fn zalsa_register_downcaster(&self) -> &salsa::plumbing::DatabaseDownCaster<dyn #trait_name>;
});

let comment = format!(" Downcast a [`dyn Database`] to a [`dyn {trait_name}`]");
let comment = format!(" downcast `Self` to a [`dyn {trait_name}`]");
input.items.push(parse_quote! {
#[doc = #comment]
///
/// # Safety
///
/// The input database must be of type `Self`.
#[doc(hidden)]
unsafe fn downcast(db: &dyn salsa::plumbing::Database) -> &dyn #trait_name where Self: Sized;
fn downcast(&self) -> &dyn #trait_name where Self: Sized;
});
Ok(())
}
Expand All @@ -138,17 +134,17 @@ impl DbMacro {
#[cold]
#[inline(never)]
#[doc(hidden)]
fn zalsa_register_downcaster(&self) -> salsa::plumbing::DatabaseDownCaster<dyn #TraitPath> {
salsa::plumbing::views(self).add(<Self as #TraitPath>::downcast)
fn zalsa_register_downcaster(&self) -> &salsa::plumbing::DatabaseDownCaster<dyn #TraitPath> {
salsa::plumbing::views(self).add::<Self, dyn #TraitPath>(unsafe {
::std::mem::transmute(<Self as #TraitPath>::downcast as fn(_) -> _)
})
}
});
input.items.push(parse_quote! {
#[doc(hidden)]
#[inline(always)]
unsafe fn downcast(db: &dyn salsa::plumbing::Database) -> &dyn #TraitPath where Self: Sized {
debug_assert_eq!(db.type_id(), ::core::any::TypeId::of::<Self>());
// SAFETY: The input database must be of type `Self`.
unsafe { &*salsa::plumbing::transmute_data_ptr::<dyn salsa::plumbing::Database, Self>(db) }
fn downcast(&self) -> &dyn #TraitPath where Self: Sized {
self
}
});
Ok(())
Expand Down
3 changes: 2 additions & 1 deletion src/accumulator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {

unsafe fn maybe_changed_after(
&self,
_db: &dyn Database,
_zalsa: &crate::zalsa::Zalsa,
_db: crate::database::RawDatabase<'_>,
_input: Id,
_revision: Revision,
_cycle_heads: &mut CycleHeads,
Expand Down
57 changes: 32 additions & 25 deletions src/database.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,39 @@
use std::any::Any;
use std::borrow::Cow;
use std::ptr::NonNull;

use crate::views::DatabaseDownCaster;
use crate::zalsa::{IngredientIndex, ZalsaDatabase};
use crate::{Durability, Revision};

#[derive(Copy, Clone)]
pub struct RawDatabase<'db> {
pub(crate) ptr: NonNull<()>,
_marker: std::marker::PhantomData<&'db dyn Database>,
}

impl<'db, Db: Database + ?Sized> From<&'db Db> for RawDatabase<'db> {
#[inline]
fn from(db: &'db Db) -> Self {
RawDatabase {
ptr: NonNull::from(db).cast(),
_marker: std::marker::PhantomData,
}
}
}

impl<'db, Db: Database + ?Sized> From<&'db mut Db> for RawDatabase<'db> {
#[inline]
fn from(db: &'db mut Db) -> Self {
RawDatabase {
ptr: NonNull::from(db).cast(),
_marker: std::marker::PhantomData,
}
}
}

/// The trait implemented by all Salsa databases.
/// You can create your own subtraits of this trait using the `#[salsa::db]`(`crate::db`) procedural macro.
pub trait Database: Send + AsDynDatabase + Any + ZalsaDatabase {
pub trait Database: Send + ZalsaDatabase + AsDynDatabase {
/// Enforces current LRU limits, evicting entries if necessary.
///
/// **WARNING:** Just like an ordinary write, this method triggers
Expand Down Expand Up @@ -84,59 +110,40 @@ pub trait Database: Send + AsDynDatabase + Any + ZalsaDatabase {
#[cold]
#[inline(never)]
#[doc(hidden)]
fn zalsa_register_downcaster(&self) -> DatabaseDownCaster<dyn Database> {
fn zalsa_register_downcaster(&self) -> &DatabaseDownCaster<dyn Database> {
self.zalsa().views().downcaster_for::<dyn Database>()
// The no-op downcaster is special cased in view caster construction.
}

#[doc(hidden)]
#[inline(always)]
unsafe fn downcast(db: &dyn Database) -> &dyn Database
fn downcast(&self) -> &dyn Database
where
Self: Sized,
{
// No-op
db
self
}
}

/// Upcast to a `dyn Database`.
///
/// Only required because upcasts not yet stabilized (*grr*).
/// Only required because upcasting does not work for unsized generic parameters.
Copy link
Member Author

Choose a reason for hiding this comment

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

I am quite annoyed by this but that's a limitation of Rust, a &T of T: ?Sized + SomeTrait cannot be coerced to a &dyn SomeTrait (or a supertrait) due to T possibly being an unrelated unsized type like str

Copy link
Member

Choose a reason for hiding this comment

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

Can we add the Sized requirement?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, specifically this is used solely for attach which needs to take a dyn database of some form.

pub trait AsDynDatabase {
fn as_dyn_database(&self) -> &dyn Database;
fn as_dyn_database_mut(&mut self) -> &mut dyn Database;
}

impl<T: Database> AsDynDatabase for T {
#[inline(always)]
fn as_dyn_database(&self) -> &dyn Database {
self
}

#[inline(always)]
fn as_dyn_database_mut(&mut self) -> &mut dyn Database {
self
}
}

pub fn current_revision<Db: ?Sized + Database>(db: &Db) -> Revision {
db.zalsa().current_revision()
}

impl dyn Database {
/// Upcasts `self` to the given view.
///
/// # Panics
///
/// If the view has not been added to the database (see [`crate::views::Views`]).
#[track_caller]
pub fn as_view<DbView: ?Sized + Database>(&self) -> &DbView {
let views = self.zalsa().views();
views.downcaster_for().downcast(self)
}
}

#[cfg(feature = "salsa_unstable")]
pub use memory_usage::IngredientInfo;

Expand Down
Loading