From 0ee6f3884d628f2b3df843bede65da406d5ebe77 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 5 Oct 2018 05:51:18 -0400 Subject: [PATCH] make `query_prototype` also define queries, remove `query_definition` --- README.md | 2 +- examples/compiler/class_table.rs | 17 +- examples/hello_world/README.md | 14 +- examples/hello_world/main.rs | 13 +- src/lib.rs | 249 ++++------------------- tests/cycles.rs | 20 +- tests/incremental/memoized_dep_inputs.rs | 24 +-- tests/incremental/memoized_inputs.rs | 19 +- tests/incremental/memoized_volatile.rs | 14 +- tests/storage_varieties/queries.rs | 18 +- 10 files changed, 101 insertions(+), 289 deletions(-) diff --git a/README.md b/README.md index f17259b7..7704e9fc 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ Using salsa is as easy as 1, 2, 3... and queries you will need. We'll start with one such trait, but later on you can use more than one to break up your system into components (or spread your code across crates). -2. **Implement the queries** using the `query_definition!` macro. +2. **Implement the query functions** where appropriate. 3. Define the **database struct**, which contains the storage for all the inputs/queries you will be using. The query struct will contain the storage for all of the inputs/queries and may also contain diff --git a/examples/compiler/class_table.rs b/examples/compiler/class_table.rs index ffc71eb2..0f809eb4 100644 --- a/examples/compiler/class_table.rs +++ b/examples/compiler/class_table.rs @@ -1,5 +1,5 @@ use crate::compiler; -use salsa::{query_definition, query_prototype}; +use salsa::query_prototype; use std::sync::Arc; query_prototype! { @@ -24,20 +24,20 @@ query_prototype! { #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub struct DefId(usize); -query_definition! { - pub AllClasses(_: &impl ClassTableDatabase, (): ()) -> Arc> { +impl salsa::QueryFunction for AllClasses { + fn execute(_: &DB, (): ()) -> Arc> { Arc::new(vec![DefId(0), DefId(10)]) // dummy impl } } -query_definition! { - pub Fields(_: &impl ClassTableDatabase, class: DefId) -> Arc> { +impl salsa::QueryFunction for Fields { + fn execute(_: &DB, class: DefId) -> Arc> { Arc::new(vec![DefId(class.0 + 1), DefId(class.0 + 2)]) // dummy impl } } -query_definition! { - pub AllFields(db: &impl ClassTableDatabase, (): ()) -> Arc> { +impl salsa::QueryFunction for AllFields { + fn execute(db: &DB, (): ()) -> Arc> { Arc::new( db.all_classes(()) .iter() @@ -45,8 +45,7 @@ query_definition! { .flat_map(|def_id| { let fields = db.fields(def_id); (0..fields.len()).map(move |i| fields[i]) - }) - .collect() + }).collect(), ) } } diff --git a/examples/hello_world/README.md b/examples/hello_world/README.md index fecc8b00..144c5222 100644 --- a/examples/hello_world/README.md +++ b/examples/hello_world/README.md @@ -20,18 +20,24 @@ example. It defines exactly two queries: `input_string` and names of the queries as methods (e.g., `input_string()`) and also a path to a type that will define the query (`InputString`). It doesn't give many other details: those are specified in the query definition -that comes later. +that comes later. XXX out of date ```rust salsa::query_prototype! { trait HelloWorldDatabase: salsa::Database { - fn input_string() for InputString; - fn length() for Length; + fn input_string(key: ()) -> Arc { + type InputString; + storage input; + } + + fn length(key: ()) -> usize { + type Length; + } } } ``` -### Step 2: Define the queries +### Step 2: Define the query bodies The actual query definitions are made using the `salsa::query_definition` macro. For an **input query**, such as diff --git a/examples/hello_world/main.rs b/examples/hello_world/main.rs index 50a3a148..aec21f6b 100644 --- a/examples/hello_world/main.rs +++ b/examples/hello_world/main.rs @@ -13,7 +13,9 @@ salsa::query_prototype! { trait HelloWorldDatabase: salsa::Database { fn input_string(key: ()) -> Arc { type InputString; + storage input; } + fn length(key: ()) -> usize { type Length; } @@ -23,21 +25,14 @@ salsa::query_prototype! { /////////////////////////////////////////////////////////////////////////// // Step 2. Define the queries. -// Define an **input query**. Like all queries, it is a map from a key -// (of type `()`) to a value (of type `Arc`). All values begin -// as `Default::default` but you can assign them new values. -salsa::query_definition! { - InputString: Map<(), Arc>; -} - // Define a **function query**. It too has a key and value type, but // it is defined with a function that -- given the key -- computes the // value. This function is supplied with a context (an `&impl // HelloWorldDatabase`) that gives access to other queries. The runtime // will track which queries you use so that we can incrementally // update memoized results. -salsa::query_definition! { - Length(db: &impl HelloWorldDatabase, _key: ()) -> usize { +impl salsa::QueryFunction for Length { + fn execute(db: &DB, _key: ()) -> usize { // Read the input string: let input_string = db.input_string(()); diff --git a/src/lib.rs b/src/lib.rs index 15a30fc4..b6b560dd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -208,14 +208,14 @@ impl DefaultKey for () { /// queries need to execute, as well as defining the queries /// themselves that are exported for others to use. /// -/// This macro declares the "prototype" for a single query. This will -/// expand into a `fn` item. This prototype specifies name of the -/// method (in the example below, that would be `my_query`) and -/// connects it to query definition type (`MyQuery`, in the example -/// below). These typically have the same name but a distinct -/// capitalization convention. Note that the actual input/output type -/// of the query are given only in the query definition (see the -/// `query_definition` macro for more details). +/// This macro declares the "prototype" for a group of queries. It will +/// expand into a trait and a set of structs, one per query. +/// +/// For each query, you give the name of the accessor method to invoke +/// the query (e.g., `my_query`, below), as well as its input/output +/// types. You also give the name for a query type (e.g., `MyQuery`, +/// below) that represents the query, and optionally other details, +/// such as its storage. /// /// ### Examples /// @@ -225,26 +225,10 @@ impl DefaultKey for () { /// trait TypeckDatabase { /// query_prototype! { /// /// Comments or other attributes can go here -/// fn my_query() for MyQuery; -/// } -/// } -/// ``` -/// -/// This just expands to something like: -/// -/// ```ignore -/// fn my_query(&self) -> QueryTable<'_, Self, $query_type>; -/// ``` -/// -/// This permits us to invoke the query via `query.my_query().of(key)`. -/// -/// You can also include more than one query if you prefer: -/// -/// ```ignore -/// trait TypeckDatabase { -/// query_prototype! { -/// fn my_query() for MyQuery; -/// fn my_other_query() for MyOtherQuery; +/// fn my_query(input: u32) -> u64 { +/// type MyQuery; +/// storage memoized; // optional, this is the default +/// } /// } /// } /// ``` @@ -273,25 +257,40 @@ macro_rules! query_prototype { // Base case: found the trait body ( attr[$($trait_attr:tt)*]; - headers[$v:vis, $name:ident, $($header:tt)*]; + headers[$v:vis, $query_trait:ident, $($header:tt)*]; tokens[{ $( $(#[$method_attr:meta])* - fn $method_name:ident($key_name:ident: $key:ty) -> $value:ty { - type $QueryType:ty; + fn $method_name:ident($key_name:ident: $key_ty:ty) -> $value_ty:ty { + type $QueryType:ident; + $(storage $storage:ident;)* // FIXME(rust-lang/rust#48075) should be `?` } )* }]; ) => { - $($trait_attr)* $v trait $name: $($crate::GetQueryTable<$QueryType> +)* $($header)* { + $($trait_attr)* $v trait $query_trait: $($crate::GetQueryTable<$QueryType> +)* $($header)* { $( $(#[$method_attr])* - fn $method_name(&self, key: $key) -> $value { + fn $method_name(&self, key: $key_ty) -> $value_ty { >::get_query_table(self) .get(key) } )* } + + $( + #[derive(Default, Debug)] + $v struct $QueryType; + + impl $crate::Query for $QueryType + where + DB: $query_trait, + { + type Key = $key_ty; + type Value = $value_ty; + type Storage = $crate::query_prototype! { @storage_ty[DB, Self, $($storage)*] }; + } + )* }; // Recursive case: found some more part of the trait header. @@ -307,143 +306,13 @@ macro_rules! query_prototype { tokens[$($tokens)*]; } }; -} -/// Creates a **Query Definition** type. This defines the input (key) -/// of the query, the output key (value), and the query context trait -/// that the query requires. -/// -/// Example: -/// -/// ```ignore -/// query_definition! { -/// pub MyQuery(db: &impl MyDatabase, key: MyKey) -> MyValue { -/// ... // fn body specifies what happens when query is invoked -/// } -/// } -/// ``` -/// -/// Here, the query context trait would be `MyDatabase` -- this -/// should be a trait containing all the other queries that the -/// definition needs to invoke (as well as any other methods that you -/// may want). -/// -/// The `MyKey` type is the **key** to the query -- it must be Clone, -/// Debug, Hash, Eq, and Send, as specified in the `Query` trait. -/// -/// The `MyKey` type is the **value** to the query -- it too must be -/// Clone, Debug, Hash, Eq, and Send, as specified in the `Query` -/// trait. -#[macro_export] -macro_rules! query_definition { - // Step 1. Filtering the attributes to look for the special ones - // we consume. + // Generate storage type ( - @filter_attrs { - input { #[storage(memoized)] $($input:tt)* }; - storage { $storage:tt }; - other_attrs { $($other_attrs:tt)* }; - } + // Default case: + @storage_ty[$DB:ident, $Self:ident, ] ) => { - $crate::query_definition! { - @filter_attrs { - input { $($input)* }; - storage { memoized }; - other_attrs { $($other_attrs)* }; - } - } - }; - - ( - @filter_attrs { - input { #[storage(volatile)] $($input:tt)* }; - storage { $storage:tt }; - other_attrs { $($other_attrs:tt)* }; - } - ) => { - $crate::query_definition! { - @filter_attrs { - input { $($input)* }; - storage { volatile }; - other_attrs { $($other_attrs)* }; - } - } - }; - - ( - @filter_attrs { - input { #[storage(dependencies)] $($input:tt)* }; - storage { $storage:tt }; - other_attrs { $($other_attrs:tt)* }; - } - ) => { - $crate::query_definition! { - @filter_attrs { - input { $($input)* }; - storage { dependencies }; - other_attrs { $($other_attrs)* }; - } - } - }; - - ( - @filter_attrs { - input { #[$attr:meta] $($input:tt)* }; - storage { $storage:tt }; - other_attrs { $($other_attrs:tt)* }; - } - ) => { - $crate::query_definition! { - @filter_attrs { - input { $($input)* }; - storage { $storage }; - other_attrs { $($other_attrs)* #[$attr] }; - } - } - }; - - // Accept a "fn-like" query definition - ( - @filter_attrs { - input { - $v:vis $name:ident( - $db:tt : &impl $query_trait:path, - $key:tt : $key_ty:ty $(,)* - ) -> $value_ty:ty { - $($body:tt)* - } - }; - storage { $storage:tt }; - other_attrs { $($attrs:tt)* }; - } - ) => { - #[derive(Default, Debug)] - $($attrs)* - $v struct $name; - - impl $crate::Query for $name - where - DB: $query_trait, - { - type Key = $key_ty; - type Value = $value_ty; - type Storage = $crate::query_definition! { @storage_ty[DB, Self, $storage] }; - } - - impl $crate::QueryFunction for $name - where - DB: $query_trait, - { - fn execute($db: &DB, $key: $key_ty) -> $value_ty { - $($body)* - } - } - }; - - ( - @storage_ty[$DB:ident, $Self:ident, default] - ) => { - $crate::query_definition! { @storage_ty[$DB, $Self, memoized] } + $crate::query_prototype! { @storage_ty[$DB, $Self, memoized] } }; ( @@ -464,52 +333,10 @@ macro_rules! query_definition { $crate::dependencies::DependencyStorage<$DB, $Self> }; - // Accept a "field-like" query definition (input) ( - @filter_attrs { - input { - $v:vis $name:ident: Map<$key_ty:ty, $value_ty:ty>; - }; - storage { default }; - other_attrs { $($attrs:tt)* }; - } + @storage_ty[$DB:ident, $Self:ident, input] ) => { - #[derive(Default, Debug)] - $($attrs)* - $v struct $name; - - impl $crate::Query for $name - where - DB: $crate::Database - { - type Key = $key_ty; - type Value = $value_ty; - type Storage = $crate::input::InputStorage; - } - }; - - // Various legal start states: - ( - # $($tokens:tt)* - ) => { - $crate::query_definition! { - @filter_attrs { - input { # $($tokens)* }; - storage { default }; - other_attrs { }; - } - } - }; - ( - $v:vis $name:ident $($tokens:tt)* - ) => { - $crate::query_definition! { - @filter_attrs { - input { $v $name $($tokens)* }; - storage { default }; - other_attrs { }; - } - } + $crate::input::InputStorage }; } diff --git a/tests/cycles.rs b/tests/cycles.rs index 5ea4aa13..7635ee35 100644 --- a/tests/cycles.rs +++ b/tests/cycles.rs @@ -33,35 +33,35 @@ salsa::query_prototype! { } fn volatile_a(key: ()) -> () { type VolatileA; + storage volatile; } fn volatile_b(key: ()) -> () { type VolatileB; + storage volatile; } } } -salsa::query_definition! { - crate MemoizedA(db: &impl Database, (): ()) -> () { +impl salsa::QueryFunction for MemoizedA { + fn execute(db: &DB, (): ()) -> () { db.memoized_b(()) } } -salsa::query_definition! { - crate MemoizedB(db: &impl Database, (): ()) -> () { +impl salsa::QueryFunction for MemoizedB { + fn execute(db: &DB, (): ()) -> () { db.memoized_a(()) } } -salsa::query_definition! { - #[storage(volatile)] - crate VolatileA(db: &impl Database, (): ()) -> () { +impl salsa::QueryFunction for VolatileA { + fn execute(db: &DB, (): ()) -> () { db.volatile_b(()) } } -salsa::query_definition! { - #[storage(volatile)] - crate VolatileB(db: &impl Database, (): ()) -> () { +impl salsa::QueryFunction for VolatileB { + fn execute(db: &DB, (): ()) -> () { db.volatile_a(()) } } diff --git a/tests/incremental/memoized_dep_inputs.rs b/tests/incremental/memoized_dep_inputs.rs index 0d8b2b5a..a1041766 100644 --- a/tests/incremental/memoized_dep_inputs.rs +++ b/tests/incremental/memoized_dep_inputs.rs @@ -11,46 +11,40 @@ salsa::query_prototype! { } fn dep_derived1(key: ()) -> usize { type Derived1; + storage dependencies; } fn dep_input1(key: ()) -> usize { type Input1; + storage input; } fn dep_input2(key: ()) -> usize { type Input2; + storage input; } } } -salsa::query_definition! { - crate Memoized2(db: &impl MemoizedDepInputsContext, (): ()) -> usize { +impl salsa::QueryFunction for Memoized2 { + fn execute(db: &DB, (): ()) -> usize { db.log().add("Memoized2 invoked"); db.dep_memoized1(()) } } -salsa::query_definition! { - crate Memoized1(db: &impl MemoizedDepInputsContext, (): ()) -> usize { +impl salsa::QueryFunction for Memoized1 { + fn execute(db: &DB, (): ()) -> usize { db.log().add("Memoized1 invoked"); db.dep_derived1(()) * 2 } } -salsa::query_definition! { - #[storage(dependencies)] - crate Derived1(db: &impl MemoizedDepInputsContext, (): ()) -> usize { +impl salsa::QueryFunction for Derived1 { + fn execute(db: &DB, (): ()) -> usize { db.log().add("Derived1 invoked"); db.dep_input1(()) / 2 } } -salsa::query_definition! { - crate Input1: Map<(), usize>; -} - -salsa::query_definition! { - crate Input2: Map<(), usize>; -} - #[test] fn revalidate() { let db = &TestContextImpl::default(); diff --git a/tests/incremental/memoized_inputs.rs b/tests/incremental/memoized_inputs.rs index 035bd005..45f1095f 100644 --- a/tests/incremental/memoized_inputs.rs +++ b/tests/incremental/memoized_inputs.rs @@ -8,31 +8,22 @@ salsa::query_prototype! { } fn input1(key: ()) -> usize { type Input1; + storage input; } fn input2(key: ()) -> usize { type Input2; + storage input; } } } -salsa::query_definition! { - crate Max(db: &impl MemoizedInputsContext, (): ()) -> usize { +impl salsa::QueryFunction for Max { + fn execute(db: &DB, (): ()) -> usize { db.log().add("Max invoked"); - std::cmp::max( - db.input1(()), - db.input2(()), - ) + std::cmp::max(db.input1(()), db.input2(())) } } -salsa::query_definition! { - crate Input1: Map<(), usize>; -} - -salsa::query_definition! { - crate Input2: Map<(), usize>; -} - #[test] fn revalidate() { let db = &TestContextImpl::default(); diff --git a/tests/incremental/memoized_volatile.rs b/tests/incremental/memoized_volatile.rs index 6b9366a7..703cb47c 100644 --- a/tests/incremental/memoized_volatile.rs +++ b/tests/incremental/memoized_volatile.rs @@ -13,28 +13,28 @@ salsa::query_prototype! { } fn volatile(key: ()) -> usize { type Volatile; + storage volatile; } } } -salsa::query_definition! { - crate Memoized2(db: &impl MemoizedVolatileContext, (): ()) -> usize { +impl salsa::QueryFunction for Memoized2 { + fn execute(db: &DB, (): ()) -> usize { db.log().add("Memoized2 invoked"); db.memoized1(()) } } -salsa::query_definition! { - crate Memoized1(db: &impl MemoizedVolatileContext, (): ()) -> usize { +impl salsa::QueryFunction for Memoized1 { + fn execute(db: &DB, (): ()) -> usize { db.log().add("Memoized1 invoked"); let v = db.volatile(()); v / 2 } } -salsa::query_definition! { - #[storage(volatile)] - crate Volatile(db: &impl MemoizedVolatileContext, (): ()) -> usize { +impl salsa::QueryFunction for Volatile { + fn execute(db: &DB, (): ()) -> usize { db.log().add("Volatile invoked"); db.clock().increment() } diff --git a/tests/storage_varieties/queries.rs b/tests/storage_varieties/queries.rs index 630ee4be..af6e4267 100644 --- a/tests/storage_varieties/queries.rs +++ b/tests/storage_varieties/queries.rs @@ -9,23 +9,23 @@ salsa::query_prototype! { } fn volatile(key: ()) -> usize { type Volatile; + storage volatile; } } } -salsa::query_definition! { - /// Because this query is memoized, we only increment the counter - /// the first time it is invoked. - crate Memoized(db: &impl Database, (): ()) -> usize { +/// Because this query is memoized, we only increment the counter +/// the first time it is invoked. +impl salsa::QueryFunction for Memoized { + fn execute(db: &DB, (): ()) -> usize { db.increment() } } -salsa::query_definition! { - /// Because this query is volatile, each time it is invoked, - /// we will increment the counter. - #[storage(volatile)] - crate Volatile(db: &impl Database, (): ()) -> usize { +/// Because this query is volatile, each time it is invoked, +/// we will increment the counter. +impl salsa::QueryFunction for Volatile { + fn execute(db: &DB, (): ()) -> usize { db.increment() } }