Skip to content
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

Introduce Input::builder API #535

Closed
wants to merge 2 commits into from
Closed
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
33 changes: 31 additions & 2 deletions components/salsa-macro-rules/src/setup_input_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ macro_rules! setup_input_struct {
$zalsa:ident,
$zalsa_struct:ident,
$Configuration:ident,
$Builder:ident,
$CACHE:ident,
$Db:ident,
]
Expand Down Expand Up @@ -121,14 +122,24 @@ macro_rules! setup_input_struct {
}

impl $Struct {
#[inline]
pub fn $new_fn<$Db>(db: &$Db, $($field_id: $field_ty),*) -> Self
where
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + salsa::Database,
{
Self::builder(db).new($($field_id,)*)
}

pub fn builder<'db, $Db>(db: &'db $Db) -> $Builder<'db>
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);
let stamps = $zalsa::Array::new([$zalsa::stamp(current_revision, Default::default()); $N]);
$Configuration::ingredient(db.as_salsa_database()).new_input(($($field_id,)*), stamps)
$Builder {
inner: $zalsa_struct::BuilderImpl::new(current_revision, $Configuration::ingredient(db.as_salsa_database())),
}
}

$(
Expand Down Expand Up @@ -204,6 +215,24 @@ macro_rules! setup_input_struct {
})
}
}

pub struct $Builder<'db> {
#[doc(hidden)]
inner: $zalsa_struct::BuilderImpl<'db, $Configuration>,
}

impl<'db> $Builder<'db> {
/// Sets the durability for all fields.
pub fn durability(mut self, durability: $zalsa::Durability) -> Self {
self.inner.durability(durability);
self
}

pub fn new(self, $($field_id: $field_ty),*) -> $Struct {
self.inner.build(($($field_id,)*))
}
}

};
};
}
1 change: 0 additions & 1 deletion components/salsa-macro-rules/src/setup_struct_fn.rs

This file was deleted.

