From 1c349d42297e295e22fb2255e4e390640c803052 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Oct 2018 22:34:30 +0300 Subject: [PATCH 1/9] Switch crate to pub(crate) --- src/lib.rs | 1 - src/runtime.rs | 22 +++++++++++----------- tests/cycles.rs | 2 -- tests/incremental/counter.rs | 4 ++-- tests/incremental/implementation.rs | 8 ++++---- tests/incremental/log.rs | 6 +++--- tests/incremental/main.rs | 1 - tests/incremental/memoized_dep_inputs.rs | 2 +- tests/incremental/memoized_inputs.rs | 2 +- tests/incremental/memoized_volatile.rs | 2 +- tests/storage_varieties/main.rs | 1 - tests/storage_varieties/queries.rs | 4 ++-- 12 files changed, 25 insertions(+), 30 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a5317cbf..4868c181 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,5 @@ #![deny(rust_2018_idioms)] #![feature(in_band_lifetimes)] -#![feature(crate_visibility_modifier)] #![feature(nll)] #![feature(integer_atomics)] #![allow(dead_code)] diff --git a/src/runtime.rs b/src/runtime.rs index 604cec31..27f6005f 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -67,14 +67,14 @@ where } /// Read current value of the revision counter. - crate fn current_revision(&self) -> Revision { + pub(crate) fn current_revision(&self) -> Revision { Revision { generation: self.shared_state.revision.load(Ordering::SeqCst), } } /// Increments the current revision counter and returns the new value. - crate fn increment_revision(&self) -> Revision { + pub(crate) fn increment_revision(&self) -> Revision { if !self.local_state.borrow().query_stack.is_empty() { panic!("increment_revision invoked during a query computation"); } @@ -88,7 +88,7 @@ where result } - crate fn execute_query_implementation( + pub(crate) fn execute_query_implementation( &self, descriptor: &DB::QueryDescriptor, execute: impl FnOnce() -> V, @@ -132,13 +132,13 @@ where /// - `descriptor`: the query whose result was read /// - `changed_revision`: the last revision in which the result of that /// query had changed - crate fn report_query_read(&self, descriptor: &DB::QueryDescriptor, changed_at: ChangedAt) { + pub(crate) fn report_query_read(&self, descriptor: &DB::QueryDescriptor, changed_at: ChangedAt) { if let Some(top_query) = self.local_state.borrow_mut().query_stack.last_mut() { top_query.add_read(descriptor, changed_at); } } - crate fn report_untracked_read(&self) { + pub(crate) fn report_untracked_read(&self) { if let Some(top_query) = self.local_state.borrow_mut().query_stack.last_mut() { let changed_at = ChangedAt::Revision(self.current_revision()); top_query.add_untracked_read(changed_at); @@ -146,7 +146,7 @@ where } /// Obviously, this should be user configurable at some point. - crate fn report_unexpected_cycle(&self, descriptor: DB::QueryDescriptor) -> ! { + pub(crate) fn report_unexpected_cycle(&self, descriptor: DB::QueryDescriptor) -> ! { let local_state = self.local_state.borrow(); let LocalState { query_stack, .. } = &*local_state; @@ -213,7 +213,7 @@ pub struct Revision { } impl Revision { - crate const ZERO: Self = Revision { generation: 0 }; + pub(crate) const ZERO: Self = Revision { generation: 0 }; } impl std::fmt::Debug for Revision { @@ -243,7 +243,7 @@ impl ChangedAt { /// An insertion-order-preserving set of queries. Used to track the /// inputs accessed during query execution. -crate enum QueryDescriptorSet { +pub(crate) enum QueryDescriptorSet { /// All reads were to tracked things: Tracked(FxIndexSet), @@ -285,7 +285,7 @@ impl QueryDescriptorSet { } #[derive(Clone, Debug)] -crate struct StampedValue { - crate value: V, - crate changed_at: ChangedAt, +pub(crate) struct StampedValue { + pub(crate) value: V, + pub(crate) changed_at: ChangedAt, } diff --git a/tests/cycles.rs b/tests/cycles.rs index d608b8f2..4bc27c36 100644 --- a/tests/cycles.rs +++ b/tests/cycles.rs @@ -1,5 +1,3 @@ -#![feature(crate_visibility_modifier)] - #[derive(Default)] pub struct DatabaseImpl { runtime: salsa::Runtime, diff --git a/tests/incremental/counter.rs b/tests/incremental/counter.rs index f0d5fbb6..c04857e2 100644 --- a/tests/incremental/counter.rs +++ b/tests/incremental/counter.rs @@ -1,12 +1,12 @@ use std::cell::Cell; #[derive(Default)] -crate struct Counter { +pub(crate) struct Counter { value: Cell, } impl Counter { - crate fn increment(&self) -> usize { + pub(crate) fn increment(&self) -> usize { let v = self.value.get(); self.value.set(v + 1); v diff --git a/tests/incremental/implementation.rs b/tests/incremental/implementation.rs index 7d0231ae..868e49c9 100644 --- a/tests/incremental/implementation.rs +++ b/tests/incremental/implementation.rs @@ -4,20 +4,20 @@ use crate::memoized_dep_inputs; use crate::memoized_inputs; use crate::memoized_volatile; -crate trait TestContext: salsa::Database { +pub(crate) trait TestContext: salsa::Database { fn clock(&self) -> &Counter; fn log(&self) -> &Log; } #[derive(Default)] -crate struct TestContextImpl { +pub(crate) struct TestContextImpl { runtime: salsa::Runtime, clock: Counter, log: Log, } impl TestContextImpl { - crate fn assert_log(&self, expected_log: &[&str]) { + pub(crate) fn assert_log(&self, expected_log: &[&str]) { use difference::{Changeset, Difference}; let expected_text = &format!("{:#?}", expected_log); @@ -42,7 +42,7 @@ impl TestContextImpl { } salsa::database_storage! { - crate struct TestContextImplStorage for TestContextImpl { + pub(crate) struct TestContextImplStorage for TestContextImpl { impl memoized_dep_inputs::MemoizedDepInputsContext { fn dep_memoized2() for memoized_dep_inputs::Memoized2; fn dep_memoized1() for memoized_dep_inputs::Memoized1; diff --git a/tests/incremental/log.rs b/tests/incremental/log.rs index 469bd124..a27a60b2 100644 --- a/tests/incremental/log.rs +++ b/tests/incremental/log.rs @@ -1,16 +1,16 @@ use std::cell::RefCell; #[derive(Default)] -crate struct Log { +pub(crate) struct Log { data: RefCell>, } impl Log { - crate fn add(&self, text: impl Into) { + pub(crate) fn add(&self, text: impl Into) { self.data.borrow_mut().push(text.into()); } - crate fn take(&self) -> Vec { + pub(crate) fn take(&self) -> Vec { std::mem::replace(&mut *self.data.borrow_mut(), vec![]) } } diff --git a/tests/incremental/main.rs b/tests/incremental/main.rs index 8fe306be..f96565e3 100644 --- a/tests/incremental/main.rs +++ b/tests/incremental/main.rs @@ -1,4 +1,3 @@ -#![feature(crate_visibility_modifier)] #![feature(underscore_imports)] mod counter; diff --git a/tests/incremental/memoized_dep_inputs.rs b/tests/incremental/memoized_dep_inputs.rs index 489b158e..152ceb61 100644 --- a/tests/incremental/memoized_dep_inputs.rs +++ b/tests/incremental/memoized_dep_inputs.rs @@ -2,7 +2,7 @@ use crate::implementation::{TestContext, TestContextImpl}; use salsa::Database; salsa::query_group! { - crate trait MemoizedDepInputsContext: TestContext { + pub(crate) trait MemoizedDepInputsContext: TestContext { fn dep_memoized2(key: ()) -> usize { type Memoized2; } diff --git a/tests/incremental/memoized_inputs.rs b/tests/incremental/memoized_inputs.rs index 4ca268ef..0fdc9b79 100644 --- a/tests/incremental/memoized_inputs.rs +++ b/tests/incremental/memoized_inputs.rs @@ -2,7 +2,7 @@ use crate::implementation::{TestContext, TestContextImpl}; use salsa::Database; salsa::query_group! { - crate trait MemoizedInputsContext: TestContext { + pub(crate) trait MemoizedInputsContext: TestContext { fn max(key: ()) -> usize { type Max; } diff --git a/tests/incremental/memoized_volatile.rs b/tests/incremental/memoized_volatile.rs index 36bd3593..b01c2976 100644 --- a/tests/incremental/memoized_volatile.rs +++ b/tests/incremental/memoized_volatile.rs @@ -2,7 +2,7 @@ use crate::implementation::{TestContext, TestContextImpl}; use salsa::Database; salsa::query_group! { - crate trait MemoizedVolatileContext: TestContext { + pub(crate) trait MemoizedVolatileContext: TestContext { // Queries for testing a "volatile" value wrapped by // memoization. fn memoized2(key: ()) -> usize { diff --git a/tests/storage_varieties/main.rs b/tests/storage_varieties/main.rs index 0e90f20e..af0f9342 100644 --- a/tests/storage_varieties/main.rs +++ b/tests/storage_varieties/main.rs @@ -1,4 +1,3 @@ -#![feature(crate_visibility_modifier)] #![feature(underscore_imports)] mod implementation; diff --git a/tests/storage_varieties/queries.rs b/tests/storage_varieties/queries.rs index b66dc53e..763e48ab 100644 --- a/tests/storage_varieties/queries.rs +++ b/tests/storage_varieties/queries.rs @@ -1,9 +1,9 @@ -crate trait Counter: salsa::Database { +pub(crate) trait Counter: salsa::Database { fn increment(&self) -> usize; } salsa::query_group! { - crate trait Database: Counter { + pub(crate) trait Database: Counter { fn memoized(key: ()) -> usize { type Memoized; } From 69b9dff5573a8ddd186fe050fc18063a7a182bd7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Oct 2018 22:37:38 +0300 Subject: [PATCH 2/9] Use AtomicUsize instead of AtomicU64 --- src/lib.rs | 1 - src/runtime.rs | 9 +++++---- tests/incremental/main.rs | 2 -- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4868c181..5ae3b919 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,6 @@ #![deny(rust_2018_idioms)] #![feature(in_band_lifetimes)] #![feature(nll)] -#![feature(integer_atomics)] #![allow(dead_code)] #![allow(unused_imports)] diff --git a/src/runtime.rs b/src/runtime.rs index 27f6005f..ee47eb01 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -6,7 +6,7 @@ use rustc_hash::FxHasher; use std::cell::RefCell; use std::fmt::Write; use std::hash::BuildHasherDefault; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; type FxIndexSet = indexmap::IndexSet>; @@ -69,7 +69,7 @@ where /// Read current value of the revision counter. pub(crate) fn current_revision(&self) -> Revision { Revision { - generation: self.shared_state.revision.load(Ordering::SeqCst), + generation: self.shared_state.revision.load(Ordering::SeqCst) as u64, } } @@ -80,7 +80,7 @@ where } let result = Revision { - generation: 1 + self.shared_state.revision.fetch_add(1, Ordering::SeqCst), + generation: 1 + self.shared_state.revision.fetch_add(1, Ordering::SeqCst) as u64, }; debug!("increment_revision: incremented to {:?}", result); @@ -167,7 +167,8 @@ where /// State that will be common to all threads (when we support multiple threads) struct SharedState { storage: DB::DatabaseStorage, - revision: AtomicU64, + // Ideally, this should be `AtomicU64`, but that is currently unstable. + revision: AtomicUsize, } /// State that will be specific to a single execution threads (when we diff --git a/tests/incremental/main.rs b/tests/incremental/main.rs index f96565e3..33a623cd 100644 --- a/tests/incremental/main.rs +++ b/tests/incremental/main.rs @@ -1,5 +1,3 @@ -#![feature(underscore_imports)] - mod counter; mod implementation; mod log; From c3fb7a1f248ba462cdd291309eadc96a2c24139a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Oct 2018 22:37:55 +0300 Subject: [PATCH 3/9] disable nll --- src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 5ae3b919..f2e23618 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,5 @@ #![deny(rust_2018_idioms)] #![feature(in_band_lifetimes)] -#![feature(nll)] #![allow(dead_code)] #![allow(unused_imports)] From f28e8c1be5a14585101d62d08b0d820ad7e5bc69 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Oct 2018 22:39:03 +0300 Subject: [PATCH 4/9] disable in-band lifetimes --- src/derived.rs | 2 +- src/input.rs | 2 +- src/lib.rs | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/derived.rs b/src/derived.rs index 83090539..c985a3cd 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -336,7 +336,7 @@ where Ok(value) } - fn maybe_changed_since( + fn maybe_changed_since<'q>( &self, db: &'q DB, revision: Revision, diff --git a/src/input.rs b/src/input.rs index a3d95323..b46cede9 100644 --- a/src/input.rs +++ b/src/input.rs @@ -91,7 +91,7 @@ where Ok(value) } - fn maybe_changed_since( + fn maybe_changed_since<'q>( &self, _db: &'q DB, revision: Revision, diff --git a/src/lib.rs b/src/lib.rs index f2e23618..d19874ff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,4 @@ #![deny(rust_2018_idioms)] -#![feature(in_band_lifetimes)] #![allow(dead_code)] #![allow(unused_imports)] @@ -157,7 +156,7 @@ where pub struct CycleDetected; -impl QueryTable<'me, DB, Q> +impl<'me, DB, Q> QueryTable<'me, DB, Q> where DB: Database, Q: Query, From 46c55a1e5b359c66694cd0e69d4e1ba3c3a89e09 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Oct 2018 22:40:03 +0300 Subject: [PATCH 5/9] Disable underscore_imports Builds with beta now! --- tests/storage_varieties/main.rs | 2 -- tests/storage_varieties/tests.rs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/storage_varieties/main.rs b/tests/storage_varieties/main.rs index af0f9342..e92c6174 100644 --- a/tests/storage_varieties/main.rs +++ b/tests/storage_varieties/main.rs @@ -1,5 +1,3 @@ -#![feature(underscore_imports)] - mod implementation; mod queries; mod tests; diff --git a/tests/storage_varieties/tests.rs b/tests/storage_varieties/tests.rs index bfd82143..486a44fe 100644 --- a/tests/storage_varieties/tests.rs +++ b/tests/storage_varieties/tests.rs @@ -2,7 +2,7 @@ use crate::implementation::DatabaseImpl; use crate::queries::Database; -use salsa::Database as _; +use salsa::Database as _Database; #[test] fn memoized_twice() { From 2714730c9dcdbe7d3971e7005d29d2bd3757f1f4 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Oct 2018 22:42:07 +0300 Subject: [PATCH 6/9] Switch travis & readme to beta --- .travis.yml | 2 +- README.md | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index c8f1c5c3..323197fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: rust rust: - - nightly + - beta script: - RUST_BACKTRACE=1 CARGO_INCREMENTAL=0 cargo test --all - RUST_BACKTRACE=1 CARGO_INCREMENTAL=0 cargo test --tests --all diff --git a/README.md b/README.md index 9130d909..d0eaeeab 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,11 @@ Using salsa is as easy as 1, 2, 3... the inputs/queries you will be using. The query struct will contain the storage for all of the inputs/queries and may also contain anything else that your code needs (e.g., configuration data). - + To see an example of this in action, check out [the `hello_world` example](examples/hello_world/main.rs), which has a number of comments explaining how things work. The [`hello_world` README](examples/hello_world/README.md) has a more detailed writeup. +Salsa requires at least Rust 1.30 (beta at the time of writing). + From 74486afdec6e181f8fa31af3f62cfd699ccc0166 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 10 Oct 2018 00:44:26 +0300 Subject: [PATCH 7/9] elide some lifetimes --- src/derived.rs | 8 ++++---- src/input.rs | 8 ++++---- src/lib.rs | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/derived.rs b/src/derived.rs index c985a3cd..49ca1e5d 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -323,9 +323,9 @@ where DB: Database, MP: MemoizationPolicy, { - fn try_fetch<'q>( + fn try_fetch( &self, - db: &'q DB, + db: &DB, key: &Q::Key, descriptor: &DB::QueryDescriptor, ) -> Result { @@ -336,9 +336,9 @@ where Ok(value) } - fn maybe_changed_since<'q>( + fn maybe_changed_since( &self, - db: &'q DB, + db: &DB, revision: Revision, key: &Q::Key, descriptor: &DB::QueryDescriptor, diff --git a/src/input.rs b/src/input.rs index b46cede9..80678bfa 100644 --- a/src/input.rs +++ b/src/input.rs @@ -78,9 +78,9 @@ where DB: Database, Q::Value: Default, { - fn try_fetch<'q>( + fn try_fetch( &self, - db: &'q DB, + db: &DB, key: &Q::Key, descriptor: &DB::QueryDescriptor, ) -> Result { @@ -91,9 +91,9 @@ where Ok(value) } - fn maybe_changed_since<'q>( + fn maybe_changed_since( &self, - _db: &'q DB, + _db: &DB, revision: Revision, key: &Q::Key, _descriptor: &DB::QueryDescriptor, diff --git a/src/lib.rs b/src/lib.rs index d19874ff..56b6faca 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -156,7 +156,7 @@ where pub struct CycleDetected; -impl<'me, DB, Q> QueryTable<'me, DB, Q> +impl QueryTable<'_, DB, Q> where DB: Database, Q: Query, From e42c68913eacb2b42521261a4dc4f46e7750506a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 10 Oct 2018 00:48:19 +0300 Subject: [PATCH 8/9] check revision for overflow --- src/runtime.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime.rs b/src/runtime.rs index ee47eb01..a6c2a52d 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -79,8 +79,10 @@ where panic!("increment_revision invoked during a query computation"); } + let old_revision = self.shared_state.revision.fetch_add(1, Ordering::SeqCst); + assert!(old_revision != usize::max_value()); let result = Revision { - generation: 1 + self.shared_state.revision.fetch_add(1, Ordering::SeqCst) as u64, + generation: 1 + old_revision as u64, }; debug!("increment_revision: incremented to {:?}", result); From a45d482a976a69aecd46dda0d29251dc73cb96d2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 9 Oct 2018 18:00:27 -0400 Subject: [PATCH 9/9] more descriptive assertion failure for overflow --- src/runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime.rs b/src/runtime.rs index a6c2a52d..e032453d 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -80,7 +80,7 @@ where } let old_revision = self.shared_state.revision.fetch_add(1, Ordering::SeqCst); - assert!(old_revision != usize::max_value()); + assert!(old_revision != usize::max_value(), "revision overflow"); let result = Revision { generation: 1 + old_revision as u64, };