From 3cde493e4e7b4f06c0a83a9c69bdab83cb3e6ae4 Mon Sep 17 00:00:00 2001
From: Niko Matsakis <niko@alum.mit.edu>
Date: Thu, 13 Dec 2018 05:33:33 -0500
Subject: [PATCH] remove the virtual fn call for creating query descriptors

Previously, to "make the connection" between the database type `DB` and
the query type `Q`, we were passing down a fn pointer that would crate
the query descriptor. But now we have this `GetQueryTable` trait -- use
that instead.
---
 src/debug.rs    |  3 ++-
 src/lib.rs      | 35 +++++++++++++++--------------------
 src/plumbing.rs | 17 +++++++++++++++++
 3 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/src/debug.rs b/src/debug.rs
index 4f180b33..f53868af 100644
--- a/src/debug.rs
+++ b/src/debug.rs
@@ -1,6 +1,7 @@
 //! Debugging APIs: these are meant for use when unit-testing or
 //! debugging your application but aren't ordinarily needed.
 
+use crate::plumbing;
 use crate::plumbing::QueryStorageOps;
 use crate::Database;
 use crate::Query;
@@ -27,7 +28,7 @@ pub trait DebugQueryTable {
 
 impl<DB, Q> DebugQueryTable for QueryTable<'_, DB, Q>
 where
-    DB: Database,
+    DB: plumbing::GetQueryTable<Q>,
     Q: Query<DB>,
 {
     type Key = Q::Key;
diff --git a/src/lib.rs b/src/lib.rs
index 2bb4d38e..12c74e8d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -351,17 +351,16 @@ pub trait Query<DB: Database>: Debug + Default + Sized + 'static {
 #[derive(new)]
 pub struct QueryTable<'me, DB, Q>
 where
-    DB: Database,
+    DB: plumbing::GetQueryTable<Q>,
     Q: Query<DB> + 'me,
 {
     db: &'me DB,
     storage: &'me Q::Storage,
-    descriptor_fn: fn(&DB, &Q::Key) -> DB::QueryDescriptor,
 }
 
 impl<DB, Q> QueryTable<'_, DB, Q>
 where
-    DB: Database,
+    DB: plumbing::GetQueryTable<Q>,
     Q: Query<DB>,
 {
     /// Execute the query on a given input. Usually it's easier to
@@ -387,7 +386,7 @@ where
     }
 
     fn descriptor(&self, key: &Q::Key) -> DB::QueryDescriptor {
-        (self.descriptor_fn)(self.db, key)
+        <DB as plumbing::GetQueryTable<Q>>::descriptor(&self.db, key.clone())
     }
 }
 
@@ -399,21 +398,20 @@ where
 #[derive(new)]
 pub struct QueryTableMut<'me, DB, Q>
 where
-    DB: Database,
+    DB: plumbing::GetQueryTable<Q>,
     Q: Query<DB> + 'me,
 {
     db: &'me DB,
     storage: &'me Q::Storage,
-    descriptor_fn: fn(&DB, &Q::Key) -> DB::QueryDescriptor,
 }
 
 impl<DB, Q> QueryTableMut<'_, DB, Q>
 where
-    DB: Database,
+    DB: plumbing::GetQueryTable<Q>,
     Q: Query<DB>,
 {
     fn descriptor(&self, key: &Q::Key) -> DB::QueryDescriptor {
-        (self.descriptor_fn)(self.db, key)
+        <DB as plumbing::GetQueryTable<Q>>::descriptor(&self.db, key.clone())
     }
 
     /// Assign a value to an "input query". Must be used outside of
@@ -885,12 +883,6 @@ macro_rules! database_storage {
                             &$crate::Database::salsa_runtime(db)
                                 .storage()
                                 .$query_method,
-                            |_, key| {
-                                let key = std::clone::Clone::clone(key);
-                                __SalsaQueryDescriptor {
-                                    kind: __SalsaQueryDescriptorKind::$query_method(key),
-                                }
-                            },
                         )
                     }
 
@@ -903,14 +895,17 @@ macro_rules! database_storage {
                             &$crate::Database::salsa_runtime(db)
                                 .storage()
                                 .$query_method,
-                            |_, key| {
-                                let key = std::clone::Clone::clone(key);
-                                __SalsaQueryDescriptor {
-                                    kind: __SalsaQueryDescriptorKind::$query_method(key),
-                                }
-                            },
                         )
                     }
+
+                    fn descriptor(
+                        db: &Self,
+                        key: <$QueryType as $crate::Query<Self>>::Key,
+                    ) -> <Self as $crate::plumbing::DatabaseStorageTypes>::QueryDescriptor {
+                        __SalsaQueryDescriptor {
+                            kind: __SalsaQueryDescriptorKind::$query_method(key),
+                        }
+                    }
                 }
             )*
         )*
diff --git a/src/plumbing.rs b/src/plumbing.rs
index 5487b444..61768bed 100644
--- a/src/plumbing.rs
+++ b/src/plumbing.rs
@@ -58,10 +58,27 @@ pub trait QueryFunction<DB: Database>: Query<DB> {
     fn execute(db: &DB, key: Self::Key) -> Self::Value;
 }
 
+/// The `GetQueryTable` trait makes the connection the *database type*
+/// `DB` and some specific *query type* `Q` that it supports. Note
+/// that the `Database` trait itself is not specific to any query, and
+/// the impls of the query trait are not specific to any *database*
+/// (in particular, query groups are defined without knowing the final
+/// database type). This trait then serves to put the query in the
+/// context of the full database. It gives access to the storage for
+/// the query and also to creating the query descriptor. For any given
+/// database, impls of this trait are created by the
+/// `database_storage` macro.
 pub trait GetQueryTable<Q: Query<Self>>: Database {
+    /// Create a query table, which has access to the storage for the query
+    /// and offers methods like `get`.
     fn get_query_table(db: &Self) -> QueryTable<'_, Self, Q>;
 
+    /// Create a mutable query table, which has access to the storage
+    /// for the query and offers methods like `set`.
     fn get_query_table_mut(db: &mut Self) -> QueryTableMut<'_, Self, Q>;
+
+    /// Create a query descriptor given a key for this query.
+    fn descriptor(db: &Self, key: Q::Key) -> Self::QueryDescriptor;
 }
 
 pub trait QueryStorageOps<DB, Q>: Default