2 changes: 2 additions & 0 deletions components/salsa-macros/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ impl Macro {
let zalsa = self.hygiene.ident("zalsa");
let zalsa_struct = self.hygiene.ident("zalsa_struct");
let Configuration = self.hygiene.ident("Configuration");
let Builder = self.hygiene.ident("Builder");
let CACHE = self.hygiene.ident("CACHE");
let Db = self.hygiene.ident("Db");

Expand All @@ -117,6 +118,7 @@ impl Macro {
#zalsa,
#zalsa_struct,
#Configuration,
#Builder,
#CACHE,
#Db,
]
Expand Down
4 changes: 3 additions & 1 deletion src/database.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #534

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ pub trait Database: DatabaseGen {
/// will block until that snapshot is dropped -- if that snapshot
/// is owned by the current thread, this could trigger deadlock.
fn synthetic_write(&mut self, durability: Durability) {
self.runtime_mut().report_tracked_write(durability);
let runtime = self.runtime_mut();
runtime.new_revision();
runtime.report_tracked_write(durability);
}

/// Reports that the query depends on some state unknown to salsa.
Expand Down
1 change: 1 addition & 0 deletions src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{
sync::atomic::{AtomicU32, Ordering},
};

pub mod builder;
pub mod input_field;
pub mod setter;
mod struct_map;
Expand Down
41 changes: 41 additions & 0 deletions src/input/builder/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use super::{Configuration, IngredientImpl};
use crate::plumbing::Array;
use crate::runtime::Stamp;
use crate::{Durability, Revision};

pub struct BuilderImpl<'builder, C>
where
C: Configuration,
{
stamps: C::Stamps,

ingredient: &'builder IngredientImpl<C>,
}

impl<'builder, const N: usize, C> BuilderImpl<'builder, C>
where
C: Configuration<Stamps = Array<Stamp, N>>,
{
pub fn new(revision: Revision, ingredient: &'builder IngredientImpl<C>) -> Self {
Self {
ingredient,
stamps: Array::new([crate::plumbing::stamp(revision, Durability::default()); N]),
}
}

/// Sets the durability of a specific field.
pub fn set_field_durability(&mut self, index: usize, durability: Durability) -> &mut Self {
self.stamps[index].durability = durability;
self
}

pub fn durability(&mut self, durability: Durability) {
for stamp in &mut *self.stamps {
stamp.durability = durability;
}
}

pub fn build(self, fields: C::Fields) -> C::Struct {
self.ingredient.new_input(fields, self.stamps)
}
}
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ pub mod plumbing {
pub use crate::update::helper::Dispatch as UpdateDispatch;
pub use crate::update::helper::Fallback as UpdateFallback;
pub use crate::update::Update;
pub use crate::Durability;

pub use salsa_macro_rules::macro_if;
pub use salsa_macro_rules::maybe_backdate;
Expand All @@ -125,6 +126,7 @@ pub mod plumbing {
}

pub mod input {
pub use crate::input::builder::BuilderImpl;
pub use crate::input::input_field::FieldIngredientImpl;
pub use crate::input::setter::SetterImpl;
pub use crate::input::Configuration;
Expand Down
8 changes: 4 additions & 4 deletions tests/accumulate-from-tracked-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
//! Then mutate the values so that the tracked function re-executes.
//! Check that we accumulate the appropriate, new values.

mod common;
use common::{HasLogger, Logger};

use expect_test::expect;
use salsa::{Accumulator, Setter};
use test_log::test;

use common::{HasLogger, Logger};
use salsa::{Accumulator, Setter};

mod common;
#[salsa::db]
trait Db: salsa::Database + HasLogger {}

Expand Down
77 changes: 77 additions & 0 deletions tests/tracked_fn_on_input_with_high_durability.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//! Test that a `tracked` fn on a `salsa::input`
//! compiles and executes successfully.
#![allow(warnings)]

use expect_test::expect;

use common::{HasLogger, Logger};
use salsa::plumbing::HasStorage;
use salsa::{Database, Durability, Event, EventKind, Setter};

mod common;
#[salsa::input]
struct MyInput {
field: u32,
}

#[salsa::tracked]
fn tracked_fn(db: &dyn salsa::Database, input: MyInput) -> u32 {
input.field(db) * 2
}

#[test]
fn execute() {
#[salsa::db]
#[derive(Default)]
struct Database {
storage: salsa::Storage<Self>,
logger: Logger,
}

#[salsa::db]
impl salsa::Database for Database {
fn salsa_event(&self, event: Event) {
match event.kind {
EventKind::WillCheckCancellation => {}
_ => {
self.push_log(format!("salsa_event({:?})", event.kind));
}
}
}
}

impl HasLogger for Database {
fn logger(&self) -> &Logger {
&self.logger
}
}

let mut db = Database::default();
let input_low = MyInput::new(&db, 22);
let input_high = MyInput::builder(&db).durability(Durability::HIGH).new(2200);

assert_eq!(tracked_fn(&db, input_low), 44);
assert_eq!(tracked_fn(&db, input_high), 4400);

db.assert_logs(expect![[r#"
[
"salsa_event(WillExecute { database_key: tracked_fn(0) })",
"salsa_event(WillExecute { database_key: tracked_fn(1) })",
]"#]]);

db.synthetic_write(Durability::LOW);

assert_eq!(tracked_fn(&db, input_low), 44);
assert_eq!(tracked_fn(&db, input_high), 4400);

// There's currently no good way to verify whether an input was validated using shallow or deep comparison.
// All we can do for now is verify that the values were validated.
// Note: It maybe confusing why it validates `input_high` when the write has `Durability::LOW`.
// This is because all values must be validated whenever a write occurs. It doesn't mean that it
// executed the query.
db.assert_logs(expect![[r#"
[
"salsa_event(DidValidateMemoizedValue { database_key: tracked_fn(0) })",
"salsa_event(DidValidateMemoizedValue { database_key: tracked_fn(1) })",
]"#]]);
}
Loading