Skip to content

Commit

Permalink
Merge pull request #482 from nikomatsakis/debug-with-db-improvements
Browse files Browse the repository at this point in the history
[breaking-salsa-2022] Debug with db: ignore deps, support customization
  • Loading branch information
nikomatsakis authored Apr 4, 2024
2 parents b5aa429 + ce2f782 commit bdf7d9c
Show file tree
Hide file tree
Showing 16 changed files with 332 additions and 210 deletions.
6 changes: 2 additions & 4 deletions components/salsa-2022-macros/src/input.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::salsa_struct::{SalsaField, SalsaStruct, SalsaStructKind};
use crate::salsa_struct::{SalsaField, SalsaStruct};
use proc_macro2::{Literal, TokenStream};

/// For an entity struct `Foo` with fields `f1: T1, ..., fN: TN`, we generate...
Expand All @@ -10,9 +10,7 @@ pub(crate) fn input(
args: proc_macro::TokenStream,
input: proc_macro::TokenStream,
) -> proc_macro::TokenStream {
match SalsaStruct::new(SalsaStructKind::Input, args, input)
.and_then(|el| InputStruct(el).generate_input())
{
match SalsaStruct::new(args, input).and_then(|el| InputStruct(el).generate_input()) {
Ok(s) => s.into(),
Err(err) => err.into_compile_error().into(),
}
Expand Down
6 changes: 2 additions & 4 deletions components/salsa-2022-macros/src/interned.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::salsa_struct::{SalsaStruct, SalsaStructKind};
use crate::salsa_struct::SalsaStruct;
use proc_macro2::TokenStream;

// #[salsa::interned(jar = Jar0, data = TyData0)]
Expand All @@ -13,9 +13,7 @@ pub(crate) fn interned(
args: proc_macro::TokenStream,
input: proc_macro::TokenStream,
) -> proc_macro::TokenStream {
match SalsaStruct::new(SalsaStructKind::Interned, args, input)
.and_then(|el| InternedStruct(el).generate_interned())
{
match SalsaStruct::new(args, input).and_then(|el| InternedStruct(el).generate_interned()) {
Ok(s) => s.into(),
Err(err) => err.into_compile_error().into(),
}
Expand Down
88 changes: 48 additions & 40 deletions components/salsa-2022-macros/src/salsa_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,50 +29,72 @@ use crate::options::{AllowedOptions, Options};
use proc_macro2::{Ident, Span, TokenStream};
use syn::spanned::Spanned;

pub(crate) enum SalsaStructKind {
Input,
Tracked,
Interned,
}

pub(crate) struct SalsaStruct<A: AllowedOptions> {
kind: SalsaStructKind,
args: Options<A>,
struct_item: syn::ItemStruct,
customizations: Vec<Customization>,
fields: Vec<SalsaField>,
}

#[derive(PartialEq, Eq, Debug, Copy, Clone)]
pub enum Customization {
DebugWithDb,
}

const BANNED_FIELD_NAMES: &[&str] = &["from", "new"];

impl<A: AllowedOptions> SalsaStruct<A> {
pub(crate) fn new(
kind: SalsaStructKind,
args: proc_macro::TokenStream,
input: proc_macro::TokenStream,
) -> syn::Result<Self> {
let struct_item = syn::parse(input)?;
Self::with_struct(kind, args, struct_item)
Self::with_struct(args, struct_item)
}

pub(crate) fn with_struct(
kind: SalsaStructKind,
args: proc_macro::TokenStream,
struct_item: syn::ItemStruct,
) -> syn::Result<Self> {
let args: Options<A> = syn::parse(args)?;
let fields = Self::extract_options(&struct_item)?;
let customizations = Self::extract_customizations(&struct_item)?;
let fields = Self::extract_fields(&struct_item)?;
Ok(Self {
kind,
args,
struct_item,
customizations,
fields,
})
}

fn extract_customizations(struct_item: &syn::ItemStruct) -> syn::Result<Vec<Customization>> {
Ok(struct_item
.attrs
.iter()
.map(|attr| {
if attr.path.is_ident("customize") {
// 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 (beta, 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 (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
Ok(vec![Customization::DebugWithDb])
} else {
Err(syn::Error::new_spanned(args, "unrecognized customization"))
}
} else {
Ok(vec![])
}
})
.collect::<Result<Vec<Vec<_>>, _>>()?
.into_iter()
.flatten()
.collect())
}

/// Extract out the fields and their options:
/// If this is a struct, it must use named fields, so we can define field accessors.
/// If it is an enum, then this is not necessary.
pub(crate) fn extract_options(struct_item: &syn::ItemStruct) -> syn::Result<Vec<SalsaField>> {
fn extract_fields(struct_item: &syn::ItemStruct) -> syn::Result<Vec<SalsaField>> {
match &struct_item.fields {
syn::Fields::Named(n) => Ok(n
.named
Expand All @@ -93,13 +115,6 @@ impl<A: AllowedOptions> SalsaStruct<A> {
self.fields.iter()
}

pub(crate) fn is_identity_field(&self, field: &SalsaField) -> bool {
match self.kind {
SalsaStructKind::Input | SalsaStructKind::Tracked => field.has_id_attr,
SalsaStructKind::Interned => true,
}
}

/// Names of all fields (id and value).
///
/// If this is an enum, empty vec.
Expand Down Expand Up @@ -163,12 +178,14 @@ impl<A: AllowedOptions> SalsaStruct<A> {
let ident = self.id_ident();
let visibility = &self.struct_item.vis;

// Extract the attributes the user gave, but screen out derive, since we are adding our own.
// Extract the attributes the user gave, but screen out derive, since we are adding our own,
// and the customize attribute that we use for our own purposes.
let attrs: Vec<_> = self
.struct_item
.attrs
.iter()
.filter(|attr| !attr.path.is_ident("derive"))
.filter(|attr| !attr.path.is_ident("customize"))
.collect();

parse_quote! {
Expand Down Expand Up @@ -231,50 +248,41 @@ impl<A: AllowedOptions> SalsaStruct<A> {
}

/// Generate `impl salsa::DebugWithDb for Foo`
pub(crate) fn as_debug_with_db_impl(&self) -> syn::ItemImpl {
pub(crate) fn as_debug_with_db_impl(&self) -> Option<syn::ItemImpl> {
if self.customizations.contains(&Customization::DebugWithDb) {
return None;
}

let ident = self.id_ident();

let db_type = self.db_dyn_ty();
let ident_string = ident.to_string();

// `::salsa::debug::helper::SalsaDebug` will use `DebugWithDb` or fallbak to `Debug`
// `::salsa::debug::helper::SalsaDebug` will use `DebugWithDb` or fallback to `Debug`
let fields = self
.all_fields()
.map(|field| -> TokenStream {
let field_name_string = field.name().to_string();
let field_getter = field.get_name();
let field_ty = field.ty();

let field_debug = quote_spanned! { field.field.span() =>
quote_spanned! { field.field.span() =>
debug_struct = debug_struct.field(
#field_name_string,
&::salsa::debug::helper::SalsaDebug::<#field_ty, #db_type>::salsa_debug(
#[allow(clippy::needless_borrow)]
&self.#field_getter(_db),
_db,
_include_all_fields
)
);
};

if self.is_identity_field(field) {
quote_spanned! { field.field.span() =>
#field_debug
}
} else {
quote_spanned! { field.field.span() =>
if _include_all_fields {
#field_debug
}
}
}
})
.collect::<TokenStream>();

// `use ::salsa::debug::helper::Fallback` is needed for the fallback to `Debug` impl
parse_quote_spanned! {ident.span()=>
Some(parse_quote_spanned! {ident.span()=>
impl ::salsa::DebugWithDb<#db_type> for #ident {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>, _db: &#db_type, _include_all_fields: bool) -> ::std::fmt::Result {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>, _db: &#db_type) -> ::std::fmt::Result {
#[allow(unused_imports)]
use ::salsa::debug::helper::Fallback;
let mut debug_struct = &mut f.debug_struct(#ident_string);
Expand All @@ -283,7 +291,7 @@ impl<A: AllowedOptions> SalsaStruct<A> {
debug_struct.finish()
}
}
}
})
}

/// Disallow `#[id]` attributes on the fields of this struct.
Expand Down
5 changes: 2 additions & 3 deletions components/salsa-2022-macros/src/tracked_struct.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use proc_macro2::{Literal, Span, TokenStream};

use crate::salsa_struct::{SalsaField, SalsaStruct, SalsaStructKind};
use crate::salsa_struct::{SalsaField, SalsaStruct};

/// For an tracked struct `Foo` with fields `f1: T1, ..., fN: TN`, we generate...
///
Expand All @@ -11,8 +11,7 @@ pub(crate) fn tracked(
args: proc_macro::TokenStream,
struct_item: syn::ItemStruct,
) -> syn::Result<TokenStream> {
SalsaStruct::with_struct(SalsaStructKind::Tracked, args, struct_item)
.and_then(|el| TrackedStruct(el).generate_tracked())
SalsaStruct::with_struct(args, struct_item).and_then(|el| TrackedStruct(el).generate_tracked())
}

struct TrackedStruct(SalsaStruct<Self>);
Expand Down
Loading

0 comments on commit bdf7d9c

Please sign in to comment.