399: Make getters and setters have the visibility of their field r=nikomatsakis a=MihailMihov

resolves #398 

Currently `#[salsa::input]` and `#[salsa::tracked]` structs made all getters and setters public, regardless of the visibility of the field and the getters for `#[salsa::interned]` had the visibility of the struct itself. This pull request changes the generated impl's to use the visibility of each field for it's getters and setters.

Co-authored-by: Mihail Mihov <mmihov.personal@gmail.com>
This commit is contained in:
bors[bot] 2022-09-02 09:26:30 +00:00 committed by GitHub
commit 68cb5e9212
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 80 additions and 18 deletions

View file

@ -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<Statement>,
pub statements: Vec<Statement>,
}
// 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<VariableId>,
pub args: Vec<VariableId>,
#[return_ref]
body: Expression,
pub body: Expression,
}
// ANCHOR_END: functions

View file

@ -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<syn::ImplItemMethod> = 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<syn::ImplItemMethod> = 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<syn::ImplItemMethod> = field_indices.iter().zip(&set_field_names).zip(&field_tys).map(|((field_index, set_field_name), field_ty)| {
let field_setters: Vec<syn::ImplItemMethod> = 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);

View file

@ -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

View file

@ -96,6 +96,11 @@ impl<A: AllowedOptions> SalsaStruct<A> {
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

View file

@ -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<syn::ImplItemMethod> = 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<syn::ImplItemMethod> = 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<syn::ImplItemMethod> = 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<syn::ImplItemMethod> = 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);

View file

@ -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<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

@ -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