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

improve spans for getters, constructors #483

Merged
merged 3 commits into from
Apr 4, 2024
Merged
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
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 @@ -76,7 +76,7 @@
// FIXME: this should be a comma separated list but I couldn't
// be bothered to remember how syn does this.
let args: syn::Ident = attr.parse_args()?;
if args.to_string() == "DebugWithDb" {

Check warning on line 79 in components/salsa-2022-macros/src/salsa_struct.rs

View workflow job for this annotation

GitHub Actions / Test (nightly, true)

this creates an owned instance just for comparison

Check warning on line 79 in components/salsa-2022-macros/src/salsa_struct.rs

View workflow job for this annotation

GitHub Actions / Test (stable, false)

this creates an owned instance just for comparison

Check warning on line 79 in components/salsa-2022-macros/src/salsa_struct.rs

View workflow job for this annotation

GitHub Actions / Test (beta, false)

this creates an owned instance just for comparison
Ok(vec![Customization::DebugWithDb])
} else {
Err(syn::Error::new_spanned(args, "unrecognized customization"))
Expand Down Expand Up @@ -188,7 +188,7 @@
.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 @@
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 @@
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 @@
));
}

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,
| ^^^^^
Loading