From 92b2f363974c354a67e497f71e625b5eeb721f7e Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sat, 20 Dec 2025 22:13:04 -0300 Subject: [PATCH] refactor: Simplify multi-file system --- crates/plotnik-lib/src/lib.rs | 2 +- crates/plotnik-lib/src/query/dependencies.rs | 19 --- crates/plotnik-lib/src/query/query.rs | 7 +- crates/plotnik-lib/src/query/source_map.rs | 153 +++++-------------- 4 files changed, 44 insertions(+), 137 deletions(-) diff --git a/crates/plotnik-lib/src/lib.rs b/crates/plotnik-lib/src/lib.rs index 0506747a..426b770a 100644 --- a/crates/plotnik-lib/src/lib.rs +++ b/crates/plotnik-lib/src/lib.rs @@ -11,7 +11,7 @@ //! "#; //! //! let query = Query::try_from(source).expect("out of fuel"); -//! eprintln!("{}", query.diagnostics().render(source)); +//! eprintln!("{}", query.diagnostics().render(query.source_map())); //! ``` #![cfg_attr(coverage_nightly, feature(coverage_attribute))] diff --git a/crates/plotnik-lib/src/query/dependencies.rs b/crates/plotnik-lib/src/query/dependencies.rs index d3d5b9ed..6af16937 100644 --- a/crates/plotnik-lib/src/query/dependencies.rs +++ b/crates/plotnik-lib/src/query/dependencies.rs @@ -31,25 +31,6 @@ pub struct DependencyAnalysis<'q> { pub sccs: Vec>, } -/// Owned variant of `DependencyAnalysis` for storage in pipeline structs. -#[derive(Debug, Clone, Default)] -pub struct DependencyAnalysisOwned { - #[allow(dead_code)] - pub sccs: Vec>, -} - -impl DependencyAnalysis<'_> { - pub fn to_owned(&self) -> DependencyAnalysisOwned { - DependencyAnalysisOwned { - sccs: self - .sccs - .iter() - .map(|scc| scc.iter().map(|s| (*s).to_owned()).collect()) - .collect(), - } - } -} - /// Analyze dependencies between definitions. /// /// Returns the SCCs in reverse topological order. diff --git a/crates/plotnik-lib/src/query/query.rs b/crates/plotnik-lib/src/query/query.rs index 63e2c4ee..780b6920 100644 --- a/crates/plotnik-lib/src/query/query.rs +++ b/crates/plotnik-lib/src/query/query.rs @@ -10,7 +10,7 @@ use plotnik_langs::Lang; use crate::Diagnostics; use crate::parser::{ParseResult, Parser, Root, SyntaxNode, lexer::lex}; use crate::query::alt_kinds::validate_alt_kinds; -use crate::query::dependencies::{self, DependencyAnalysisOwned}; +use crate::query::dependencies; use crate::query::expr_arity::{ExprArity, ExprArityTable, infer_arities, resolve_arity}; use crate::query::link; use crate::query::source_map::{SourceId, SourceMap}; @@ -116,15 +116,11 @@ impl QueryParsed { ); let arity_table = infer_arities(&self.ast_map, &symbol_table, &mut self.diag); - - // Convert to owned for storage let symbol_table_owned = crate::query::symbol_table::to_owned(symbol_table); - let dependency_analysis_owned = dependency_analysis.to_owned(); QueryAnalyzed { query_parsed: self, symbol_table: symbol_table_owned, - dependency_analysis: dependency_analysis_owned, arity_table, } } @@ -147,7 +143,6 @@ pub type Query = QueryAnalyzed; pub struct QueryAnalyzed { query_parsed: QueryParsed, pub symbol_table: SymbolTableOwned, - dependency_analysis: DependencyAnalysisOwned, arity_table: ExprArityTable, } diff --git a/crates/plotnik-lib/src/query/source_map.rs b/crates/plotnik-lib/src/query/source_map.rs index 16b9b9c8..73b047f6 100644 --- a/crates/plotnik-lib/src/query/source_map.rs +++ b/crates/plotnik-lib/src/query/source_map.rs @@ -1,27 +1,24 @@ -//! Arena-based source storage for unified lifetimes. +//! Source storage for query compilation. //! -//! All sources are stored in a single contiguous buffer, allowing all string slices -//! to share the same lifetime as `&SourceMap`. This eliminates lifetime complexity -//! when multiple sources need to be analyzed together. - -use std::ops::Range; +//! Stores sources as owned strings, providing a simple interface for +//! multi-source compilation sessions. /// Lightweight handle to a source in a compilation session. #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Default)] pub struct SourceId(u32); /// Describes the origin of a source. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub enum SourceKind<'a> { +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum SourceKind { /// A one-liner query passed directly (e.g., CLI `-q` argument). OneLiner, /// Input read from stdin. Stdin, /// A file with its path. - File(&'a str), + File(String), } -impl SourceKind<'_> { +impl SourceKind { /// Returns the display name for diagnostics. pub fn display_name(&self) -> &str { match self { @@ -33,11 +30,10 @@ impl SourceKind<'_> { } /// A borrowed view of a source: id, kind, and content. -/// All string slices share the lifetime of the `SourceMap`. -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug)] pub struct Source<'a> { pub id: SourceId, - pub kind: SourceKind<'a>, + pub kind: &'a SourceKind, pub content: &'a str, } @@ -48,32 +44,16 @@ impl<'a> Source<'a> { } } -/// Internal representation of source kind, storing ranges instead of slices. -#[derive(Clone, Debug)] -enum SourceKindEntry { - OneLiner, - Stdin, - /// Stores the byte range of the filename in the shared buffer. - File { - name_range: Range, - }, -} - -/// Metadata for a source in the arena. +/// Metadata for a source. #[derive(Clone, Debug)] struct SourceEntry { - kind: SourceKindEntry, - /// Byte range of content in the shared buffer. - content_range: Range, + kind: SourceKind, + content: String, } -/// Arena-based registry of all sources. Owns a single buffer. -/// -/// All content slices returned have the same lifetime as `&SourceMap`, -/// eliminating the need for separate lifetimes per source file. +/// Registry of all sources. #[derive(Clone, Debug, Default)] pub struct SourceMap { - buffer: String, entries: Vec, } @@ -84,30 +64,17 @@ impl SourceMap { /// Add a one-liner source (CLI `-q` argument, REPL, tests). pub fn add_one_liner(&mut self, content: &str) -> SourceId { - let content_range = self.push_content(content); - self.push_entry(SourceKindEntry::OneLiner, content_range) + self.push_entry(SourceKind::OneLiner, content) } /// Add a source read from stdin. pub fn add_stdin(&mut self, content: &str) -> SourceId { - let content_range = self.push_content(content); - self.push_entry(SourceKindEntry::Stdin, content_range) + self.push_entry(SourceKind::Stdin, content) } /// Add a file source with its path. pub fn add_file(&mut self, path: &str, content: &str) -> SourceId { - let name_start = self.buffer.len() as u32; - self.buffer.push_str(path); - let name_end = self.buffer.len() as u32; - - let content_range = self.push_content(content); - - self.push_entry( - SourceKindEntry::File { - name_range: name_start..name_end, - }, - content_range, - ) + self.push_entry(SourceKind::File(path.to_owned()), content) } /// Create a SourceMap with a single one-liner source. @@ -122,15 +89,15 @@ impl SourceMap { pub fn content(&self, id: SourceId) -> &str { self.entries .get(id.0 as usize) - .map(|e| self.slice(&e.content_range)) + .map(|e| e.content.as_str()) .expect("invalid SourceId") } /// Get the kind of a source by ID. - pub fn kind(&self, id: SourceId) -> SourceKind<'_> { + pub fn kind(&self, id: SourceId) -> &SourceKind { self.entries .get(id.0 as usize) - .map(|e| self.resolve_kind(&e.kind)) + .map(|e| &e.kind) .expect("invalid SourceId") } @@ -138,7 +105,7 @@ impl SourceMap { pub fn path(&self, id: SourceId) -> Option<&str> { let entry = self.entries.get(id.0 as usize).expect("invalid SourceId"); match &entry.kind { - SourceKindEntry::File { name_range } => Some(self.slice(name_range)), + SourceKind::File(path) => Some(path), _ => None, } } @@ -158,8 +125,8 @@ impl SourceMap { let entry = self.entries.get(id.0 as usize).expect("invalid SourceId"); Source { id, - kind: self.resolve_kind(&entry.kind), - content: self.slice(&entry.content_range), + kind: &entry.kind, + content: &entry.content, } } @@ -167,44 +134,19 @@ impl SourceMap { pub fn iter(&self) -> impl Iterator> { self.entries.iter().enumerate().map(|(idx, entry)| Source { id: SourceId(idx as u32), - kind: self.resolve_kind(&entry.kind), - content: self.slice(&entry.content_range), + kind: &entry.kind, + content: &entry.content, }) } - /// Reserve additional capacity in the buffer. - /// Useful when loading multiple files to avoid reallocations. - pub fn reserve(&mut self, additional: usize) { - self.buffer.reserve(additional); - } - - fn push_content(&mut self, content: &str) -> Range { - let start = self.buffer.len() as u32; - self.buffer.push_str(content); - let end = self.buffer.len() as u32; - start..end - } - - fn push_entry(&mut self, kind: SourceKindEntry, content_range: Range) -> SourceId { + fn push_entry(&mut self, kind: SourceKind, content: &str) -> SourceId { let id = SourceId(self.entries.len() as u32); self.entries.push(SourceEntry { kind, - content_range, + content: content.to_owned(), }); id } - - fn slice(&self, range: &Range) -> &str { - &self.buffer[range.start as usize..range.end as usize] - } - - fn resolve_kind(&self, kind: &SourceKindEntry) -> SourceKind<'_> { - match kind { - SourceKindEntry::OneLiner => SourceKind::OneLiner, - SourceKindEntry::Stdin => SourceKind::Stdin, - SourceKindEntry::File { name_range } => SourceKind::File(self.slice(name_range)), - } - } } #[cfg(test)] @@ -217,7 +159,7 @@ mod tests { let id = SourceId(0); assert_eq!(map.content(id), "hello world"); - assert_eq!(map.kind(id), SourceKind::OneLiner); + assert_eq!(map.kind(id), &SourceKind::OneLiner); assert_eq!(map.len(), 1); } @@ -227,7 +169,7 @@ mod tests { let id = map.add_stdin("from stdin"); assert_eq!(map.content(id), "from stdin"); - assert_eq!(map.kind(id), SourceKind::Stdin); + assert_eq!(map.kind(id), &SourceKind::Stdin); } #[test] @@ -236,7 +178,7 @@ mod tests { let id = map.add_file("main.ptk", "Foo = (bar)"); assert_eq!(map.content(id), "Foo = (bar)"); - assert_eq!(map.kind(id), SourceKind::File("main.ptk")); + assert_eq!(map.kind(id), &SourceKind::File("main.ptk".to_owned())); } #[test] @@ -253,10 +195,10 @@ mod tests { assert_eq!(map.content(c), "inline"); assert_eq!(map.content(d), "piped"); - assert_eq!(map.kind(a), SourceKind::File("a.ptk")); - assert_eq!(map.kind(b), SourceKind::File("b.ptk")); - assert_eq!(map.kind(c), SourceKind::OneLiner); - assert_eq!(map.kind(d), SourceKind::Stdin); + assert_eq!(map.kind(a), &SourceKind::File("a.ptk".to_owned())); + assert_eq!(map.kind(b), &SourceKind::File("b.ptk".to_owned())); + assert_eq!(map.kind(c), &SourceKind::OneLiner); + assert_eq!(map.kind(d), &SourceKind::Stdin); } #[test] @@ -268,10 +210,10 @@ mod tests { let items: Vec<_> = map.iter().collect(); assert_eq!(items.len(), 2); assert_eq!(items[0].id, SourceId(0)); - assert_eq!(items[0].kind, SourceKind::File("a.ptk")); + assert_eq!(items[0].kind, &SourceKind::File("a.ptk".to_owned())); assert_eq!(items[0].content, "aaa"); assert_eq!(items[1].id, SourceId(1)); - assert_eq!(items[1].kind, SourceKind::OneLiner); + assert_eq!(items[1].kind, &SourceKind::OneLiner); assert_eq!(items[1].content, "bbb"); } @@ -282,7 +224,7 @@ mod tests { let source = map.get(id); assert_eq!(source.id, id); - assert_eq!(source.kind, SourceKind::File("test.ptk")); + assert_eq!(source.kind, &SourceKind::File("test.ptk".to_owned())); assert_eq!(source.content, "hello"); assert_eq!(source.as_str(), "hello"); } @@ -291,7 +233,10 @@ mod tests { fn display_name() { assert_eq!(SourceKind::OneLiner.display_name(), ""); assert_eq!(SourceKind::Stdin.display_name(), ""); - assert_eq!(SourceKind::File("foo.ptk").display_name(), "foo.ptk"); + assert_eq!( + SourceKind::File("foo.ptk".to_owned()).display_name(), + "foo.ptk" + ); } #[test] @@ -301,20 +246,6 @@ mod tests { let _ = map.content(SourceId(999)); } - #[test] - fn shared_buffer_lifetime() { - let mut map = SourceMap::new(); - map.add_file("a", "first"); - map.add_file("b", "second"); - - // Both slices have the same lifetime as &map - let a_content = map.content(SourceId(0)); - let b_content = map.content(SourceId(1)); - - // Can use both simultaneously - assert_eq!(format!("{} {}", a_content, b_content), "first second"); - } - #[test] fn multiple_stdin_sources() { let mut map = SourceMap::new(); @@ -323,7 +254,7 @@ mod tests { assert_eq!(map.content(a), "first stdin"); assert_eq!(map.content(b), "second stdin"); - assert_eq!(map.kind(a), SourceKind::Stdin); - assert_eq!(map.kind(b), SourceKind::Stdin); + assert_eq!(map.kind(a), &SourceKind::Stdin); + assert_eq!(map.kind(b), &SourceKind::Stdin); } }