From 940eed92a6bd32182e1e4b66f67405f56fe3e1d7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 5 May 2019 16:40:47 +0300 Subject: [PATCH 1/4] allow private requirements in query groups --- components/salsa-macros/src/lib.rs | 2 +- components/salsa-macros/src/query_group.rs | 67 ++++++++++++++++++---- tests/requires.rs | 64 +++++++++++++++++++++ 3 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 tests/requires.rs diff --git a/components/salsa-macros/src/lib.rs b/components/salsa-macros/src/lib.rs index 648c47e..ccd66a0 100644 --- a/components/salsa-macros/src/lib.rs +++ b/components/salsa-macros/src/lib.rs @@ -1,6 +1,6 @@ //! This crate provides salsa's macros and attributes. -#![recursion_limit = "128"] +#![recursion_limit = "256"] extern crate proc_macro; extern crate proc_macro2; diff --git a/components/salsa-macros/src/query_group.rs b/components/salsa-macros/src/query_group.rs index 31f8b2a..0864144 100644 --- a/components/salsa-macros/src/query_group.rs +++ b/components/salsa-macros/src/query_group.rs @@ -3,11 +3,19 @@ use heck::CamelCase; use proc_macro::TokenStream; use proc_macro2::Span; use quote::ToTokens; -use syn::{parse_macro_input, parse_quote, FnArg, Ident, ItemTrait, ReturnType, TraitItem, Type}; +use syn::parse::{Parse, ParseStream}; +use syn::punctuated::Punctuated; +use syn::{ + parse_macro_input, parse_quote, FnArg, Ident, ItemTrait, Lit, MetaNameValue, Path, TypeParamBound, + ReturnType, Token, TraitItem, Type, TraitBound, TraitBoundModifier +}; /// Implementation for `[salsa::query_group]` decorator. pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream { - let group_struct: Ident = parse_macro_input!(args as Ident); + let GroupDef { + group_struct, + requires, + } = parse_macro_input!(args as GroupDef); let input: ItemTrait = parse_macro_input!(input as ItemTrait); // println!("args: {:#?}", args); // println!("input: {:#?}", input); @@ -314,7 +322,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream impl salsa::plumbing::QueryGroup for #group_struct where - DB__: #trait_name, + DB__: #trait_name + #requires, DB__: salsa::plumbing::HasQueryGroup<#group_struct>, DB__: salsa::Database, { @@ -325,7 +333,15 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream // Emit an impl of the trait output.extend({ - let bounds = &input.supertraits; + let mut bounds = input.supertraits.clone(); + for path in requires.clone() { + bounds.push(TypeParamBound::Trait(TraitBound { + paren_token: None, + modifier: TraitBoundModifier::None, + lifetimes: None, + path, + })); + } quote! { impl #trait_name for T where @@ -365,7 +381,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream impl<#db> salsa::Query<#db> for #qt where - DB: #trait_name, + DB: #trait_name + #requires, DB: salsa::plumbing::HasQueryGroup<#group_struct>, DB: salsa::Database, { @@ -401,7 +417,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream output.extend(quote_spanned! {span=> impl salsa::plumbing::QueryFunction for #qt where - DB: #trait_name, + DB: #trait_name + #requires, DB: salsa::plumbing::HasQueryGroup<#group_struct>, DB: salsa::Database, { @@ -430,7 +446,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream revision: salsa::plumbing::Revision, ) -> bool where - DB__: #trait_name, + DB__: #trait_name + #requires, DB__: salsa::plumbing::HasQueryGroup<#group_struct>, { match self { @@ -456,7 +472,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream output.extend(quote! { #trait_vis struct #group_storage where - DB__: #trait_name, + DB__: #trait_name + #requires, DB__: salsa::plumbing::HasQueryGroup<#group_struct>, DB__: salsa::Database, { @@ -465,7 +481,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream impl Default for #group_storage where - DB__: #trait_name, + DB__: #trait_name + #requires, DB__: salsa::plumbing::HasQueryGroup<#group_struct>, DB__: salsa::Database, { @@ -479,7 +495,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream impl #group_storage where - DB__: #trait_name, + DB__: #trait_name + #requires, DB__: salsa::plumbing::HasQueryGroup<#group_struct>, { #trait_vis fn for_each_query( @@ -509,6 +525,37 @@ fn is_not_salsa_attr_path(path: &syn::Path) -> bool { || path.segments.len() != 2 } +#[derive(Debug)] +struct GroupDef { + group_struct: Ident, + requires: Punctuated, +} + +impl Parse for GroupDef { + fn parse(input: ParseStream) -> syn::Result { + let res = GroupDef { + group_struct: input.parse()?, + requires: { + if input.lookahead1().peek(Token![,]) { + input.parse::()?; + let name_value: MetaNameValue = input.parse()?; + if name_value.ident != "requires" { + return Err(syn::Error::new_spanned(name_value, "invalid attribute")); + } + let str_lit = match name_value.lit { + Lit::Str(it) => it, + _ => return Err(syn::Error::new_spanned(name_value, "invalid attribute")), + }; + str_lit.parse_with(Punctuated::::parse_separated_nonempty)? + } else { + Punctuated::new() + } + }, + }; + Ok(res) + } +} + #[derive(Debug)] struct Query { fn_name: Ident, diff --git a/tests/requires.rs b/tests/requires.rs new file mode 100644 index 0000000..b4c2630 --- /dev/null +++ b/tests/requires.rs @@ -0,0 +1,64 @@ +//! Test that transparent (uncached) queries work + + +mod queries { + #[salsa::query_group(InputGroupStorage)] + pub trait InputGroup { + #[salsa::input] + fn input(&self, x: u32) -> u32; + } + + #[salsa::query_group(PrivGroupAStorage)] + pub trait PrivGroupA: InputGroup { + fn private_a(&self, x: u32) -> u32; + } + + fn private_a(db: &impl PrivGroupA, x: u32) -> u32{ + db.input(x) + } + + #[salsa::query_group(PrivGroupBStorage)] + pub trait PrivGroupB: InputGroup { + fn private_b(&self, x: u32) -> u32; + } + + fn private_b(db: &impl PrivGroupB, x: u32) -> u32{ + db.input(x) + } + + #[salsa::query_group(PubGroupStorage, requires = "PrivGroupA + PrivGroupB")] + pub trait PubGroup: InputGroup { + fn public(&self, x: u32) -> u32; + } + + + fn public(db: &(impl PubGroup + PrivGroupA + PrivGroupB), x: u32) -> u32 { + db.private_a(x) + db.private_b(x) + } +} + +#[salsa::database( + queries::InputGroupStorage, + queries::PrivGroupAStorage, + queries::PrivGroupBStorage, + queries::PubGroupStorage, +)] +#[derive(Default)] +struct Database { + runtime: salsa::Runtime, +} + +impl salsa::Database for Database { + fn salsa_runtime(&self) -> &salsa::Runtime { + &self.runtime + } +} + +#[test] +fn require_clauses_work() { + use queries::{InputGroup, PubGroup}; + let mut db = Database::default(); + + db.set_input(1, 10); + assert_eq!(db.public(1), 20); +} From c816df7208858c08fe195b138cfd1f38992c0938 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 21 May 2019 18:30:19 +0300 Subject: [PATCH 2/4] extract attribute filtering --- components/salsa-macros/src/query_group.rs | 55 ++++++++++++++++------ 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/components/salsa-macros/src/query_group.rs b/components/salsa-macros/src/query_group.rs index 0864144..1392a29 100644 --- a/components/salsa-macros/src/query_group.rs +++ b/components/salsa-macros/src/query_group.rs @@ -1,3 +1,5 @@ +use std::convert::TryFrom; + use crate::parenthesized::Parenthesized; use heck::CamelCase; use proc_macro::TokenStream; @@ -6,8 +8,8 @@ use quote::ToTokens; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; use syn::{ - parse_macro_input, parse_quote, FnArg, Ident, ItemTrait, Lit, MetaNameValue, Path, TypeParamBound, - ReturnType, Token, TraitItem, Type, TraitBound, TraitBoundModifier + parse_macro_input, parse_quote, Attribute, FnArg, Ident, ItemTrait, Lit, MetaNameValue, Path, + ReturnType, Token, TraitBound, TraitBoundModifier, TraitItem, Type, TypeParamBound, }; /// Implementation for `[salsa::query_group]` decorator. @@ -38,19 +40,8 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream let mut num_storages = 0; // Extract attributes. - let mut attrs = vec![]; - for attr in method.attrs { - // Leave non-salsa attributes untouched. These are - // attributes that don't start with `salsa::` or don't have - // exactly two segments in their path. - if is_not_salsa_attr_path(&attr.path) { - attrs.push(attr); - continue; - } - - // Keep the salsa attributes around. - let name = attr.path.segments[1].ident.to_string(); - let tts = attr.tts.into(); + let (attrs, salsa_attrs) = filter_attrs(method.attrs); + for SalsaAttr { name, tts } in salsa_attrs { match name.as_str() { "memoized" => { storage = QueryStorage::Memoized; @@ -517,6 +508,24 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream output.into() } +struct SalsaAttr { + name: String, + tts: TokenStream, +} + +impl TryFrom for SalsaAttr { + type Error = syn::Attribute; + fn try_from(attr: syn::Attribute) -> Result { + if is_not_salsa_attr_path(&attr.path) { + return Err(attr); + } + + let name = attr.path.segments[1].ident.to_string(); + let tts = attr.tts.into(); + Ok(SalsaAttr { name, tts }) + } +} + fn is_not_salsa_attr_path(path: &syn::Path) -> bool { path.segments .first() @@ -525,6 +534,22 @@ fn is_not_salsa_attr_path(path: &syn::Path) -> bool { || path.segments.len() != 2 } +fn filter_attrs(attrs: Vec) -> (Vec, Vec) { + let mut other = vec![]; + let mut salsa = vec![]; + // Leave non-salsa attributes untouched. These are + // attributes that don't start with `salsa::` or don't have + // exactly two segments in their path. + // Keep the salsa attributes around. + for attr in attrs { + match SalsaAttr::try_from(attr) { + Ok(it) => salsa.push(it), + Err(it) => other.push(it), + } + } + (other, salsa) +} + #[derive(Debug)] struct GroupDef { group_struct: Ident, From 6ea5413ef5619c291251cccf2406e30f4386ef32 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 21 May 2019 18:49:18 +0300 Subject: [PATCH 3/4] switch requires syntax to an attribute --- components/salsa-macros/src/query_group.rs | 53 ++++++---------------- tests/requires.rs | 4 +- 2 files changed, 17 insertions(+), 40 deletions(-) diff --git a/components/salsa-macros/src/query_group.rs b/components/salsa-macros/src/query_group.rs index 1392a29..7ec8692 100644 --- a/components/salsa-macros/src/query_group.rs +++ b/components/salsa-macros/src/query_group.rs @@ -5,23 +5,30 @@ use heck::CamelCase; use proc_macro::TokenStream; use proc_macro2::Span; use quote::ToTokens; -use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; use syn::{ - parse_macro_input, parse_quote, Attribute, FnArg, Ident, ItemTrait, Lit, MetaNameValue, Path, + parse_macro_input, parse_quote, Attribute, FnArg, Ident, ItemTrait, Path, ReturnType, Token, TraitBound, TraitBoundModifier, TraitItem, Type, TypeParamBound, }; /// Implementation for `[salsa::query_group]` decorator. pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream { - let GroupDef { - group_struct, - requires, - } = parse_macro_input!(args as GroupDef); + let group_struct = parse_macro_input!(args as Ident); let input: ItemTrait = parse_macro_input!(input as ItemTrait); // println!("args: {:#?}", args); // println!("input: {:#?}", input); + let (trait_attrs, salsa_attrs) = filter_attrs(input.attrs); + let mut requires: Punctuated = Punctuated::new(); + for SalsaAttr { name, tts } in salsa_attrs { + match name.as_str() { + "requires" => { + requires.push(parse_macro_input!(tts as Parenthesized).0); + } + _ => panic!("unknown salsa attribute `{}`", name), + } + } + let trait_vis = input.vis; let trait_name = input.ident; let _generics = input.generics.clone(); @@ -296,10 +303,9 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream // Emit the trait itself. let mut output = { - let attrs = &input.attrs; let bounds = &input.supertraits; quote! { - #(#attrs)* + #(#trait_attrs)* #trait_vis trait #trait_name : #bounds { #query_fn_declarations } @@ -550,37 +556,6 @@ fn filter_attrs(attrs: Vec) -> (Vec, Vec) { (other, salsa) } -#[derive(Debug)] -struct GroupDef { - group_struct: Ident, - requires: Punctuated, -} - -impl Parse for GroupDef { - fn parse(input: ParseStream) -> syn::Result { - let res = GroupDef { - group_struct: input.parse()?, - requires: { - if input.lookahead1().peek(Token![,]) { - input.parse::()?; - let name_value: MetaNameValue = input.parse()?; - if name_value.ident != "requires" { - return Err(syn::Error::new_spanned(name_value, "invalid attribute")); - } - let str_lit = match name_value.lit { - Lit::Str(it) => it, - _ => return Err(syn::Error::new_spanned(name_value, "invalid attribute")), - }; - str_lit.parse_with(Punctuated::::parse_separated_nonempty)? - } else { - Punctuated::new() - } - }, - }; - Ok(res) - } -} - #[derive(Debug)] struct Query { fn_name: Ident, diff --git a/tests/requires.rs b/tests/requires.rs index b4c2630..edb3134 100644 --- a/tests/requires.rs +++ b/tests/requires.rs @@ -26,7 +26,9 @@ mod queries { db.input(x) } - #[salsa::query_group(PubGroupStorage, requires = "PrivGroupA + PrivGroupB")] + #[salsa::query_group(PubGroupStorage)] + #[salsa::requires(PrivGroupA)] + #[salsa::requires(PrivGroupB)] pub trait PubGroup: InputGroup { fn public(&self, x: u32) -> u32; } From ccc01cc189731fdd252622198720d8798ea5d43a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 30 May 2019 12:32:24 +0300 Subject: [PATCH 4/4] fix test doc comment --- tests/requires.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/requires.rs b/tests/requires.rs index edb3134..ea4502a 100644 --- a/tests/requires.rs +++ b/tests/requires.rs @@ -1,4 +1,5 @@ -//! Test that transparent (uncached) queries work +//! Test `salsa::requires` attribute for private query dependencies +//! https://github.com/salsa-rs/salsa-rfcs/pull/3 mod queries {