From 1d52d5d422d4a024bde1897facc1d5e3e826315c Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Tue, 22 Apr 2025 14:01:10 +0300 Subject: [PATCH 1/6] Depend on Salsa v0.20.0 --- Cargo.lock | 17 +++++++++-------- Cargo.toml | 4 ++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2cf6731aad73..cecaaa803b74 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -122,9 +122,9 @@ dependencies = [ [[package]] name = "boxcar" -version = "0.2.10" +version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "225450ee9328e1e828319b48a89726cffc1b0ad26fd9211ad435de9fa376acae" +checksum = "6740c6e2fc6360fa57c35214c7493826aee95993926092606f27c983b40837be" dependencies = [ "loom", ] @@ -2039,9 +2039,9 @@ checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" [[package]] name = "salsa" -version = "0.19.0" +version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd55c6549513b2a42884dae31e3d4f4ac8a6cc51062e68e24d162133889f327c" +checksum = "1be22155f8d9732518b2db2bf379fe6f0b2375e76b08b7c8fe6c1b887d548c24" dependencies = [ "boxcar", "crossbeam-queue", @@ -2056,20 +2056,21 @@ dependencies = [ "salsa-macro-rules", "salsa-macros", "smallvec", + "thin-vec", "tracing", ] [[package]] name = "salsa-macro-rules" -version = "0.19.0" +version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2619b4b451beab0a7e4364ff1e6f31950e7e418888fd9bf2f28889671563166a" +checksum = "f55a7ef0a84e336f7c5f0332d81727f5629fe042d2aa556c75307afebc9f78a5" [[package]] name = "salsa-macros" -version = "0.19.0" +version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4be57a99b3896e8d26850428a6874fb86849e2db874e1db3528e5cee4337d277" +checksum = "8d0e88a9c0c0d231a63f83dcd1a2c5e5d11044fac4b65bc9ad3b68ab48b0a0ab" dependencies = [ "heck", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 851235eb99d4..66d42b12f09c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,7 +46,7 @@ debug = 2 # ungrammar = { path = "../ungrammar" } -# rust-analyzer-salsa = { path = "../salsa" } +# salsa = { path = "../salsa" } [workspace.dependencies] # local crates @@ -131,7 +131,7 @@ process-wrap = { version = "8.2.0", features = ["std"] } pulldown-cmark-to-cmark = "10.0.4" pulldown-cmark = { version = "0.9.6", default-features = false } rayon = "1.10.0" -salsa = "0.19" +salsa = "0.20.0" rustc-hash = "2.1.1" semver = "1.0.26" serde = { version = "1.0.219" } From 2bba385dda5bbdfb1eff2e48f79b446c112d791e Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 14 Apr 2025 00:54:41 +0300 Subject: [PATCH 2/6] Adjust for new Salsa not implementing `Debug` by default --- crates/base-db/src/input.rs | 2 +- crates/base-db/src/lib.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 3f6766b33251..addbb3f5851d 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -392,7 +392,7 @@ impl BuiltDependency { pub type CratesIdMap = FxHashMap; -#[salsa::input(no_debug)] +#[salsa::input] #[derive(Debug)] pub struct Crate { #[return_ref] diff --git a/crates/base-db/src/lib.rs b/crates/base-db/src/lib.rs index 55010a0349a4..7f7a712577e4 100644 --- a/crates/base-db/src/lib.rs +++ b/crates/base-db/src/lib.rs @@ -28,7 +28,7 @@ pub use vfs::{AnchoredPath, AnchoredPathBuf, FileId, VfsPath, file_set::FileSet} #[macro_export] macro_rules! impl_intern_key { ($id:ident, $loc:ident) => { - #[salsa::interned(no_debug, no_lifetime)] + #[salsa::interned(no_lifetime)] pub struct $id { pub loc: $loc, } @@ -152,7 +152,7 @@ impl Files { } } -#[salsa::interned(no_lifetime, constructor=from_span)] +#[salsa::interned(no_lifetime, debug, constructor=from_span)] pub struct EditionedFileId { pub editioned_file_id: span::EditionedFileId, } @@ -187,18 +187,18 @@ impl EditionedFileId { } } -#[salsa::input] +#[salsa::input(debug)] pub struct FileText { pub text: Arc, pub file_id: vfs::FileId, } -#[salsa::input] +#[salsa::input(debug)] pub struct FileSourceRootInput { pub source_root_id: SourceRootId, } -#[salsa::input] +#[salsa::input(debug)] pub struct SourceRootInput { pub source_root: Arc, } From db72e2ff4155e88b9853661f61171f9fdb95c004 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 14 Apr 2025 00:55:27 +0300 Subject: [PATCH 3/6] Adjust for `salsa::Id::from_u32()` being unsafe This impacts our manual `salsa::Id` wrappers. I refactored them a bit to improve safety. --- crates/hir-expand/src/lib.rs | 2 +- crates/hir-ty/src/mapping.rs | 6 +- .../src/legacy_protocol/msg/flat.rs | 4 +- crates/span/src/hygiene.rs | 125 +++++++++-------- crates/span/src/lib.rs | 126 ------------------ 5 files changed, 80 insertions(+), 183 deletions(-) diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index cd2448bad4a3..f0a9a2ad52c8 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -1051,7 +1051,7 @@ impl ExpandTo { intern::impl_internable!(ModPath, attrs::AttrInput); -#[salsa::interned(no_lifetime)] +#[salsa::interned(no_lifetime, debug)] #[doc(alias = "MacroFileId")] pub struct MacroCallId { pub loc: MacroCallLoc, diff --git a/crates/hir-ty/src/mapping.rs b/crates/hir-ty/src/mapping.rs index f7511e5f63a3..2abc1ac62a99 100644 --- a/crates/hir-ty/src/mapping.rs +++ b/crates/hir-ty/src/mapping.rs @@ -136,7 +136,8 @@ pub fn from_assoc_type_id(id: AssocTypeId) -> TypeAliasId { pub fn from_placeholder_idx(db: &dyn HirDatabase, idx: PlaceholderIndex) -> TypeOrConstParamId { assert_eq!(idx.ui, chalk_ir::UniverseIndex::ROOT); - let interned_id = FromId::from_id(Id::from_u32(idx.idx.try_into().unwrap())); + // SAFETY: We cannot really encapsulate this unfortunately, so just hope this is sound. + let interned_id = FromId::from_id(unsafe { Id::from_u32(idx.idx.try_into().unwrap()) }); db.lookup_intern_type_or_const_param_id(interned_id) } @@ -150,7 +151,8 @@ pub fn to_placeholder_idx(db: &dyn HirDatabase, id: TypeOrConstParamId) -> Place pub fn lt_from_placeholder_idx(db: &dyn HirDatabase, idx: PlaceholderIndex) -> LifetimeParamId { assert_eq!(idx.ui, chalk_ir::UniverseIndex::ROOT); - let interned_id = FromId::from_id(Id::from_u32(idx.idx.try_into().unwrap())); + // SAFETY: We cannot really encapsulate this unfortunately, so just hope this is sound. + let interned_id = FromId::from_id(unsafe { Id::from_u32(idx.idx.try_into().unwrap()) }); db.lookup_intern_lifetime_param_id(interned_id) } diff --git a/crates/proc-macro-api/src/legacy_protocol/msg/flat.rs b/crates/proc-macro-api/src/legacy_protocol/msg/flat.rs index 101c4b3105ad..597ffa05d203 100644 --- a/crates/proc-macro-api/src/legacy_protocol/msg/flat.rs +++ b/crates/proc-macro-api/src/legacy_protocol/msg/flat.rs @@ -72,7 +72,9 @@ pub fn deserialize_span_data_index_map(map: &[u32]) -> SpanDataIndexMap { ast_id: ErasedFileAstId::from_raw(ast_id), }, range: TextRange::new(start.into(), end.into()), - ctx: SyntaxContext::from_u32(e), + // SAFETY: We only receive spans from the server. If someone mess up the communication UB can happen, + // but that will be their problem. + ctx: unsafe { SyntaxContext::from_u32(e) }, } }) .collect() diff --git a/crates/span/src/hygiene.rs b/crates/span/src/hygiene.rs index a2923cd22334..6022b5b12094 100644 --- a/crates/span/src/hygiene.rs +++ b/crates/span/src/hygiene.rs @@ -27,7 +27,10 @@ use crate::Edition; #[cfg(feature = "salsa")] #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] pub struct SyntaxContext( - salsa::Id, + /// # Invariant + /// + /// This is either a valid `salsa::Id` or a root `SyntaxContext`. + u32, std::marker::PhantomData<&'static salsa::plumbing::interned::Value>, ); @@ -95,10 +98,11 @@ const _: () = { type Fields<'a> = SyntaxContextData; type Struct<'a> = SyntaxContext; fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> { - SyntaxContext(id, std::marker::PhantomData) + SyntaxContext::from_salsa_id(id) } fn deref_struct(s: Self::Struct<'_>) -> salsa::Id { - s.0 + s.as_salsa_id() + .expect("`SyntaxContext::deref_structs()` called on a root `SyntaxContext`") } } impl SyntaxContext { @@ -115,12 +119,12 @@ const _: () = { } impl zalsa_::AsId for SyntaxContext { fn as_id(&self) -> salsa::Id { - self.0 + self.as_salsa_id().expect("`SyntaxContext::as_id()` called on a root `SyntaxContext`") } } impl zalsa_::FromId for SyntaxContext { fn from_id(id: salsa::Id) -> Self { - Self(id, std::marker::PhantomData) + Self::from_salsa_id(id) } } unsafe impl Send for SyntaxContext {} @@ -210,44 +214,44 @@ const _: () = { where Db: ?Sized + zalsa_::Database, { - if self.is_root() { - return None; - } - let fields = SyntaxContext::ingredient(db).fields(db.as_dyn_database(), self); - std::clone::Clone::clone(&fields.outer_expn) + let id = self.as_salsa_id()?; + let fields = SyntaxContext::ingredient(db).data(db.as_dyn_database(), id); + fields.outer_expn } pub fn outer_transparency(self, db: &'db Db) -> Transparency where Db: ?Sized + zalsa_::Database, { - if self.is_root() { - return Transparency::Opaque; - } - let fields = SyntaxContext::ingredient(db).fields(db.as_dyn_database(), self); - std::clone::Clone::clone(&fields.outer_transparency) + let Some(id) = self.as_salsa_id() else { return Transparency::Opaque }; + let fields = SyntaxContext::ingredient(db).data(db.as_dyn_database(), id); + fields.outer_transparency } pub fn edition(self, db: &'db Db) -> Edition where Db: ?Sized + zalsa_::Database, { - if self.is_root() { - return Edition::from_u32(SyntaxContext::MAX_ID - self.0.as_u32()); + match self.as_salsa_id() { + Some(id) => { + let fields = SyntaxContext::ingredient(db).data(db.as_dyn_database(), id); + fields.edition + } + None => Edition::from_u32(SyntaxContext::MAX_ID - self.into_u32()), } - let fields = SyntaxContext::ingredient(db).fields(db.as_dyn_database(), self); - std::clone::Clone::clone(&fields.edition) } pub fn parent(self, db: &'db Db) -> SyntaxContext where Db: ?Sized + zalsa_::Database, { - if self.is_root() { - return self; + match self.as_salsa_id() { + Some(id) => { + let fields = SyntaxContext::ingredient(db).data(db.as_dyn_database(), id); + fields.parent + } + None => self, } - let fields = SyntaxContext::ingredient(db).fields(db.as_dyn_database(), self); - std::clone::Clone::clone(&fields.parent) } /// This context, but with all transparent and semi-transparent expansions filtered away. @@ -255,11 +259,13 @@ const _: () = { where Db: ?Sized + zalsa_::Database, { - if self.is_root() { - return self; + match self.as_salsa_id() { + Some(id) => { + let fields = SyntaxContext::ingredient(db).data(db.as_dyn_database(), id); + fields.opaque + } + None => self, } - let fields = SyntaxContext::ingredient(db).fields(db.as_dyn_database(), self); - std::clone::Clone::clone(&fields.opaque) } /// This context, but with all transparent expansions filtered away. @@ -267,33 +273,19 @@ const _: () = { where Db: ?Sized + zalsa_::Database, { - if self.is_root() { - return self; + match self.as_salsa_id() { + Some(id) => { + let fields = SyntaxContext::ingredient(db).data(db.as_dyn_database(), id); + fields.opaque_and_semitransparent + } + None => self, } - let fields = SyntaxContext::ingredient(db).fields(db.as_dyn_database(), self); - std::clone::Clone::clone(&fields.opaque_and_semitransparent) - } - - pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - salsa::with_attached_database(|db| { - let fields = SyntaxContext::ingredient(db).fields(db.as_dyn_database(), this); - let mut f = f.debug_struct("SyntaxContextData"); - let f = f.field("outer_expn", &fields.outer_expn); - let f = f.field("outer_transparency", &fields.outer_expn); - let f = f.field("edition", &fields.edition); - let f = f.field("parent", &fields.parent); - let f = f.field("opaque", &fields.opaque); - let f = f.field("opaque_and_semitransparent", &fields.opaque_and_semitransparent); - f.finish() - }) - .unwrap_or_else(|| { - f.debug_tuple("SyntaxContextData").field(&zalsa_::AsId::as_id(&this)).finish() - }) } } }; impl SyntaxContext { + #[inline] pub fn is_root(self) -> bool { (SyntaxContext::MAX_ID - Edition::LATEST as u32) <= self.into_u32() && self.into_u32() <= (SyntaxContext::MAX_ID - Edition::Edition2015 as u32) @@ -307,9 +299,11 @@ impl SyntaxContext { } /// The root context, which is the parent of all other contexts. All [`FileId`]s have this context. + #[inline] pub const fn root(edition: Edition) -> Self { let edition = edition as u32; - SyntaxContext::from_u32(SyntaxContext::MAX_ID - edition) + // SAFETY: Roots are valid `SyntaxContext`s + unsafe { SyntaxContext::from_u32(SyntaxContext::MAX_ID - edition) } } } @@ -317,12 +311,34 @@ impl SyntaxContext { impl SyntaxContext { const MAX_ID: u32 = salsa::Id::MAX_U32 - 1; + #[inline] pub const fn into_u32(self) -> u32 { - self.0.as_u32() + self.0 } - pub const fn from_u32(u32: u32) -> Self { - Self(salsa::Id::from_u32(u32), std::marker::PhantomData) + /// # Safety + /// + /// The ID must be a valid `SyntaxContext`. + #[inline] + pub const unsafe fn from_u32(u32: u32) -> Self { + // INVARIANT: Our precondition. + Self(u32, std::marker::PhantomData) + } + + #[inline] + fn as_salsa_id(self) -> Option { + if self.is_root() { + None + } else { + // SAFETY: By our invariant, this is either a root (which we verified it's not) or a valid `salsa::Id`. + unsafe { Some(salsa::Id::from_u32(self.0)) } + } + } + + #[inline] + fn from_salsa_id(id: salsa::Id) -> Self { + // SAFETY: This comes from a Salsa ID. + unsafe { Self::from_u32(id.as_u32()) } } } #[cfg(not(feature = "salsa"))] @@ -342,7 +358,10 @@ impl SyntaxContext { self.0 } - pub const fn from_u32(u32: u32) -> Self { + /// # Safety + /// + /// None. This is always safe to call without the `salsa` feature. + pub const unsafe fn from_u32(u32: u32) -> Self { Self(u32) } } diff --git a/crates/span/src/lib.rs b/crates/span/src/lib.rs index 67f49928f885..54f90908f367 100644 --- a/crates/span/src/lib.rs +++ b/crates/span/src/lib.rs @@ -184,16 +184,6 @@ impl EditionedFileId { mod salsa { #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct Id(u32); - - impl Id { - pub(crate) const fn from_u32(u32: u32) -> Self { - Self(u32) - } - - pub(crate) const fn as_u32(self) -> u32 { - self.0 - } - } } /// Input to the analyzer is a set of files, where each file is identified by @@ -216,127 +206,11 @@ mod salsa { #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct HirFileId(pub salsa::Id); -impl From for HirFileId { - fn from(value: MacroCallId) -> Self { - value.as_file() - } -} - -impl fmt::Debug for HirFileId { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.repr().fmt(f) - } -} - -impl PartialEq for HirFileId { - fn eq(&self, &other: &FileId) -> bool { - self.file_id().map(EditionedFileId::file_id) == Some(other) - } -} -impl PartialEq for FileId { - fn eq(&self, other: &HirFileId) -> bool { - other.file_id().map(EditionedFileId::file_id) == Some(*self) - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct MacroFileId { - pub macro_call_id: MacroCallId, -} - /// `MacroCallId` identifies a particular macro invocation, like /// `println!("Hello, {}", world)`. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct MacroCallId(pub salsa::Id); -impl MacroCallId { - pub const MAX_ID: u32 = 0x7fff_ffff; - - pub fn as_file(self) -> HirFileId { - MacroFileId { macro_call_id: self }.into() - } - - pub fn as_macro_file(self) -> MacroFileId { - MacroFileId { macro_call_id: self } - } -} - -#[derive(Clone, Copy, PartialEq, Eq, Hash)] -pub enum HirFileIdRepr { - FileId(EditionedFileId), - MacroFile(MacroFileId), -} - -impl fmt::Debug for HirFileIdRepr { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::FileId(arg0) => arg0.fmt(f), - Self::MacroFile(arg0) => { - f.debug_tuple("MacroFile").field(&arg0.macro_call_id.0).finish() - } - } - } -} - -impl From for HirFileId { - #[allow(clippy::let_unit_value)] - fn from(id: EditionedFileId) -> Self { - assert!(id.as_u32() <= Self::MAX_HIR_FILE_ID, "FileId index {} is too large", id.as_u32()); - HirFileId(salsa::Id::from_u32(id.0)) - } -} - -impl From for HirFileId { - #[allow(clippy::let_unit_value)] - fn from(MacroFileId { macro_call_id: MacroCallId(id) }: MacroFileId) -> Self { - let id: u32 = id.as_u32(); - assert!(id <= Self::MAX_HIR_FILE_ID, "MacroCallId index {id} is too large"); - HirFileId(salsa::Id::from_u32(id | Self::MACRO_FILE_TAG_MASK)) - } -} - -impl HirFileId { - const MAX_HIR_FILE_ID: u32 = u32::MAX ^ Self::MACRO_FILE_TAG_MASK; - const MACRO_FILE_TAG_MASK: u32 = 1 << 31; - - #[inline] - pub fn is_macro(self) -> bool { - self.0.as_u32() & Self::MACRO_FILE_TAG_MASK != 0 - } - - #[inline] - pub fn macro_file(self) -> Option { - match self.0.as_u32() & Self::MACRO_FILE_TAG_MASK { - 0 => None, - _ => Some(MacroFileId { - macro_call_id: MacroCallId(salsa::Id::from_u32( - self.0.as_u32() ^ Self::MACRO_FILE_TAG_MASK, - )), - }), - } - } - - #[inline] - pub fn file_id(self) -> Option { - match self.0.as_u32() & Self::MACRO_FILE_TAG_MASK { - 0 => Some(EditionedFileId(self.0.as_u32())), - _ => None, - } - } - - #[inline] - pub fn repr(self) -> HirFileIdRepr { - match self.0.as_u32() & Self::MACRO_FILE_TAG_MASK { - 0 => HirFileIdRepr::FileId(EditionedFileId(self.0.as_u32())), - _ => HirFileIdRepr::MacroFile(MacroFileId { - macro_call_id: MacroCallId(salsa::Id::from_u32( - self.0.as_u32() ^ Self::MACRO_FILE_TAG_MASK, - )), - }), - } - } -} - /// Legacy span type, only defined here as it is still used by the proc-macro server. /// While rust-analyzer doesn't use this anymore at all, RustRover relies on the legacy type for /// proc-macro expansion. From 57c019a3c5ab8552a77b4c6b80f858dfcc302a33 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 14 Apr 2025 09:42:29 +0300 Subject: [PATCH 4/6] Adapt for new cycle handling changing in Salsa --- crates/hir-def/src/lib.rs | 2 +- crates/hir-ty/src/consteval.rs | 10 +- crates/hir-ty/src/db.rs | 29 +- crates/hir-ty/src/drop.rs | 3 +- crates/hir-ty/src/layout.rs | 6 +- crates/hir-ty/src/layout/adt.rs | 4 +- crates/hir-ty/src/lower.rs | 23 +- crates/hir-ty/src/mir.rs | 4 +- crates/hir-ty/src/mir/lower.rs | 5 +- crates/hir-ty/src/mir/monomorphization.rs | 5 +- crates/hir-ty/src/variance.rs | 14 +- crates/query-group-macro/src/lib.rs | 71 ++++- crates/query-group-macro/src/queries.rs | 24 +- crates/query-group-macro/tests/cycle.rs | 265 ------------------ crates/rust-analyzer/src/handlers/dispatch.rs | 7 +- 15 files changed, 133 insertions(+), 339 deletions(-) delete mode 100644 crates/query-group-macro/tests/cycle.rs diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index c2e29edd31a0..737941dba07e 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -619,7 +619,7 @@ impl_from!( /// A constant, which might appears as a const item, an anonymous const block in expressions /// or patterns, or as a constant in types with const generics. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, salsa::Supertype)] pub enum GeneralConstId { ConstId(ConstId), StaticId(StaticId), diff --git a/crates/hir-ty/src/consteval.rs b/crates/hir-ty/src/consteval.rs index 8d66d47334ea..d1a1e135ffff 100644 --- a/crates/hir-ty/src/consteval.rs +++ b/crates/hir-ty/src/consteval.rs @@ -10,7 +10,6 @@ use hir_def::{ type_ref::LiteralConstRef, }; use hir_expand::Lookup; -use salsa::Cycle; use stdx::never; use triomphe::Arc; @@ -220,9 +219,8 @@ pub fn try_const_isize(db: &dyn HirDatabase, c: &Const) -> Option { } } -pub(crate) fn const_eval_recover( +pub(crate) fn const_eval_cycle_result( _: &dyn HirDatabase, - _: &Cycle, _: GeneralConstId, _: Substitution, _: Option>, @@ -230,17 +228,15 @@ pub(crate) fn const_eval_recover( Err(ConstEvalError::MirLowerError(MirLowerError::Loop)) } -pub(crate) fn const_eval_static_recover( +pub(crate) fn const_eval_static_cycle_result( _: &dyn HirDatabase, - _: &Cycle, _: StaticId, ) -> Result { Err(ConstEvalError::MirLowerError(MirLowerError::Loop)) } -pub(crate) fn const_eval_discriminant_recover( +pub(crate) fn const_eval_discriminant_cycle_result( _: &dyn HirDatabase, - _: &Cycle, _: EnumVariantId, ) -> Result { Err(ConstEvalError::MirLowerError(MirLowerError::Loop)) diff --git a/crates/hir-ty/src/db.rs b/crates/hir-ty/src/db.rs index 75a680b5885c..c24ef16b4969 100644 --- a/crates/hir-ty/src/db.rs +++ b/crates/hir-ty/src/db.rs @@ -36,14 +36,14 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { // region:mir #[salsa::invoke(crate::mir::mir_body_query)] - #[salsa::cycle(crate::mir::mir_body_recover)] + #[salsa::cycle(cycle_result = crate::mir::mir_body_cycle_result)] fn mir_body(&self, def: DefWithBodyId) -> Result, MirLowerError>; #[salsa::invoke(crate::mir::mir_body_for_closure_query)] fn mir_body_for_closure(&self, def: InternedClosureId) -> Result, MirLowerError>; #[salsa::invoke(crate::mir::monomorphized_mir_body_query)] - #[salsa::cycle(crate::mir::monomorphized_mir_body_recover)] + #[salsa::cycle(cycle_result = crate::mir::monomorphized_mir_body_cycle_result)] fn monomorphized_mir_body( &self, def: DefWithBodyId, @@ -64,7 +64,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { fn borrowck(&self, def: DefWithBodyId) -> Result, MirLowerError>; #[salsa::invoke(crate::consteval::const_eval_query)] - #[salsa::cycle(crate::consteval::const_eval_recover)] + #[salsa::cycle(cycle_result = crate::consteval::const_eval_cycle_result)] fn const_eval( &self, def: GeneralConstId, @@ -73,11 +73,11 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { ) -> Result; #[salsa::invoke(crate::consteval::const_eval_static_query)] - #[salsa::cycle(crate::consteval::const_eval_static_recover)] + #[salsa::cycle(cycle_result = crate::consteval::const_eval_static_cycle_result)] fn const_eval_static(&self, def: StaticId) -> Result; #[salsa::invoke(crate::consteval::const_eval_discriminant_variant)] - #[salsa::cycle(crate::consteval::const_eval_discriminant_recover)] + #[salsa::cycle(cycle_result = crate::consteval::const_eval_discriminant_cycle_result)] fn const_eval_discriminant(&self, def: EnumVariantId) -> Result; #[salsa::invoke(crate::method_resolution::lookup_impl_method_query)] @@ -91,7 +91,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { // endregion:mir #[salsa::invoke(crate::layout::layout_of_adt_query)] - #[salsa::cycle(crate::layout::layout_of_adt_recover)] + #[salsa::cycle(cycle_result = crate::layout::layout_of_adt_cycle_result)] fn layout_of_adt( &self, def: AdtId, @@ -100,7 +100,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { ) -> Result, LayoutError>; #[salsa::invoke(crate::layout::layout_of_ty_query)] - #[salsa::cycle(crate::layout::layout_of_ty_recover)] + #[salsa::cycle(cycle_result = crate::layout::layout_of_ty_cycle_result)] fn layout_of_ty(&self, ty: Ty, env: Arc) -> Result, LayoutError>; #[salsa::invoke(crate::layout::target_data_layout_query)] @@ -113,8 +113,8 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { #[salsa::transparent] fn ty(&self, def: TyDefId) -> Binders; - #[salsa::cycle(crate::lower::type_for_type_alias_with_diagnostics_query_recover)] #[salsa::invoke(crate::lower::type_for_type_alias_with_diagnostics_query)] + #[salsa::cycle(cycle_result = crate::lower::type_for_type_alias_with_diagnostics_cycle_result)] fn type_for_type_alias_with_diagnostics(&self, def: TypeAliasId) -> (Binders, Diagnostics); /// Returns the type of the value of the given constant, or `None` if the `ValueTyDefId` is @@ -123,7 +123,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { fn value_ty(&self, def: ValueTyDefId) -> Option>; #[salsa::invoke(crate::lower::impl_self_ty_with_diagnostics_query)] - #[salsa::cycle(crate::lower::impl_self_ty_with_diagnostics_recover)] + #[salsa::cycle(cycle_result = crate::lower::impl_self_ty_with_diagnostics_cycle_result)] fn impl_self_ty_with_diagnostics(&self, def: ImplId) -> (Binders, Diagnostics); #[salsa::invoke(crate::lower::impl_self_ty_query)] @@ -165,7 +165,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { fn type_alias_impl_traits(&self, def: TypeAliasId) -> Option>>; #[salsa::invoke(crate::lower::generic_predicates_for_param_query)] - #[salsa::cycle(crate::lower::generic_predicates_for_param_recover)] + #[salsa::cycle(cycle_result = crate::lower::generic_predicates_for_param_cycle_result)] fn generic_predicates_for_param( &self, def: GenericDefId, @@ -194,7 +194,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { fn trait_environment(&self, def: GenericDefId) -> Arc; #[salsa::invoke(crate::lower::generic_defaults_with_diagnostics_query)] - #[salsa::cycle(crate::lower::generic_defaults_with_diagnostics_recover)] + #[salsa::cycle(cycle_result = crate::lower::generic_defaults_with_diagnostics_cycle_result)] fn generic_defaults_with_diagnostics( &self, def: GenericDefId, @@ -282,7 +282,10 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { fn adt_variance(&self, adt_id: AdtId) -> chalk_db::Variances; #[salsa::invoke(crate::variance::variances_of)] - #[salsa::cycle(crate::variance::variances_of_cycle)] + #[salsa::cycle( + cycle_fn = crate::variance::variances_of_cycle_fn, + cycle_initial = crate::variance::variances_of_cycle_initial, + )] fn variances_of(&self, def: GenericDefId) -> Option>; #[salsa::invoke(chalk_db::associated_ty_value_query)] @@ -317,7 +320,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug { ) -> chalk_ir::ProgramClauses; #[salsa::invoke(crate::drop::has_drop_glue)] - #[salsa::cycle(crate::drop::has_drop_glue_recover)] + #[salsa::cycle(cycle_result = crate::drop::has_drop_glue_cycle_result)] fn has_drop_glue(&self, ty: Ty, env: Arc) -> DropGlue; } diff --git a/crates/hir-ty/src/drop.rs b/crates/hir-ty/src/drop.rs index 9ea0b58559f5..9823c854d5b3 100644 --- a/crates/hir-ty/src/drop.rs +++ b/crates/hir-ty/src/drop.rs @@ -193,9 +193,8 @@ fn is_copy(db: &dyn HirDatabase, ty: Ty, env: Arc) -> bool { db.trait_solve(env.krate, env.block, goal).is_some() } -pub(crate) fn has_drop_glue_recover( +pub(crate) fn has_drop_glue_cycle_result( _db: &dyn HirDatabase, - _cycle: &salsa::Cycle, _ty: Ty, _env: Arc, ) -> DropGlue { diff --git a/crates/hir-ty/src/layout.rs b/crates/hir-ty/src/layout.rs index a7a4e8d40481..c253fe25672f 100644 --- a/crates/hir-ty/src/layout.rs +++ b/crates/hir-ty/src/layout.rs @@ -13,7 +13,6 @@ use hir_def::{ use la_arena::{Idx, RawIdx}; use rustc_abi::AddressSpace; use rustc_index::IndexVec; -use salsa::Cycle; use triomphe::Arc; @@ -25,7 +24,7 @@ use crate::{ utils::ClosureSubst, }; -pub(crate) use self::adt::layout_of_adt_recover; +pub(crate) use self::adt::layout_of_adt_cycle_result; pub use self::{adt::layout_of_adt_query, target::target_data_layout_query}; mod adt; @@ -365,9 +364,8 @@ pub fn layout_of_ty_query( Ok(Arc::new(result)) } -pub(crate) fn layout_of_ty_recover( +pub(crate) fn layout_of_ty_cycle_result( _: &dyn HirDatabase, - _: &Cycle, _: Ty, _: Arc, ) -> Result, LayoutError> { diff --git a/crates/hir-ty/src/layout/adt.rs b/crates/hir-ty/src/layout/adt.rs index 73dba300efd9..3a020bf050d6 100644 --- a/crates/hir-ty/src/layout/adt.rs +++ b/crates/hir-ty/src/layout/adt.rs @@ -9,7 +9,6 @@ use hir_def::{ }; use intern::sym; use rustc_index::IndexVec; -use salsa::Cycle; use smallvec::SmallVec; use triomphe::Arc; @@ -131,9 +130,8 @@ fn layout_scalar_valid_range(db: &dyn HirDatabase, def: AdtId) -> (Bound, (get(sym::rustc_layout_scalar_valid_range_start), get(sym::rustc_layout_scalar_valid_range_end)) } -pub(crate) fn layout_of_adt_recover( +pub(crate) fn layout_of_adt_cycle_result( _: &dyn HirDatabase, - _: &Cycle, _: AdtId, _: Substitution, _: Arc, diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index ea42c5929627..2fb6cdc02a76 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -45,7 +45,6 @@ use hir_expand::name::Name; use la_arena::{Arena, ArenaMap}; use rustc_hash::FxHashSet; use rustc_pattern_analysis::Captures; -use salsa::Cycle; use stdx::{impl_from, never}; use triomphe::{Arc, ThinArc}; @@ -961,9 +960,8 @@ pub(crate) fn generic_predicates_for_param_query( GenericPredicates(predicates.is_empty().not().then(|| predicates.into())) } -pub(crate) fn generic_predicates_for_param_recover( +pub(crate) fn generic_predicates_for_param_cycle_result( _db: &dyn HirDatabase, - _cycle: &salsa::Cycle, _def: GenericDefId, _param_id: TypeOrConstParamId, _assoc_name: Option, @@ -1264,9 +1262,8 @@ pub(crate) fn generic_defaults_with_diagnostics_query( } } -pub(crate) fn generic_defaults_with_diagnostics_recover( +pub(crate) fn generic_defaults_with_diagnostics_cycle_result( _db: &dyn HirDatabase, - _cycle: &Cycle, _def: GenericDefId, ) -> (GenericDefaults, Diagnostics) { (GenericDefaults(None), None) @@ -1402,16 +1399,12 @@ fn type_for_enum_variant_constructor( } } -#[salsa::tracked(recovery_fn = type_for_adt_recovery)] +#[salsa::tracked(cycle_result = type_for_adt_cycle_result)] fn type_for_adt_tracked(db: &dyn HirDatabase, adt: AdtId) -> Binders { type_for_adt(db, adt) } -pub(crate) fn type_for_adt_recovery( - db: &dyn HirDatabase, - _cycle: &salsa::Cycle, - adt: AdtId, -) -> Binders { +fn type_for_adt_cycle_result(db: &dyn HirDatabase, adt: AdtId) -> Binders { let generics = generics(db, adt.into()); make_binders(db, &generics, TyKind::Error.intern(Interner)) } @@ -1449,9 +1442,8 @@ pub(crate) fn type_for_type_alias_with_diagnostics_query( (make_binders(db, &generics, inner), diags) } -pub(crate) fn type_for_type_alias_with_diagnostics_query_recover( +pub(crate) fn type_for_type_alias_with_diagnostics_cycle_result( db: &dyn HirDatabase, - _cycle: &salsa::Cycle, adt: TypeAliasId, ) -> (Binders, Diagnostics) { let generics = generics(db, adt.into()); @@ -1555,12 +1547,11 @@ pub(crate) fn const_param_ty_with_diagnostics_query( (ty, create_diagnostics(ctx.diagnostics)) } -pub(crate) fn impl_self_ty_with_diagnostics_recover( +pub(crate) fn impl_self_ty_with_diagnostics_cycle_result( db: &dyn HirDatabase, - _cycle: &salsa::Cycle, impl_id: ImplId, ) -> (Binders, Diagnostics) { - let generics = generics(db, (impl_id).into()); + let generics = generics(db, impl_id.into()); (make_binders(db, &generics, TyKind::Error.intern(Interner)), None) } diff --git a/crates/hir-ty/src/mir.rs b/crates/hir-ty/src/mir.rs index 247f0ec429fc..6dc20203e0ee 100644 --- a/crates/hir-ty/src/mir.rs +++ b/crates/hir-ty/src/mir.rs @@ -41,8 +41,8 @@ use rustc_hash::FxHashMap; use smallvec::{SmallVec, smallvec}; use stdx::{impl_from, never}; -pub(crate) use lower::mir_body_recover; -pub(crate) use monomorphization::monomorphized_mir_body_recover; +pub(crate) use lower::mir_body_cycle_result; +pub(crate) use monomorphization::monomorphized_mir_body_cycle_result; use super::consteval::{intern_const_scalar, try_const_usize}; diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index c2501dc35eae..557027756f39 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -2,7 +2,7 @@ use std::{fmt::Write, iter, mem}; -use base_db::{Crate, salsa::Cycle}; +use base_db::Crate; use chalk_ir::{BoundVar, ConstData, DebruijnIndex, TyKind}; use hir_def::{ AdtId, DefWithBodyId, EnumVariantId, GeneralConstId, HasModule, ItemContainerId, LocalFieldId, @@ -2145,9 +2145,8 @@ pub fn mir_body_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Result Result> { Err(MirLowerError::Loop) diff --git a/crates/hir-ty/src/mir/monomorphization.rs b/crates/hir-ty/src/mir/monomorphization.rs index 91081d70ccde..d4f10c032cb1 100644 --- a/crates/hir-ty/src/mir/monomorphization.rs +++ b/crates/hir-ty/src/mir/monomorphization.rs @@ -313,9 +313,8 @@ pub fn monomorphized_mir_body_query( Ok(Arc::new(body)) } -pub(crate) fn monomorphized_mir_body_recover( - _: &dyn HirDatabase, - _: &salsa::Cycle, +pub(crate) fn monomorphized_mir_body_cycle_result( + _db: &dyn HirDatabase, _: DefWithBodyId, _: Substitution, _: Arc, diff --git a/crates/hir-ty/src/variance.rs b/crates/hir-ty/src/variance.rs index d711f2e57b1e..405398278800 100644 --- a/crates/hir-ty/src/variance.rs +++ b/crates/hir-ty/src/variance.rs @@ -19,10 +19,10 @@ use crate::{ AliasTy, Const, ConstScalar, DynTyExt, GenericArg, GenericArgData, Interner, Lifetime, LifetimeData, Ty, TyKind, }; -use base_db::salsa::Cycle; use chalk_ir::Mutability; use hir_def::signatures::StructFlags; use hir_def::{AdtId, GenericDefId, GenericParamId, VariantId}; +use salsa::CycleRecoveryAction; use std::fmt; use std::ops::Not; use stdx::never; @@ -55,9 +55,17 @@ pub(crate) fn variances_of(db: &dyn HirDatabase, def: GenericDefId) -> Option>, + _count: u32, + _def: GenericDefId, +) -> CycleRecoveryAction>> { + CycleRecoveryAction::Fallback(result.clone()) +} + +pub(crate) fn variances_of_cycle_initial( db: &dyn HirDatabase, - _cycle: &Cycle, def: GenericDefId, ) -> Option> { let generics = generics(db, def); diff --git a/crates/query-group-macro/src/lib.rs b/crates/query-group-macro/src/lib.rs index 2e2a24908ede..3ade12733a75 100644 --- a/crates/query-group-macro/src/lib.rs +++ b/crates/query-group-macro/src/lib.rs @@ -10,10 +10,13 @@ use queries::{ Queries, SetterKind, TrackedQuery, Transparent, }; use quote::{ToTokens, format_ident, quote}; +use syn::parse::{Parse, ParseStream}; +use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::visit_mut::VisitMut; use syn::{ - Attribute, FnArg, ItemTrait, Path, TraitItem, TraitItemFn, parse_quote, parse_quote_spanned, + Attribute, FnArg, ItemTrait, Path, Token, TraitItem, TraitItemFn, parse_quote, + parse_quote_spanned, }; mod queries; @@ -106,6 +109,66 @@ enum QueryKind { Interned, } +#[derive(Default, Debug, Clone)] +struct Cycle { + cycle_fn: Option<(syn::Ident, Path)>, + cycle_initial: Option<(syn::Ident, Path)>, + cycle_result: Option<(syn::Ident, Path)>, +} + +impl Parse for Cycle { + fn parse(input: ParseStream<'_>) -> syn::Result { + let options = Punctuated::::parse_terminated(input)?; + let mut cycle_fn = None; + let mut cycle_initial = None; + let mut cycle_result = None; + for option in options { + let name = option.name.to_string(); + match &*name { + "cycle_fn" => { + if cycle_fn.is_some() { + return Err(syn::Error::new_spanned(&option.name, "duplicate option")); + } + cycle_fn = Some((option.name, option.value)); + } + "cycle_initial" => { + if cycle_initial.is_some() { + return Err(syn::Error::new_spanned(&option.name, "duplicate option")); + } + cycle_initial = Some((option.name, option.value)); + } + "cycle_result" => { + if cycle_result.is_some() { + return Err(syn::Error::new_spanned(&option.name, "duplicate option")); + } + cycle_result = Some((option.name, option.value)); + } + _ => { + return Err(syn::Error::new_spanned( + &option.name, + "unknown cycle option. Accepted values: `cycle_result`, `cycle_fn`, `cycle_initial`", + )); + } + } + } + return Ok(Self { cycle_fn, cycle_initial, cycle_result }); + + struct Option { + name: syn::Ident, + value: Path, + } + + impl Parse for Option { + fn parse(input: ParseStream) -> syn::Result { + let name = input.parse()?; + input.parse::()?; + let value = input.parse()?; + Ok(Self { name, value }) + } + } + } +} + pub(crate) fn query_group_impl( _args: proc_macro::TokenStream, input: proc_macro::TokenStream, @@ -155,8 +218,8 @@ pub(crate) fn query_group_impl( for SalsaAttr { name, tts, span } in salsa_attrs { match name.as_str() { "cycle" => { - let path = syn::parse::>(tts)?; - cycle = Some(path.0.clone()) + let c = syn::parse::>(tts)?; + cycle = Some(c.0); } "input" => { if !pat_and_tys.is_empty() { @@ -415,7 +478,7 @@ impl syn::parse::Parse for Parenthesized where T: syn::parse::Parse, { - fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result { + fn parse(input: ParseStream<'_>) -> syn::Result { let content; syn::parenthesized!(content in input); content.parse::().map(Parenthesized) diff --git a/crates/query-group-macro/src/queries.rs b/crates/query-group-macro/src/queries.rs index 20458acd552c..edbb645bf421 100644 --- a/crates/query-group-macro/src/queries.rs +++ b/crates/query-group-macro/src/queries.rs @@ -3,13 +3,15 @@ use quote::{ToTokens, format_ident, quote, quote_spanned}; use syn::{FnArg, Ident, PatType, Path, Receiver, ReturnType, Type, parse_quote, spanned::Spanned}; +use crate::Cycle; + pub(crate) struct TrackedQuery { pub(crate) trait_name: Ident, pub(crate) signature: syn::Signature, pub(crate) pat_and_tys: Vec, pub(crate) invoke: Option, pub(crate) default: Option, - pub(crate) cycle: Option, + pub(crate) cycle: Option, pub(crate) lru: Option, pub(crate) generated_struct: Option, } @@ -34,12 +36,20 @@ impl ToTokens for TrackedQuery { let fn_ident = &sig.ident; let shim: Ident = format_ident!("{}_shim", fn_ident); - let annotation = match (self.cycle.clone(), self.lru) { - (Some(cycle), Some(lru)) => quote!(#[salsa::tracked(lru = #lru, recovery_fn = #cycle)]), - (Some(cycle), None) => quote!(#[salsa::tracked(recovery_fn = #cycle)]), - (None, Some(lru)) => quote!(#[salsa::tracked(lru = #lru)]), - (None, None) => quote!(#[salsa::tracked]), - }; + let options = self + .cycle + .as_ref() + .map(|Cycle { cycle_fn, cycle_initial, cycle_result }| { + let cycle_fn = cycle_fn.as_ref().map(|(ident, path)| quote!(#ident=#path)); + let cycle_initial = + cycle_initial.as_ref().map(|(ident, path)| quote!(#ident=#path)); + let cycle_result = cycle_result.as_ref().map(|(ident, path)| quote!(#ident=#path)); + let options = cycle_fn.into_iter().chain(cycle_initial).chain(cycle_result); + quote!(#(#options),*) + }) + .into_iter() + .chain(self.lru.map(|lru| quote!(lru = #lru))); + let annotation = quote!(#[salsa::tracked( #(#options),* )]); let pat_and_tys = &self.pat_and_tys; let params = self diff --git a/crates/query-group-macro/tests/cycle.rs b/crates/query-group-macro/tests/cycle.rs deleted file mode 100644 index 8d195cbd8d59..000000000000 --- a/crates/query-group-macro/tests/cycle.rs +++ /dev/null @@ -1,265 +0,0 @@ -use std::panic::UnwindSafe; - -use expect_test::expect; -use query_group_macro::query_group; -use salsa::Setter; - -/// The queries A, B, and C in `Database` can be configured -/// to invoke one another in arbitrary ways using this -/// enum. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -enum CycleQuery { - None, - A, - B, - C, - AthenC, -} - -#[salsa::input] -struct ABC { - a: CycleQuery, - b: CycleQuery, - c: CycleQuery, -} - -impl CycleQuery { - fn invoke(self, db: &dyn CycleDatabase, abc: ABC) -> Result<(), Error> { - match self { - CycleQuery::A => db.cycle_a(abc), - CycleQuery::B => db.cycle_b(abc), - CycleQuery::C => db.cycle_c(abc), - CycleQuery::AthenC => { - let _ = db.cycle_a(abc); - db.cycle_c(abc) - } - CycleQuery::None => Ok(()), - } - } -} - -#[salsa::input] -struct MyInput {} - -#[salsa::tracked] -fn memoized_a(db: &dyn CycleDatabase, input: MyInput) { - memoized_b(db, input) -} - -#[salsa::tracked] -fn memoized_b(db: &dyn CycleDatabase, input: MyInput) { - memoized_a(db, input) -} - -#[salsa::tracked] -fn volatile_a(db: &dyn CycleDatabase, input: MyInput) { - db.report_untracked_read(); - volatile_b(db, input) -} - -#[salsa::tracked] -fn volatile_b(db: &dyn CycleDatabase, input: MyInput) { - db.report_untracked_read(); - volatile_a(db, input) -} - -#[track_caller] -fn extract_cycle(f: impl FnOnce() + UnwindSafe) -> salsa::Cycle { - let v = std::panic::catch_unwind(f); - if let Err(d) = &v { - if let Some(cycle) = d.downcast_ref::() { - return cycle.clone(); - } - } - panic!("unexpected value: {:?}", v) -} - -#[derive(PartialEq, Eq, Hash, Clone, Debug)] -struct Error { - cycle: Vec, -} - -#[query_group] -trait CycleDatabase: salsa::Database { - #[salsa::cycle(recover_a)] - fn cycle_a(&self, abc: ABC) -> Result<(), Error>; - - #[salsa::cycle(recover_b)] - fn cycle_b(&self, abc: ABC) -> Result<(), Error>; - - fn cycle_c(&self, abc: ABC) -> Result<(), Error>; -} - -fn cycle_a(db: &dyn CycleDatabase, abc: ABC) -> Result<(), Error> { - abc.a(db).invoke(db, abc) -} - -fn recover_a(_db: &dyn CycleDatabase, cycle: &salsa::Cycle, _abc: ABC) -> Result<(), Error> { - Err(Error { cycle: cycle.participant_keys().map(|k| format!("{k:?}")).collect() }) -} - -fn cycle_b(db: &dyn CycleDatabase, abc: ABC) -> Result<(), Error> { - abc.b(db).invoke(db, abc) -} - -fn recover_b(_db: &dyn CycleDatabase, cycle: &salsa::Cycle, _abc: ABC) -> Result<(), Error> { - Err(Error { cycle: cycle.participant_keys().map(|k| format!("{k:?}")).collect() }) -} - -fn cycle_c(db: &dyn CycleDatabase, abc: ABC) -> Result<(), Error> { - abc.c(db).invoke(db, abc) -} - -#[test] -fn cycle_memoized() { - let db = salsa::DatabaseImpl::new(); - - let input = MyInput::new(&db); - let cycle = extract_cycle(|| memoized_a(&db, input)); - let expected = expect![[r#" - [ - DatabaseKeyIndex( - IngredientIndex( - 1, - ), - Id(0), - ), - DatabaseKeyIndex( - IngredientIndex( - 2, - ), - Id(0), - ), - ] - "#]]; - expected.assert_debug_eq(&cycle.all_participants(&db)); -} - -#[test] -fn inner_cycle() { - // A --> B <-- C - // ^ | - // +-----+ - let db = salsa::DatabaseImpl::new(); - - let abc = ABC::new(&db, CycleQuery::B, CycleQuery::A, CycleQuery::B); - let err = db.cycle_c(abc); - assert!(err.is_err()); - let expected = expect![[r#" - [ - "cycle_a_shim(Id(0))", - "cycle_b_shim(Id(0))", - ] - "#]]; - expected.assert_debug_eq(&err.unwrap_err().cycle); -} - -#[test] -fn cycle_revalidate() { - // A --> B - // ^ | - // +-----+ - let mut db = salsa::DatabaseImpl::new(); - let abc = ABC::new(&db, CycleQuery::B, CycleQuery::A, CycleQuery::None); - assert!(db.cycle_a(abc).is_err()); - abc.set_b(&mut db).to(CycleQuery::A); // same value as default - assert!(db.cycle_a(abc).is_err()); -} - -#[test] -fn cycle_recovery_unchanged_twice() { - // A --> B - // ^ | - // +-----+ - let mut db = salsa::DatabaseImpl::new(); - let abc = ABC::new(&db, CycleQuery::B, CycleQuery::A, CycleQuery::None); - assert!(db.cycle_a(abc).is_err()); - - abc.set_c(&mut db).to(CycleQuery::A); // force new revision - assert!(db.cycle_a(abc).is_err()); -} - -#[test] -fn cycle_appears() { - let mut db = salsa::DatabaseImpl::new(); - // A --> B - let abc = ABC::new(&db, CycleQuery::B, CycleQuery::None, CycleQuery::None); - assert!(db.cycle_a(abc).is_ok()); - - // A --> B - // ^ | - // +-----+ - abc.set_b(&mut db).to(CycleQuery::A); - assert!(db.cycle_a(abc).is_err()); -} - -#[test] -fn cycle_disappears() { - let mut db = salsa::DatabaseImpl::new(); - - // A --> B - // ^ | - // +-----+ - let abc = ABC::new(&db, CycleQuery::B, CycleQuery::A, CycleQuery::None); - assert!(db.cycle_a(abc).is_err()); - - // A --> B - abc.set_b(&mut db).to(CycleQuery::None); - assert!(db.cycle_a(abc).is_ok()); -} - -#[test] -fn cycle_multiple() { - // No matter whether we start from A or B, we get the same set of participants: - let db = salsa::DatabaseImpl::new(); - - // Configuration: - // - // A --> B <-- C - // ^ | ^ - // +-----+ | - // | | - // +-----+ - // - // Here, conceptually, B encounters a cycle with A and then - // recovers. - let abc = ABC::new(&db, CycleQuery::B, CycleQuery::AthenC, CycleQuery::A); - - let c = db.cycle_c(abc); - let b = db.cycle_b(abc); - let a = db.cycle_a(abc); - let expected = expect![[r#" - ( - [ - "cycle_a_shim(Id(0))", - "cycle_b_shim(Id(0))", - ], - [ - "cycle_a_shim(Id(0))", - "cycle_b_shim(Id(0))", - ], - [ - "cycle_a_shim(Id(0))", - "cycle_b_shim(Id(0))", - ], - ) - "#]]; - expected.assert_debug_eq(&(c.unwrap_err().cycle, b.unwrap_err().cycle, a.unwrap_err().cycle)); -} - -#[test] -fn cycle_mixed_1() { - let db = salsa::DatabaseImpl::new(); - // A --> B <-- C - // | ^ - // +-----+ - let abc = ABC::new(&db, CycleQuery::B, CycleQuery::C, CycleQuery::B); - - let expected = expect![[r#" - [ - "cycle_b_shim(Id(0))", - "cycle_c_shim(Id(0))", - ] - "#]]; - expected.assert_debug_eq(&db.cycle_c(abc).unwrap_err().cycle); -} diff --git a/crates/rust-analyzer/src/handlers/dispatch.rs b/crates/rust-analyzer/src/handlers/dispatch.rs index de0a5aba8944..3b76edf528b6 100644 --- a/crates/rust-analyzer/src/handlers/dispatch.rs +++ b/crates/rust-analyzer/src/handlers/dispatch.rs @@ -4,7 +4,7 @@ use std::{ panic, thread, }; -use ide_db::base_db::salsa::{self, Cancelled, Cycle}; +use ide_db::base_db::salsa::{self, Cancelled}; use lsp_server::{ExtractError, Response, ResponseError}; use serde::{Serialize, de::DeserializeOwned}; use stdx::thread::ThreadIntent; @@ -309,14 +309,12 @@ impl RequestDispatcher<'_> { #[derive(Debug)] enum HandlerCancelledError { - PropagatedPanic, Inner(salsa::Cancelled), } impl std::error::Error for HandlerCancelledError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { - HandlerCancelledError::PropagatedPanic => None, HandlerCancelledError::Inner(cancelled) => Some(cancelled), } } @@ -349,9 +347,6 @@ where if let Some(panic_message) = panic_message { message.push_str(": "); message.push_str(panic_message) - } else if let Some(cycle) = panic.downcast_ref::() { - tracing::error!("Cycle propagated out of salsa! This is a bug: {cycle:?}"); - return Err(HandlerCancelledError::PropagatedPanic); } else if let Ok(cancelled) = panic.downcast::() { tracing::error!("Cancellation propagated out of salsa! This is a bug"); return Err(HandlerCancelledError::Inner(*cancelled)); From 547c124c0505778807a49dc44080ba6835721f48 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Fri, 18 Apr 2025 06:56:10 +0300 Subject: [PATCH 5/6] Fix variance This one does need fixpoint. --- crates/hir-ty/src/variance.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/crates/hir-ty/src/variance.rs b/crates/hir-ty/src/variance.rs index 405398278800..4e9aa5610a52 100644 --- a/crates/hir-ty/src/variance.rs +++ b/crates/hir-ty/src/variance.rs @@ -57,11 +57,11 @@ pub(crate) fn variances_of(db: &dyn HirDatabase, def: GenericDefId) -> Option>, + _result: &Option>, _count: u32, _def: GenericDefId, ) -> CycleRecoveryAction>> { - CycleRecoveryAction::Fallback(result.clone()) + CycleRecoveryAction::Iterate } pub(crate) fn variances_of_cycle_initial( @@ -961,16 +961,12 @@ struct S3(S); #[test] fn prove_fixedpoint() { - // FIXME: This is wrong, this should be `FixedPoint[T: covariant, U: covariant, V: covariant]` - // This is a limitation of current salsa where a cycle may only set a fallback value to the - // query result, but we need to solve a fixpoint here. The new salsa will have this - // fortunately. check( r#" struct FixedPoint(&'static FixedPoint<(), T, U>, V); "#, expect![[r#" - FixedPoint[T: bivariant, U: bivariant, V: bivariant] + FixedPoint[T: covariant, U: covariant, V: covariant] "#]], ); } From 6e4abf126e3c4adf6e91347529454fb462884217 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Tue, 22 Apr 2025 14:02:55 +0300 Subject: [PATCH 6/6] Account for `IngredientCache::get_or_create()` taking `&Zalsa` and not `&dyn Database` --- crates/span/src/hygiene.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/span/src/hygiene.rs b/crates/span/src/hygiene.rs index 6022b5b12094..b21102f2db71 100644 --- a/crates/span/src/hygiene.rs +++ b/crates/span/src/hygiene.rs @@ -112,7 +112,7 @@ const _: () = { { static CACHE: zalsa_::IngredientCache> = zalsa_::IngredientCache::new(); - CACHE.get_or_create(db.as_dyn_database(), || { + CACHE.get_or_create(db.zalsa(), || { db.zalsa().add_or_lookup_jar_by_type::>() }) }