From 3542f2a1e72b992e9a3d703169619173d6de18b7 Mon Sep 17 00:00:00 2001 From: Mihail Mihov Date: Wed, 31 Aug 2022 22:58:56 +0300 Subject: [PATCH 1/6] Add methods in `SalsaField` and `SalsaStruct` to get visibility of field --- components/salsa-2022-macros/src/salsa_struct.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/components/salsa-2022-macros/src/salsa_struct.rs b/components/salsa-2022-macros/src/salsa_struct.rs index 420cfcdb..6a86a3f9 100644 --- a/components/salsa-2022-macros/src/salsa_struct.rs +++ b/components/salsa-2022-macros/src/salsa_struct.rs @@ -96,6 +96,11 @@ impl SalsaStruct { self.all_fields().map(|ef| ef.name()).collect() } + /// Visibilities of all fields + pub(crate) fn all_field_vises(&self) -> Vec<&syn::Visibility> { + self.all_fields().map(|ef| ef.vis()).collect() + } + /// Names of getters of all fields pub(crate) fn all_get_field_names(&self) -> Vec<&syn::Ident> { self.all_fields().map(|ef| ef.get_name()).collect() @@ -406,6 +411,11 @@ impl SalsaField { self.field.ident.as_ref().unwrap() } + /// The visibility of this field. + pub(crate) fn vis(&self) -> &syn::Visibility { + &self.field.vis + } + /// The type of this field (all `SalsaField` instances are named). pub(crate) fn ty(&self) -> &syn::Type { &self.field.ty From 784828ec426d8c08774a57de95c693cc97047546 Mon Sep 17 00:00:00 2001 From: Mihail Mihov Date: Wed, 31 Aug 2022 22:59:37 +0300 Subject: [PATCH 2/6] Use visibility of `field` in `SalsaField` in `#[salsa::input]` getters and setters --- components/salsa-2022-macros/src/input.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/components/salsa-2022-macros/src/input.rs b/components/salsa-2022-macros/src/input.rs index e1a815c4..b7733320 100644 --- a/components/salsa-2022-macros/src/input.rs +++ b/components/salsa-2022-macros/src/input.rs @@ -87,13 +87,14 @@ impl InputStruct { let field_indices = self.all_field_indices(); let field_names = self.all_field_names(); + let field_vises = self.all_field_vises(); let field_tys: Vec<_> = self.all_field_tys(); let field_clones: Vec<_> = self.all_fields().map(SalsaField::is_clone_field).collect(); let get_field_names: Vec<_> = self.all_get_field_names(); - let field_getters: Vec = field_indices.iter().zip(&get_field_names).zip(&field_tys).zip(&field_clones).map(|(((field_index, get_field_name), field_ty), is_clone_field)| + let field_getters: Vec = field_indices.iter().zip(&get_field_names).zip(&field_vises).zip(&field_tys).zip(&field_clones).map(|((((field_index, get_field_name), field_vis), field_ty), is_clone_field)| if !*is_clone_field { parse_quote! { - pub fn #get_field_name<'db>(self, __db: &'db #db_dyn_ty) -> &'db #field_ty + #field_vis fn #get_field_name<'db>(self, __db: &'db #db_dyn_ty) -> &'db #field_ty { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar); @@ -102,7 +103,7 @@ impl InputStruct { } } else { parse_quote! { - pub fn #get_field_name<'db>(self, __db: &'db #db_dyn_ty) -> #field_ty + #field_vis fn #get_field_name<'db>(self, __db: &'db #db_dyn_ty) -> #field_ty { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar); @@ -114,9 +115,9 @@ impl InputStruct { .collect(); let set_field_names = self.all_set_field_names(); - let field_setters: Vec = field_indices.iter().zip(&set_field_names).zip(&field_tys).map(|((field_index, set_field_name), field_ty)| { + let field_setters: Vec = field_indices.iter().zip(&set_field_names).zip(&field_vises).zip(&field_tys).map(|(((field_index, set_field_name), field_vis), field_ty)| { parse_quote! { - pub fn #set_field_name<'db>(self, __db: &'db mut #db_dyn_ty) -> salsa::setter::Setter<'db, #ident, #field_ty> + #field_vis fn #set_field_name<'db>(self, __db: &'db mut #db_dyn_ty) -> salsa::setter::Setter<'db, #ident, #field_ty> { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar_mut(__db); let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient_mut(__jar); From 3df74dc0ac4d9060cb501e8178a908e93755095e Mon Sep 17 00:00:00 2001 From: Mihail Mihov Date: Wed, 31 Aug 2022 23:48:53 +0300 Subject: [PATCH 3/6] Use visibility of `field` in `SalsaField` in `#[salsa::tracked]` struct getters and setters --- components/salsa-2022-macros/src/tracked_struct.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/components/salsa-2022-macros/src/tracked_struct.rs b/components/salsa-2022-macros/src/tracked_struct.rs index dcc66c74..8e654a58 100644 --- a/components/salsa-2022-macros/src/tracked_struct.rs +++ b/components/salsa-2022-macros/src/tracked_struct.rs @@ -91,11 +91,12 @@ impl TrackedStruct { let id_field_names: Vec<_> = self.id_fields().map(SalsaField::name).collect(); let id_field_get_names: Vec<_> = self.id_fields().map(SalsaField::get_name).collect(); let id_field_tys: Vec<_> = self.id_fields().map(SalsaField::ty).collect(); + let id_field_vises: Vec<_> = self.id_fields().map(SalsaField::vis).collect(); let id_field_clones: Vec<_> = self.id_fields().map(SalsaField::is_clone_field).collect(); - let id_field_getters: Vec = id_field_indices.iter().zip(&id_field_get_names).zip(&id_field_tys).zip(&id_field_clones).map(|(((field_index, field_get_name), field_ty), is_clone_field)| + let id_field_getters: Vec = id_field_indices.iter().zip(&id_field_get_names).zip(&id_field_tys).zip(&id_field_vises).zip(&id_field_clones).map(|((((field_index, field_get_name), field_ty), field_vis), is_clone_field)| if !*is_clone_field { parse_quote! { - pub fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> &'db #field_ty + #field_vis fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> &'db #field_ty { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar); @@ -104,7 +105,7 @@ impl TrackedStruct { } } else { parse_quote! { - pub fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> #field_ty + #field_vis fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> #field_ty { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar); @@ -117,16 +118,17 @@ impl TrackedStruct { let value_field_indices = self.value_field_indices(); let value_field_names: Vec<_> = self.value_fields().map(SalsaField::name).collect(); + let value_field_vises: Vec<_> = self.value_fields().map(SalsaField::vis).collect(); let value_field_tys: Vec<_> = self.value_fields().map(SalsaField::ty).collect(); let value_field_get_names: Vec<_> = self.value_fields().map(SalsaField::get_name).collect(); let value_field_clones: Vec<_> = self .value_fields() .map(SalsaField::is_clone_field) .collect(); - let value_field_getters: Vec = value_field_indices.iter().zip(&value_field_get_names).zip(&value_field_tys).zip(&value_field_clones).map(|(((field_index, field_get_name), field_ty), is_clone_field)| + let value_field_getters: Vec = value_field_indices.iter().zip(&value_field_get_names).zip(&value_field_tys).zip(&value_field_vises).zip(&value_field_clones).map(|((((field_index, field_get_name), field_ty), field_vis), is_clone_field)| if !*is_clone_field { parse_quote! { - pub fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> &'db #field_ty + #field_vis fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> &'db #field_ty { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar); @@ -135,7 +137,7 @@ impl TrackedStruct { } } else { parse_quote! { - pub fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> #field_ty + #field_vis fn #field_get_name<'db>(self, __db: &'db #db_dyn_ty) -> #field_ty { let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db); let __ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #ident >>::ingredient(__jar); From 55f20344d1f713604585c92a488fc93f39deef8b Mon Sep 17 00:00:00 2001 From: Mihail Mihov Date: Wed, 31 Aug 2022 23:49:09 +0300 Subject: [PATCH 4/6] Use visibility of `field` in `SalsaField` in `#[salsa::interned]` getters --- components/salsa-2022-macros/src/interned.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/salsa-2022-macros/src/interned.rs b/components/salsa-2022-macros/src/interned.rs index f3a4ab85..587a4e52 100644 --- a/components/salsa-2022-macros/src/interned.rs +++ b/components/salsa-2022-macros/src/interned.rs @@ -90,10 +90,11 @@ impl InternedStruct { .map(|field| { let field_name = field.name(); let field_ty = field.ty(); + let field_vis = field.vis(); let field_get_name = field.get_name(); if field.is_clone_field() { parse_quote! { - #vis fn #field_get_name(self, db: &#db_dyn_ty) -> #field_ty { + #field_vis fn #field_get_name(self, db: &#db_dyn_ty) -> #field_ty { let (jar, runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #id_ident >>::ingredient(jar); std::clone::Clone::clone(&ingredients.data(runtime, self).#field_name) @@ -101,7 +102,7 @@ impl InternedStruct { } } else { parse_quote! { - #vis fn #field_get_name<'db>(self, db: &'db #db_dyn_ty) -> &'db #field_ty { + #field_vis fn #field_get_name<'db>(self, db: &'db #db_dyn_ty) -> &'db #field_ty { let (jar, runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #id_ident >>::ingredient(jar); &ingredients.data(runtime, self).#field_name From 13facddde2eeb40cfafc31caf65d084dc3d76bac Mon Sep 17 00:00:00 2001 From: Mihail Mihov Date: Thu, 1 Sep 2022 00:28:01 +0300 Subject: [PATCH 5/6] Fix field visibilities in `calc-example` --- calc-example/calc/src/ir.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/calc-example/calc/src/ir.rs b/calc-example/calc/src/ir.rs index fe8c24dd..3c2a3aea 100644 --- a/calc-example/calc/src/ir.rs +++ b/calc-example/calc/src/ir.rs @@ -7,7 +7,7 @@ use ordered_float::OrderedFloat; #[salsa::input] pub struct SourceProgram { #[return_ref] - text: String, + pub text: String, } // ANCHOR_END: input @@ -29,7 +29,7 @@ pub struct FunctionId { #[salsa::tracked] pub struct Program { #[return_ref] - statements: Vec, + pub statements: Vec, } // ANCHOR_END: program @@ -77,15 +77,15 @@ pub enum Op { #[salsa::tracked] pub struct Function { #[id] - name: FunctionId, + pub name: FunctionId, name_span: Span, #[return_ref] - args: Vec, + pub args: Vec, #[return_ref] - body: Expression, + pub body: Expression, } // ANCHOR_END: functions From 0a3b74da57d742a80017a1d2e156fccb22bbe737 Mon Sep 17 00:00:00 2001 From: Mihail Mihov Date: Fri, 2 Sep 2022 03:19:21 +0300 Subject: [PATCH 6/6] Add compile-fail test for getter and setter visibility --- .../compile-fail/get-set-on-private-field.rs | 31 +++++++++++++++++++ .../get-set-on-private-field.stderr | 17 ++++++++++ 2 files changed, 48 insertions(+) create mode 100644 salsa-2022-tests/tests/compile-fail/get-set-on-private-field.rs create mode 100644 salsa-2022-tests/tests/compile-fail/get-set-on-private-field.stderr diff --git a/salsa-2022-tests/tests/compile-fail/get-set-on-private-field.rs b/salsa-2022-tests/tests/compile-fail/get-set-on-private-field.rs new file mode 100644 index 00000000..d8e9bbba --- /dev/null +++ b/salsa-2022-tests/tests/compile-fail/get-set-on-private-field.rs @@ -0,0 +1,31 @@ +#[salsa::jar(db = Db)] +pub struct Jar(a::MyInput); + +mod a { + use crate::Jar; + + #[salsa::input(jar = Jar)] + pub struct MyInput { + field: u32, + } +} + +pub trait Db: salsa::DbWithJar {} + +#[salsa::db(Jar)] +#[derive(Default)] +struct Database { + storage: salsa::Storage, +} + +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); +} diff --git a/salsa-2022-tests/tests/compile-fail/get-set-on-private-field.stderr b/salsa-2022-tests/tests/compile-fail/get-set-on-private-field.stderr new file mode 100644 index 00000000..8d547292 --- /dev/null +++ b/salsa-2022-tests/tests/compile-fail/get-set-on-private-field.stderr @@ -0,0 +1,17 @@ +error[E0624]: associated function `field` is private + --> tests/compile-fail/get-set-on-private-field.rs:29:11 + | +7 | #[salsa::input(jar = Jar)] + | -------------------------- private associated function defined here +... +29 | input.field(&db); + | ^^^^^ private associated function + +error[E0624]: associated function `set_field` is private + --> tests/compile-fail/get-set-on-private-field.rs:30:11 + | +7 | #[salsa::input(jar = Jar)] + | -------------------------- private associated function defined here +... +30 | input.set_field(&mut db).to(23); + | ^^^^^^^^^ private associated function