make volatile queries memoize

This ensures consistency of results.

src/derived.rs

@@ -95,7 +95,12 @@ where
     DB: Database,
 {
     fn should_memoize_value(_key: &Q::Key) -> bool {
-        false
+        // 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 is_volatile(_key: &Q::Key) -> bool {

tests/incremental/memoized_volatile.rs

@@ -38,10 +38,11 @@ fn volatile(db: &impl MemoizedVolatileContext, (): ()) -> usize {
 fn volatile_x2() {
     let query = TestContextImpl::default();
 
-    // Invoking volatile twice will simply execute twice.
+    // Invoking volatile twice doesn't execute twice, because volatile
+    // queries are memoized by default.
     query.volatile(());
     query.volatile(());
-    query.assert_log(&["Volatile invoked", "Volatile invoked"]);
+    query.assert_log(&["Volatile invoked"]);
 }
 
 /// Test that:
@@ -67,7 +68,7 @@ fn revalidate() {
     query.salsa_runtime().next_revision();
 
     query.memoized2(());
-    query.assert_log(&["Memoized1 invoked", "Volatile invoked"]);
+    query.assert_log(&["Volatile invoked", "Memoized1 invoked"]);
 
     query.memoized2(());
     query.assert_log(&[]);
@@ -78,7 +79,7 @@ fn revalidate() {
     query.salsa_runtime().next_revision();
 
     query.memoized2(());
-    query.assert_log(&["Memoized1 invoked", "Volatile invoked", "Memoized2 invoked"]);
+    query.assert_log(&["Volatile invoked", "Memoized1 invoked", "Memoized2 invoked"]);
 
     query.memoized2(());
     query.assert_log(&[]);

tests/storage_varieties/main.rs

@@ -1,4 +1,5 @@
 #![feature(crate_visibility_modifier)]
+#![feature(underscore_imports)]
 
 mod implementation;
 mod queries;

tests/storage_varieties/queries.rs

@@ -17,7 +17,7 @@ salsa::query_group! {
 /// Because this query is memoized, we only increment the counter
 /// the first time it is invoked.
 fn memoized(db: &impl Database, (): ()) -> usize {
-    db.increment()
+    db.volatile(())
 }
 
 /// Because this query is volatile, each time it is invoked,

tests/storage_varieties/tests.rs

@@ -2,32 +2,47 @@
 
 use crate::implementation::DatabaseImpl;
 use crate::queries::Database;
+use salsa::Database as _;
 
 #[test]
 fn memoized_twice() {
-    let query = DatabaseImpl::default();
-    let v1 = query.memoized(());
-    let v2 = query.memoized(());
+    let db = DatabaseImpl::default();
+    let v1 = db.memoized(());
+    let v2 = db.memoized(());
     assert_eq!(v1, v2);
 }
 
 #[test]
 fn volatile_twice() {
-    let query = DatabaseImpl::default();
-    let v1 = query.volatile(());
-    let v2 = query.volatile(());
-    assert_eq!(v1 + 1, v2);
+    let db = DatabaseImpl::default();
+    let v1 = db.volatile(());
+    let v2 = db.volatile(()); // volatiles are cached, so 2nd read returns the same
+    assert_eq!(v1, v2);
+
+    db.salsa_runtime().next_revision(); // clears volatile caches
+
+    let v3 = db.volatile(()); // will re-increment the counter
+    let v4 = db.volatile(()); // second call will be cached
+    assert_eq!(v1 + 1, v3);
+    assert_eq!(v3, v4);
 }
 
 #[test]
 fn intermingled() {
-    let query = DatabaseImpl::default();
-    let v1 = query.volatile(());
-    let v2 = query.memoized(());
-    let v3 = query.volatile(());
-    let v4 = query.memoized(());
+    let db = DatabaseImpl::default();
+    let v1 = db.volatile(());
+    let v2 = db.memoized(());
+    let v3 = db.volatile(()); // cached
+    let v4 = db.memoized(()); // cached
 
-    assert_eq!(v1 + 1, v2);
-    assert_eq!(v2 + 1, v3);
+    assert_eq!(v1, v2);
+    assert_eq!(v1, v3);
     assert_eq!(v2, v4);
+
+    db.salsa_runtime().next_revision(); // clears volatile caches
+
+    let v5 = db.memoized(()); // re-executes volatile, caches new result
+    let v6 = db.memoized(()); // re-use cached result
+    assert_eq!(v4 + 1, v5);
+    assert_eq!(v5, v6);
 }