wip: accum, privacy for inputs

This commit is contained in:
Niko Matsakis 2024-07-19 07:08:11 -04:00
parent 82a1e59e46
commit 3304acc5dd
25 changed files with 249 additions and 200 deletions

View file

@ -20,7 +20,10 @@ macro_rules! setup_input_struct {
// Field names
field_ids: [$($field_id:ident),*],
// Names for field setter methods (typically `set_foo`)
// Visibilities for field accessor methods
field_vis: [$($field_vis:vis f),*],
// Names for field getter methods (typically `foo`)
field_getter_ids: [$($field_getter_id:ident),*],
// Names for field setter methods (typically `set_foo`)
@ -132,7 +135,7 @@ macro_rules! setup_input_struct {
}
$(
pub fn $field_getter_id<'db, $Db>(self, db: &'db $Db) -> $zalsa::maybe_cloned_ty!($field_option, 'db, $field_ty)
$field_vis fn $field_getter_id<'db, $Db>(self, db: &'db $Db) -> $zalsa::maybe_cloned_ty!($field_option, 'db, $field_ty)
where
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
@ -149,7 +152,7 @@ macro_rules! setup_input_struct {
$(
#[must_use]
pub fn $field_setter_id<'db, $Db>(self, db: &'db mut $Db) -> impl salsa::Setter<FieldTy = $field_ty> + 'db
$field_vis fn $field_setter_id<'db, $Db>(self, db: &'db mut $Db) -> impl salsa::Setter<FieldTy = $field_ty> + 'db
where
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,

View file

@ -1,7 +1,10 @@
use proc_macro2::TokenStream;
use syn::parse::Nothing;
use crate::hygiene::Hygiene;
use crate::{
hygiene::Hygiene,
options::{AllowedOptions, Options},
};
// #[salsa::accumulator(jar = Jar0)]
// struct Accumulator(DataType);
@ -11,11 +14,12 @@ pub(crate) fn accumulator(
input: proc_macro::TokenStream,
) -> proc_macro::TokenStream {
let hygiene = Hygiene::from1(&input);
let _ = syn::parse_macro_input!(args as Nothing);
let args = syn::parse_macro_input!(args as Options<Accumulator>);
let struct_item = syn::parse_macro_input!(input as syn::ItemStruct);
let ident = struct_item.ident.clone();
let m = StructMacro {
hygiene,
args,
struct_item,
};
match m.try_expand() {
@ -24,8 +28,25 @@ pub(crate) fn accumulator(
}
}
struct Accumulator;
impl AllowedOptions for Accumulator {
const RETURN_REF: bool = false;
const SPECIFY: bool = false;
const NO_EQ: bool = false;
const NO_DEBUG: bool = true;
const NO_CLONE: bool = true;
const SINGLETON: bool = false;
const DATA: bool = false;
const DB: bool = false;
const RECOVERY_FN: bool = false;
const LRU: bool = false;
const CONSTRUCTOR_NAME: bool = false;
}
struct StructMacro {
hygiene: Hygiene,
args: Options<Accumulator>,
struct_item: syn::ItemStruct,
}
@ -41,7 +62,16 @@ impl StructMacro {
let struct_item = self.struct_item;
let mut derives = vec![];
if self.args.no_debug.is_none() {
derives.push(quote!(Debug));
}
if self.args.no_clone.is_none() {
derives.push(quote!(Clone));
}
Ok(quote! {
#[derive(#(#derives),*)]
#struct_item
salsa::plumbing::setup_accumulator_impl! {

View file

@ -41,6 +41,8 @@ impl crate::options::AllowedOptions for InputStruct {
const NO_DEBUG: bool = true;
const NO_CLONE: bool = false;
const SINGLETON: bool = true;
const DATA: bool = true;
@ -80,6 +82,7 @@ impl Macro {
let field_ids = salsa_struct.field_ids();
let field_indices = salsa_struct.field_indices();
let num_fields = salsa_struct.num_fields();
let field_vis = salsa_struct.field_vis();
let field_getter_ids = salsa_struct.field_getter_ids();
let field_setter_ids = salsa_struct.field_setter_ids();
let field_options = salsa_struct.field_options();
@ -103,6 +106,7 @@ impl Macro {
new_fn: #new_fn,
field_options: [#(#field_options),*],
field_ids: [#(#field_ids),*],
field_vis: [#(#field_vis f),*],
field_getter_ids: [#(#field_getter_ids),*],
field_setter_ids: [#(#field_setter_ids),*],
field_tys: [#(#field_tys),*],

View file

@ -42,6 +42,8 @@ impl crate::options::AllowedOptions for InternedStruct {
const NO_DEBUG: bool = true;
const NO_CLONE: bool = false;
const SINGLETON: bool = true;
const DATA: bool = true;

View file

@ -24,6 +24,11 @@ pub(crate) struct Options<A: AllowedOptions> {
/// If this is `Some`, the value is the `no_debug` identifier.
pub no_debug: Option<syn::Ident>,
/// Signal we should not generate a `Clone` impl.
///
/// If this is `Some`, the value is the `no_clone` identifier.
pub no_clone: Option<syn::Ident>,
/// The `singleton` option is used on input with only one field
/// It allows the creation of convenient methods
pub singleton: Option<syn::Ident>,
@ -72,6 +77,7 @@ impl<A: AllowedOptions> Default for Options<A> {
specify: Default::default(),
no_eq: Default::default(),
no_debug: Default::default(),
no_clone: Default::default(),
db_path: Default::default(),
recovery_fn: Default::default(),
data: Default::default(),
@ -89,6 +95,7 @@ pub(crate) trait AllowedOptions {
const SPECIFY: bool;
const NO_EQ: bool;
const NO_DEBUG: bool;
const NO_CLONE: bool;
const SINGLETON: bool;
const DATA: bool;
const DB: bool;
@ -145,6 +152,20 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
"`no_debug` option not allowed here",
));
}
} else if ident == "no_clone" {
if A::NO_CLONE {
if let Some(old) = std::mem::replace(&mut options.no_clone, Some(ident)) {
return Err(syn::Error::new(
old.span(),
"option `no_clone` provided twice",
));
}
} else {
return Err(syn::Error::new(
ident.span(),
"`no_clone` option not allowed here",
));
}
} else if ident == "singleton" {
if A::SINGLETON {
if let Some(old) = std::mem::replace(&mut options.singleton, Some(ident)) {

View file

@ -173,6 +173,10 @@ where
.collect()
}
pub(crate) fn field_vis(&self) -> Vec<&syn::Visibility> {
self.fields.iter().map(|f| &f.field.vis).collect()
}
pub(crate) fn field_getter_ids(&self) -> Vec<&syn::Ident> {
self.fields.iter().map(|f| &f.get_name).collect()
}

View file

@ -30,6 +30,8 @@ impl crate::options::AllowedOptions for TrackedFn {
const NO_DEBUG: bool = false;
const NO_CLONE: bool = false;
const SINGLETON: bool = false;
const DATA: bool = false;

View file

@ -37,6 +37,8 @@ impl crate::options::AllowedOptions for TrackedStruct {
const NO_DEBUG: bool = true;
const NO_CLONE: bool = false;
const SINGLETON: bool = true;
const DATA: bool = true;

View file

@ -97,7 +97,7 @@ pub struct Span<'db> {
// ANCHOR: diagnostic
#[salsa::accumulator]
#[derive(new, Clone, Debug)]
#[derive(new)]
pub struct Diagnostic {
pub start: usize,
pub end: usize,

View file

@ -133,7 +133,6 @@ impl salsa::Database for Database {
}
#[salsa::accumulator]
#[derive(Clone, Debug)]
struct Diagnostic(String);
impl Diagnostic {

View file

@ -0,0 +1,45 @@
mod common;
use expect_test::expect;
use salsa::{Accumulator, Database};
use test_log::test;
#[salsa::input]
struct MyInput {
count: u32,
}
#[salsa::accumulator(no_clone)]
struct Log(String);
impl Clone for Log {
fn clone(&self) -> Self {
Self(format!("{}.clone()", self.0))
}
}
#[salsa::tracked]
fn push_logs(db: &dyn salsa::Database, input: MyInput) {
for i in 0..input.count(db) {
Log(format!("#{i}")).accumulate(db);
}
}
#[test]
fn accumulate_custom_clone() {
salsa::default_database().attach(|db| {
let input = MyInput::new(db, 2);
let logs = push_logs::accumulated::<Log>(db, input);
expect![[r##"
[
Log(
"#0.clone()",
),
Log(
"#1.clone()",
),
]
"##]]
.assert_debug_eq(&logs);
})
}

View file

@ -0,0 +1,45 @@
mod common;
use expect_test::expect;
use salsa::{Accumulator, Database};
use test_log::test;
#[salsa::input]
struct MyInput {
count: u32,
}
#[salsa::accumulator(no_debug)]
struct Log(String);
impl std::fmt::Debug for Log {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_tuple("CustomLog").field(&self.0).finish()
}
}
#[salsa::tracked]
fn push_logs(db: &dyn salsa::Database, input: MyInput) {
for i in 0..input.count(db) {
Log(format!("#{i}")).accumulate(db);
}
}
#[test]
fn accumulate_custom_debug() {
salsa::default_database().attach(|db| {
let input = MyInput::new(db, 2);
let logs = push_logs::accumulated::<Log>(db, input);
expect![[r##"
[
CustomLog(
"#0",
),
CustomLog(
"#1",
),
]
"##]]
.assert_debug_eq(&logs);
})
}

View file

@ -1,5 +1,4 @@
mod common;
use common::{HasLogger, Logger};
use expect_test::expect;
use salsa::{Accumulator, Database};
@ -12,7 +11,6 @@ struct MyInput {
}
#[salsa::accumulator]
#[derive(Clone, Debug)]
struct Log(#[allow(dead_code)] String);
#[salsa::tracked]

View file

@ -19,7 +19,7 @@ struct List {
}
#[salsa::accumulator]
#[derive(Copy, Clone, Debug)]
#[derive(Copy)]
struct Integers(u32);
#[salsa::tracked]

View file

@ -19,7 +19,7 @@ struct List {
}
#[salsa::accumulator]
#[derive(Clone, Copy, Debug)]
#[derive(Copy)]
struct Integers(u32);
#[salsa::tracked]

View file

@ -20,7 +20,6 @@ struct List {
}
#[salsa::accumulator]
#[derive(Clone, Debug)]
struct Integers(u32);
#[salsa::tracked]

View file

@ -1,7 +1,3 @@
//! Basic deletion test:
//!
//! * entities not created in a revision are deleted, as is any memoized data keyed on them.
mod common;
use common::{HasLogger, Logger};
@ -19,7 +15,6 @@ struct MyInput {
}
#[salsa::accumulator]
#[derive(Clone, Debug)]
struct Log(#[allow(dead_code)] String);
#[salsa::tracked]

View file

@ -1,17 +0,0 @@
#[salsa::jar(db = Db)]
struct Jar(AccTwoUnnamedFields, AccNamedField);
trait Db: salsa::DbWithJar<Jar> {}
// accumulator with more than one unnamed fields
#[salsa::accumulator]
struct AccTwoUnnamedFields (u32, u32);
// accumulator with named fields
#[salsa::accumulator]
struct AccNamedField {
field: u32,
}
fn main() {}

View file

@ -1,23 +0,0 @@
error: accumulator structs should have only one anonymous field
--> tests/compile-fail/accumulator_fields_incompatibles.rs:8:8
|
8 | struct AccTwoUnnamedFields (u32, u32);
| ^^^^^^^^^^^^^^^^^^^
error: accumulator structs should have only one anonymous field
--> tests/compile-fail/accumulator_fields_incompatibles.rs:13:8
|
13 | struct AccNamedField {
| ^^^^^^^^^^^^^
error[E0412]: cannot find type `AccTwoUnnamedFields` in this scope
--> tests/compile-fail/accumulator_fields_incompatibles.rs:2:12
|
2 | struct Jar(AccTwoUnnamedFields, AccNamedField);
| ^^^^^^^^^^^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AccNamedField` in this scope
--> tests/compile-fail/accumulator_fields_incompatibles.rs:2:33
|
2 | struct Jar(AccTwoUnnamedFields, AccNamedField);
| ^^^^^^^^^^^^^ not found in this scope

View file

@ -1,30 +1,25 @@
#[salsa::jar(db = Db)]
struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
#[salsa::accumulator(return_ref)]
struct AccWithRetRef(u32);
trait Db: salsa::DbWithJar<Jar> {}
#[salsa::accumulator(specify)]
struct AccWithSpecify(u32);
#[salsa::accumulator(jar = Jar, return_ref)]
struct AccWithRetRef (u32);
#[salsa::accumulator(no_eq)]
struct AccWithNoEq(u32);
#[salsa::accumulator(jar = Jar, specify)]
struct AccWithSpecify (u32);
#[salsa::accumulator(data = MyAcc)]
struct AccWithData(u32);
#[salsa::accumulator(jar = Jar, no_eq)]
struct AccWithNoEq (u32);
#[salsa::accumulator(db = Db)]
struct AcWithcDb(u32);
#[salsa::accumulator(jar = Jar, data = MyAcc)]
struct AccWithData (u32);
#[salsa::accumulator(recover_fn = recover)]
struct AccWithRecover(u32);
#[salsa::accumulator(jar = Jar, db = Db)]
struct AcWithcDb (u32);
#[salsa::accumulator(lru = 12)]
struct AccWithLru(u32);
#[salsa::accumulator(jar = Jar, recover_fn = recover)]
struct AccWithRecover (u32);
#[salsa::accumulator(constructor = Constructor)]
struct AccWithConstructor(u32);
#[salsa::accumulator(jar = Jar, lru =12)]
struct AccWithLru (u32);
#[salsa::accumulator(jar = Jar, constructor = Constructor)]
struct AccWithConstructor (u32);
fn main() {}
fn main() {}

View file

@ -1,95 +1,47 @@
error: `return_ref` option not allowed here
--> tests/compile-fail/accumulator_incompatibles.rs:6:33
--> tests/compile-fail/accumulator_incompatibles.rs:1:22
|
6 | #[salsa::accumulator(jar = Jar, return_ref)]
| ^^^^^^^^^^
1 | #[salsa::accumulator(return_ref)]
| ^^^^^^^^^^
error: `specify` option not allowed here
--> tests/compile-fail/accumulator_incompatibles.rs:9:33
--> tests/compile-fail/accumulator_incompatibles.rs:4:22
|
9 | #[salsa::accumulator(jar = Jar, specify)]
| ^^^^^^^
4 | #[salsa::accumulator(specify)]
| ^^^^^^^
error: `no_eq` option not allowed here
--> tests/compile-fail/accumulator_incompatibles.rs:12:33
|
12 | #[salsa::accumulator(jar = Jar, no_eq)]
| ^^^^^
--> tests/compile-fail/accumulator_incompatibles.rs:7:22
|
7 | #[salsa::accumulator(no_eq)]
| ^^^^^
error: `data` option not allowed here
--> tests/compile-fail/accumulator_incompatibles.rs:15:33
--> tests/compile-fail/accumulator_incompatibles.rs:10:22
|
15 | #[salsa::accumulator(jar = Jar, data = MyAcc)]
| ^^^^
10 | #[salsa::accumulator(data = MyAcc)]
| ^^^^
error: `db` option not allowed here
--> tests/compile-fail/accumulator_incompatibles.rs:18:33
--> tests/compile-fail/accumulator_incompatibles.rs:13:22
|
18 | #[salsa::accumulator(jar = Jar, db = Db)]
| ^^
13 | #[salsa::accumulator(db = Db)]
| ^^
error: unrecognized option `recover_fn`
--> tests/compile-fail/accumulator_incompatibles.rs:21:33
--> tests/compile-fail/accumulator_incompatibles.rs:16:22
|
21 | #[salsa::accumulator(jar = Jar, recover_fn = recover)]
| ^^^^^^^^^^
16 | #[salsa::accumulator(recover_fn = recover)]
| ^^^^^^^^^^
error: `lru` option not allowed here
--> tests/compile-fail/accumulator_incompatibles.rs:24:33
--> tests/compile-fail/accumulator_incompatibles.rs:19:22
|
24 | #[salsa::accumulator(jar = Jar, lru =12)]
| ^^^
19 | #[salsa::accumulator(lru = 12)]
| ^^^
error: `constructor` option not allowed here
--> tests/compile-fail/accumulator_incompatibles.rs:27:33
--> tests/compile-fail/accumulator_incompatibles.rs:22:22
|
27 | #[salsa::accumulator(jar = Jar, constructor = Constructor)]
| ^^^^^^^^^^^
error[E0412]: cannot find type `AccWithRetRef` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:12
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AccWithSpecify` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:27
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AccWithNoEq` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:43
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AccWithData` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:56
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AcWithcDb` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:69
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AccWithRecover` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:80
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AccWithLru` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:96
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^^ not found in this scope
error[E0412]: cannot find type `AccWithConstructor` in this scope
--> tests/compile-fail/accumulator_incompatibles.rs:2:108
|
2 | struct Jar(AccWithRetRef, AccWithSpecify, AccWithNoEq, AccWithData, AcWithcDb, AccWithRecover, AccWithLru, AccWithConstructor);
| ^^^^^^^^^^^^^^^^^^ not found in this scope
22 | #[salsa::accumulator(constructor = Constructor)]
| ^^^^^^^^^^^

View file

@ -1,31 +0,0 @@
#[salsa::jar(db = Db)]
pub struct Jar(a::MyInput);
mod a {
use crate::Jar;
#[salsa::input]
pub struct MyInput {
field: u32,
}
}
pub trait Db: salsa::DbWithJar<Jar> {}
#[salsa::db(Jar)]
#[derive(Default)]
struct Database {
storage: salsa::Storage<Self>,
}
impl salsa::Database for Database {}
impl Db for Database {}
fn main() {
let mut db = Database::default();
let input = a::MyInput::new(&mut db, 22);
input.field(&db);
input.set_field(&mut db).to(23);
}

View file

@ -1,17 +0,0 @@
error[E0624]: method `field` is private
--> tests/compile-fail/get-set-on-private-field.rs:29:11
|
9 | field: u32,
| ---------- private method defined here
...
29 | input.field(&db);
| ^^^^^ private method
error[E0624]: method `set_field` is private
--> tests/compile-fail/get-set-on-private-field.rs:30:11
|
9 | field: u32,
| ----- private method defined here
...
30 | input.set_field(&mut db).to(23);
| ^^^^^^^^^ private method

View file

@ -0,0 +1,16 @@
use salsa::prelude::*;
mod a {
#[salsa::input]
pub struct MyInput {
field: u32,
}
}
fn main() {
let mut db = salsa::default_database();
let input = a::MyInput::new(&mut db, 22);
input.field(&db);
input.set_field(&mut db).to(23);
}

View file

@ -0,0 +1,25 @@
error[E0624]: method `field` is private
--> tests/compile-fail/get-set-on-private-input-field.rs:14:11
|
4 | #[salsa::input]
| --------------- private method defined here
...
14 | input.field(&db);
| ^^^^^ private method
error[E0624]: method `set_field` is private
--> tests/compile-fail/get-set-on-private-input-field.rs:15:11
|
4 | #[salsa::input]
| --------------- private method defined here
...
15 | input.set_field(&mut db).to(23);
| ^^^^^^^^^ private method
warning: unused import: `salsa::prelude`
--> tests/compile-fail/get-set-on-private-input-field.rs:1:5
|
1 | use salsa::prelude::*;
| ^^^^^^^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default