Replace volatile query type with report_untracked_read fn

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "salsa"
-version = "0.12.3"
+version = "0.13.0"
 authors = ["Salsa developers"]
 edition = "2018"
 license = "Apache-2.0 OR MIT"
@@ -16,7 +16,7 @@ lock_api = "0.2.0"
 indexmap = "1.0.1"
 log = "0.4.5"
 smallvec = "0.6.5"
-salsa-macros = { version = "0.12.1", path = "components/salsa-macros" }
+salsa-macros = { version = "0.13.0", path = "components/salsa-macros" }
 linked-hash-map = "0.5.2"
 
 [dev-dependencies]

components/salsa-macros/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "salsa-macros"
-version = "0.12.1"
+version = "0.13.0"
 authors = ["Salsa developers"]
 edition = "2018"
 license = "Apache-2.0 OR MIT"

components/salsa-macros/src/lib.rs

@@ -58,7 +58,6 @@ mod query_group;
 ///   are described in detail in the section below.
 ///   - `#[salsa::input]`
 ///   - `#[salsa::memoized]`
-///   - `#[salsa::volatile]`
 ///   - `#[salsa::dependencies]`
 /// - Query execution:
 ///   - `#[salsa::invoke(path::to::my_fn)]` -- for a non-input, this
@@ -96,8 +95,6 @@ mod query_group;
 ///   which can significantly reduce the amount of recomputation
 ///   required in new revisions. This does require that the value
 ///   implements `Eq`.
-/// - `#[salsa::volatile]` -- indicates that the inputs are not fully
-///   captured by salsa. The result will be recomputed once per revision.
 /// - `#[salsa::dependencies]` -- does not cache the value, so it will
 ///   be recomputed every time it is needed. We do track the inputs, however,
 ///   so if they have not changed, then things that rely on this query

components/salsa-macros/src/query_group.rs

@@ -54,10 +54,6 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
                             storage = QueryStorage::Memoized;
                             num_storages += 1;
                         }
