Skip to content

Commit

Permalink
Merge pull request #483 from nikomatsakis/salsa-2022-spans
Browse files Browse the repository at this point in the history
improve spans for getters, constructors
  • Loading branch information
nikomatsakis authored Apr 4, 2024
2 parents bdf7d9c + 5d6f883 commit 45fd941
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 27 deletions.
10 changes: 5 additions & 5 deletions components/salsa-2022-macros/src/accumulator.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use syn::ItemStruct;
use syn::{spanned::Spanned, ItemStruct};

// #[salsa::accumulator(jar = Jar0)]
// struct Accumulator(DataType);
Expand Down Expand Up @@ -86,15 +86,15 @@ fn struct_item_out(
data_ty: &syn::Type,
) -> syn::ItemStruct {
let mut struct_item_out = struct_item.clone();
struct_item_out.fields = syn::Fields::Unnamed(parse_quote! {
struct_item_out.fields = syn::Fields::Unnamed(parse_quote_spanned! { data_ty.span() =>
(std::marker::PhantomData<#data_ty>)
});
struct_item_out
}

fn inherent_impl(args: &Args, struct_ty: &syn::Type, data_ty: &syn::Type) -> syn::ItemImpl {
let jar_ty = args.jar_ty();
parse_quote! {
parse_quote_spanned! { struct_ty.span() =>
impl #struct_ty {
pub fn push<DB: ?Sized>(db: &DB, data: #data_ty)
where
Expand All @@ -115,7 +115,7 @@ fn ingredients_for_impl(
) -> syn::ItemImpl {
let jar_ty = args.jar_ty();
let debug_name = crate::literal(struct_name);
parse_quote! {
parse_quote_spanned! { struct_name.span() =>
impl salsa::storage::IngredientsFor for #struct_name {
type Ingredients = salsa::accumulator::AccumulatorIngredient<#data_ty>;
type Jar = #jar_ty;
Expand All @@ -142,7 +142,7 @@ fn ingredients_for_impl(

fn accumulator_impl(args: &Args, struct_ty: &syn::Type, data_ty: &syn::Type) -> syn::ItemImpl {
let jar_ty = args.jar_ty();
parse_quote! {
parse_quote_spanned! { struct_ty.span() =>
impl salsa::accumulator::Accumulator for #struct_ty {
type Data = #data_ty;
type Jar = #jar_ty;
Expand Down
12 changes: 7 additions & 5 deletions components/salsa-2022-macros/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl InputStruct {
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_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! {
parse_quote_spanned! { get_field_name.span() =>
#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);
Expand All @@ -90,7 +90,7 @@ impl InputStruct {
}
}
} else {
parse_quote! {
parse_quote_spanned! { get_field_name.span() =>
#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);
Expand All @@ -110,7 +110,7 @@ impl InputStruct {
.zip(&field_tys)
.filter_map(|(((field_index, &set_field_name), field_vis), field_ty)| {
let set_field_name = set_field_name?;
Some(parse_quote! {
Some(parse_quote_spanned! { set_field_name.span() =>
#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);
Expand All @@ -125,7 +125,7 @@ impl InputStruct {
let singleton = self.0.is_isingleton();

let constructor: syn::ImplItemMethod = if singleton {
parse_quote! {
parse_quote_spanned! { constructor_name.span() =>
/// Creates a new singleton input
///
/// # Panics
Expand All @@ -143,7 +143,7 @@ impl InputStruct {
}
}
} else {
parse_quote! {
parse_quote_spanned! { constructor_name.span() =>
pub fn #constructor_name(__db: &#db_dyn_ty, #(#field_names: #field_tys,)*) -> Self
{
let (__jar, __runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(__db);
Expand Down Expand Up @@ -177,6 +177,7 @@ impl InputStruct {
};

parse_quote! {
#[allow(dead_code)]
impl #ident {
#constructor

Expand All @@ -191,6 +192,7 @@ impl InputStruct {
}
} else {
parse_quote! {
#[allow(dead_code)]
impl #ident {
#constructor

Expand Down
7 changes: 4 additions & 3 deletions components/salsa-2022-macros/src/interned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ impl InternedStruct {
let field_vis = field.vis();
let field_get_name = field.get_name();
if field.is_clone_field() {
parse_quote! {
parse_quote_spanned! { field_get_name.span() =>
#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)
}
}
} else {
parse_quote! {
parse_quote_spanned! { field_get_name.span() =>
#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);
Expand All @@ -117,7 +117,7 @@ impl InternedStruct {
let field_tys = self.all_field_tys();
let data_ident = self.data_ident();
let constructor_name = self.constructor_name();
let new_method: syn::ImplItemMethod = parse_quote! {
let new_method: syn::ImplItemMethod = parse_quote_spanned! { constructor_name.span() =>
#vis fn #constructor_name(
db: &#db_dyn_ty,
#(#field_names: #field_tys,)*
Expand All @@ -131,6 +131,7 @@ impl InternedStruct {
};

parse_quote! {
#[allow(dead_code)]
impl #id_ident {
#(#field_getters)*

Expand Down
12 changes: 6 additions & 6 deletions components/salsa-2022-macros/src/salsa_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ impl<A: AllowedOptions> SalsaStruct<A> {
.filter(|attr| !attr.path.is_ident("customize"))
.collect();

parse_quote! {
parse_quote_spanned! { ident.span() =>
#(#attrs)*
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
#visibility struct #ident(salsa::Id);
Expand All @@ -206,7 +206,7 @@ impl<A: AllowedOptions> SalsaStruct<A> {
let visibility = self.visibility();
let all_field_names = self.all_field_names();
let all_field_tys = self.all_field_tys();
parse_quote! {
parse_quote_spanned! { ident.span() =>
/// Internal struct used for interned item
#[derive(Eq, PartialEq, Hash, Clone)]
#visibility struct #ident {
Expand All @@ -226,14 +226,14 @@ impl<A: AllowedOptions> SalsaStruct<A> {
pub(crate) fn constructor_name(&self) -> syn::Ident {
match self.args.constructor_name.clone() {
Some(name) => name,
None => Ident::new("new", Span::call_site()),
None => Ident::new("new", self.id_ident().span()),
}
}

/// Generate `impl salsa::AsId for Foo`
pub(crate) fn as_id_impl(&self) -> syn::ItemImpl {
let ident = self.id_ident();
parse_quote! {
parse_quote_spanned! { ident.span() =>
impl salsa::AsId for #ident {
fn as_id(self) -> salsa::Id {
self.0
Expand Down Expand Up @@ -352,8 +352,8 @@ impl SalsaField {
));
}

let get_name = Ident::new(&field_name_str, Span::call_site());
let set_name = Ident::new(&format!("set_{}", field_name_str), Span::call_site());
let get_name = Ident::new(&field_name_str, field_name.span());
let set_name = Ident::new(&format!("set_{}", field_name_str), field_name.span());
let mut result = SalsaField {
field: field.clone(),
has_id_attr: false,
Expand Down
4 changes: 2 additions & 2 deletions components/salsa-2022-macros/src/tracked_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl TrackedStruct {
let field_clones: Vec<_> = self.all_fields().map(SalsaField::is_clone_field).collect();
let field_getters: Vec<syn::ImplItemMethod> = field_indices.iter().zip(&field_get_names).zip(&field_tys).zip(&field_vises).zip(&field_clones).map(|((((field_index, field_get_name), field_ty), field_vis), is_clone_field)|
if !*is_clone_field {
parse_quote! {
parse_quote_spanned! { field_get_name.span() =>
#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);
Expand All @@ -177,7 +177,7 @@ impl TrackedStruct {
}
}
} else {
parse_quote! {
parse_quote_spanned! { field_get_name.span() =>
#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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
error[E0624]: method `field` is private
--> tests/compile-fail/get-set-on-private-field.rs:29:11
|
7 | #[salsa::input(jar = Jar)]
| -------------------------- private method defined here
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
|
7 | #[salsa::input(jar = Jar)]
| -------------------------- private method defined here
9 | field: u32,
| ----- private method defined here
...
30 | input.set_field(&mut db).to(23);
| ^^^^^^^^^ private method
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0599]: no method named `set_id_one` found for struct `MyInput` in the current scope
--> tests/compile-fail/input_struct_id_fields_no_setters.rs:30:11
|
7 | #[salsa::input(jar = Jar)]
| -------------------------- method `set_id_one` not found for this struct
8 | struct MyInput {
| ------- method `set_id_one` not found for this struct
...
30 | input.set_id_one(1);
| ^^^^^^^^^^ help: there is a method with a similar name: `id_one`
27 changes: 27 additions & 0 deletions salsa-2022-tests/tests/compile-fail/span-input-setter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#[salsa::jar(db = Db)]
pub struct Jar(MyInput);

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 {}

#[salsa::input]
pub struct MyInput {
field: u32,
}

fn main() {
let mut db = Database::default();
let input = MyInput::new(&mut db, 22);

input.field(&db);
input.set_field(22);
}
18 changes: 18 additions & 0 deletions salsa-2022-tests/tests/compile-fail/span-input-setter.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0308]: mismatched types
--> tests/compile-fail/span-input-setter.rs:26:21
|
26 | input.set_field(22);
| --------- ^^ expected `&mut dyn Db`, found integer
| |
| arguments to this method are incorrect
|
= note: expected mutable reference `&mut dyn Db`
found type `{integer}`
note: method defined here
--> tests/compile-fail/span-input-setter.rs:18:5
|
16 | #[salsa::input]
| ---------------
17 | pub struct MyInput {
18 | field: u32,
| ^^^^^
30 changes: 30 additions & 0 deletions salsa-2022-tests/tests/compile-fail/span-tracked-getter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#[salsa::jar(db = Db)]
pub struct Jar(MyTracked, my_fn);

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 {}

#[salsa::tracked]
pub struct MyTracked {
field: u32,
}

#[salsa::tracked]
fn my_fn(db: &dyn crate::Db) {
let x = MyTracked::new(db, 22);
x.field(22);
}

fn main() {
let mut db = Database::default();
my_fn(&db);
}
18 changes: 18 additions & 0 deletions salsa-2022-tests/tests/compile-fail/span-tracked-getter.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0308]: mismatched types
--> tests/compile-fail/span-tracked-getter.rs:24:13
|
24 | x.field(22);
| ----- ^^ expected `&dyn Db`, found integer
| |
| arguments to this method are incorrect
|
= note: expected reference `&dyn Db`
found type `{integer}`
note: method defined here
--> tests/compile-fail/span-tracked-getter.rs:18:5
|
16 | #[salsa::tracked]
| -----------------
17 | pub struct MyTracked {
18 | field: u32,
| ^^^^^

0 comments on commit 45fd941

Please sign in to comment.