-                        "volatile" => {
-                            storage = QueryStorage::Volatile;
-                            num_storages += 1;
-                        }
                         "dependencies" => {
                             storage = QueryStorage::Dependencies;
                             num_storages += 1;
@@ -359,7 +355,6 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
 
         let storage = match &query.storage {
             QueryStorage::Memoized => quote!(salsa::plumbing::MemoizedStorage<#db, Self>),
-            QueryStorage::Volatile => quote!(salsa::plumbing::VolatileStorage<#db, Self>),
             QueryStorage::Dependencies => quote!(salsa::plumbing::DependencyStorage<#db, Self>),
             QueryStorage::Input => quote!(salsa::plumbing::InputStorage<#db, Self>),
             QueryStorage::Interned => quote!(salsa::plumbing::InternedStorage<#db, Self>),
@@ -579,7 +574,6 @@ impl Query {
 #[derive(Debug, Clone, PartialEq, Eq)]
 enum QueryStorage {
     Memoized,
-    Volatile,
     Dependencies,
     Input,
     Interned,
@@ -594,7 +588,7 @@ impl QueryStorage {
             | QueryStorage::Interned
             | QueryStorage::InternedLookup { .. }
             | QueryStorage::Transparent => false,
-            QueryStorage::Memoized | QueryStorage::Volatile | QueryStorage::Dependencies => true,
+            QueryStorage::Memoized | QueryStorage::Dependencies => true,
         }
     }
 }

src/derived.rs

@@ -35,11 +35,6 @@ pub type MemoizedStorage<DB, Q> = DerivedStorage<DB, Q, AlwaysMemoizeValue>;
 /// storage requirements.
 pub type DependencyStorage<DB, Q> = DerivedStorage<DB, Q, NeverMemoizeValue>;
 
-/// "Dependency" queries just track their dependencies and not the
-/// actual value (which they produce on demand). This lessens the
-/// storage requirements.
-pub type VolatileStorage<DB, Q> = DerivedStorage<DB, Q, VolatileValue>;
-
 /// Handles storage where the value is 'derived' by executing a
 /// function (in contrast to "inputs").
 pub struct DerivedStorage<DB, Q, MP>
@@ -73,8 +68,6 @@ where
     fn should_memoize_value(key: &Q::Key) -> bool;
 
     fn memoized_value_eq(old_value: &Q::Value, new_value: &Q::Value) -> bool;
-
-    fn should_track_inputs(key: &Q::Key) -> bool;
 }
 
 pub enum AlwaysMemoizeValue {}
@@ -91,10 +84,6 @@ where
     fn memoized_value_eq(old_value: &Q::Value, new_value: &Q::Value) -> bool {
         old_value == new_value
     }
-
-    fn should_track_inputs(_key: &Q::Key) -> bool {
-        true
-    }
 }
 
 pub enum NeverMemoizeValue {}
@@ -110,34 +99,6 @@ where
     fn memoized_value_eq(_old_value: &Q::Value, _new_value: &Q::Value) -> bool {
         panic!("cannot reach since we never memoize")
     }
-
-    fn should_track_inputs(_key: &Q::Key) -> bool {
-        true
-    }
-}
-
-pub enum VolatileValue {}
-impl<DB, Q> MemoizationPolicy<DB, Q> for VolatileValue
-where
-    Q: QueryFunction<DB>,
-    DB: Database,
-{
-    fn should_memoize_value(_key: &Q::Key) -> bool {
-        // Why memoize? Well, if the "volatile" value really is
-        // constantly changing, we still want to capture its value
-        // until the next revision is triggered and ensure it doesn't
-        // change -- otherwise the system gets into an inconsistent
-        // state where the same query reports back different values.
-        true
-    }
-
-    fn memoized_value_eq(_old_value: &Q::Value, _new_value: &Q::Value) -> bool {
-        false
-    }
-
-    fn should_track_inputs(_key: &Q::Key) -> bool {
-        false
-    }
 }
 
 /// Defines the "current state" of query's memoized results.
@@ -443,11 +404,6 @@ where
         // stale, or value is absent. Let's execute!
         let mut result = runtime.execute_query_implementation(db, database_key, || {
             info!("{:?}({:?}): executing query", Q::default(), key);
-
-            if !self.should_track_inputs(key) {
-                runtime.report_untracked_read();
-            }
-
             Q::execute(db, key.clone())
         });
 
@@ -649,10 +605,6 @@ where
     fn should_memoize_value(&self, key: &Q::Key) -> bool {
         MP::should_memoize_value(key)
     }
-
-    fn should_track_inputs(&self, key: &Q::Key) -> bool {
-        MP::should_track_inputs(key)
-    }
 }
 
 struct PanicGuard<'db, DB, Q>

src/plumbing.rs

@@ -11,7 +11,6 @@ use std::hash::Hash;
 
 pub use crate::derived::DependencyStorage;
 pub use crate::derived::MemoizedStorage;
-pub use crate::derived::VolatileStorage;
 pub use crate::input::InputStorage;
 pub use crate::interned::InternedStorage;
 pub use crate::interned::LookupInternedStorage;

src/runtime.rs

@@ -344,7 +344,11 @@ where
         self.local_state.report_query_read(database_key, changed_at);
     }
 
-    pub(crate) fn report_untracked_read(&self) {
+    /// Reports that the query depends on some state unknown to salsa.
+    ///
+    /// If query reports untracked read, it will be reexuted in the next
+    /// revision.
+    pub fn report_untracked_read(&self) {
         self.local_state
             .report_untracked_read(self.current_revision());
     }

tests/cycles.rs

@@ -15,9 +15,7 @@ trait Database: salsa::Database {
     // `a` and `b` depend on each other and form a cycle
     fn memoized_a(&self) -> ();
     fn memoized_b(&self) -> ();
-    #[salsa::volatile]
     fn volatile_a(&self) -> ();
-    #[salsa::volatile]
     fn volatile_b(&self) -> ();
 }
 
@@ -30,10 +28,12 @@ fn memoized_b(db: &impl Database) -> () {
 }
 
 fn volatile_a(db: &impl Database) -> () {
+    db.salsa_runtime().report_untracked_read();
     db.volatile_b()
 }
 
 fn volatile_b(db: &impl Database) -> () {
+    db.salsa_runtime().report_untracked_read();
     db.volatile_a()
 }
 

tests/gc/volatile_tests.rs

@@ -5,12 +5,11 @@ use std::sync::Arc;
 
 /// Query group for tests for how interned keys interact with GC.
 #[salsa::query_group(Volatile)]
-pub(crate) trait VolatileDatabase {
+pub(crate) trait VolatileDatabase: Database {
     #[salsa::input]
     fn atomic_cell(&self) -> Arc<AtomicUsize>;
 
     /// Underlying volatile query.
-    #[salsa::volatile]
     fn volatile(&self) -> usize;
 
     /// This just executes the intern query and returns the result.
@@ -21,6 +20,7 @@ pub(crate) trait VolatileDatabase {
 }
 
 fn volatile(db: &impl VolatileDatabase) -> usize {
+    db.salsa_runtime().report_untracked_read();
     db.atomic_cell().load(Ordering::SeqCst)
 }
 

tests/incremental/memoized_volatile.rs

@@ -7,7 +7,6 @@ pub(crate) trait MemoizedVolatileContext: TestContext {
     // memoization.
     fn memoized2(&self) -> usize;
     fn memoized1(&self) -> usize;
-    #[salsa::volatile]
     fn volatile(&self) -> usize;
 }
 
@@ -24,6 +23,7 @@ fn memoized1(db: &impl MemoizedVolatileContext) -> usize {
 
 fn volatile(db: &impl MemoizedVolatileContext) -> usize {
     db.log().add("Volatile invoked");
+    db.salsa_runtime().report_untracked_read();
     db.clock().increment()
 }
 

tests/lru.rs

@@ -25,9 +25,8 @@ impl Drop for HotPotato {
 }
 
 #[salsa::query_group(QueryGroupStorage)]
-trait QueryGroup {
+trait QueryGroup: salsa::Database {
     fn get(&self, x: u32) -> Arc<HotPotato>;
-    #[salsa::volatile]
     fn get_volatile(&self, x: u32) -> usize;
 }
 
@@ -35,8 +34,9 @@ fn get(_db: &impl QueryGroup, x: u32) -> Arc<HotPotato> {
     Arc::new(HotPotato::new(x))
 }
 
-fn get_volatile(_db: &impl QueryGroup, _x: u32) -> usize {
+fn get_volatile(db: &impl QueryGroup, _x: u32) -> usize {
     static COUNTER: AtomicUsize = AtomicUsize::new(0);
+    db.salsa_runtime().report_untracked_read();
     COUNTER.fetch_add(1, Ordering::SeqCst)
 }
 

tests/storage_varieties/queries.rs

@@ -5,7 +5,6 @@ pub(crate) trait Counter: salsa::Database {
 #[salsa::query_group(GroupStruct)]
 pub(crate) trait Database: Counter {
     fn memoized(&self) -> usize;
-    #[salsa::volatile]
     fn volatile(&self) -> usize;
 }
 
@@ -18,5 +17,6 @@ fn memoized(db: &impl Database) -> usize {
 /// Because this query is volatile, each time it is invoked,
 /// we will increment the counter.
 fn volatile(db: &impl Database) -> usize {
+    db.salsa_runtime().report_untracked_read();
     db.increment()
 }