Skip to content

Commit 177cc71

Browse files
zharinovzharinov
authored
refactor: Simplify multi-file system (#143)
1 parent f67de64 commit 177cc71

4 files changed

Lines changed: 44 additions & 137 deletions

File tree

‎crates/plotnik-lib/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//! "#;
1212
//!
1313
//! let query = Query::try_from(source).expect("out of fuel");
14-
//! eprintln!("{}", query.diagnostics().render(source));
14+
//! eprintln!("{}", query.diagnostics().render(query.source_map()));
1515
//! ```
1616
1717
#![cfg_attr(coverage_nightly, feature(coverage_attribute))]

‎crates/plotnik-lib/src/query/dependencies.rs‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,6 @@ pub struct DependencyAnalysis<'q> {
3131
pub sccs: Vec<Vec<&'q str>>,
3232
}
3333

34-
/// Owned variant of `DependencyAnalysis` for storage in pipeline structs.
35-
#[derive(Debug, Clone, Default)]
36-
pub struct DependencyAnalysisOwned {
37-
#[allow(dead_code)]
38-
pub sccs: Vec<Vec<String>>,
39-
}
40-
41-
impl DependencyAnalysis<'_> {
42-
pub fn to_owned(&self) -> DependencyAnalysisOwned {
43-
DependencyAnalysisOwned {
44-
sccs: self
45-
.sccs
46-
.iter()
47-
.map(|scc| scc.iter().map(|s| (*s).to_owned()).collect())
48-
.collect(),
49-
}
50-
}
51-
}
52-
5334
/// Analyze dependencies between definitions.
5435
///
5536
/// Returns the SCCs in reverse topological order.

‎crates/plotnik-lib/src/query/query.rs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use plotnik_langs::Lang;
1010
use crate::Diagnostics;
1111
use crate::parser::{ParseResult, Parser, Root, SyntaxNode, lexer::lex};
1212
use crate::query::alt_kinds::validate_alt_kinds;
13-
use crate::query::dependencies::{self, DependencyAnalysisOwned};
13+
use crate::query::dependencies;
1414
use crate::query::expr_arity::{ExprArity, ExprArityTable, infer_arities, resolve_arity};
1515
use crate::query::link;
1616
use crate::query::source_map::{SourceId, SourceMap};
@@ -116,15 +116,11 @@ impl QueryParsed {
116116
);
117117

118118
let arity_table = infer_arities(&self.ast_map, &symbol_table, &mut self.diag);
119-
120-
// Convert to owned for storage
121119
let symbol_table_owned = crate::query::symbol_table::to_owned(symbol_table);
122-
let dependency_analysis_owned = dependency_analysis.to_owned();
123120

124121
QueryAnalyzed {
125122
query_parsed: self,
126123
symbol_table: symbol_table_owned,
127-
dependency_analysis: dependency_analysis_owned,
128124
arity_table,
129125
}
130126
}
@@ -147,7 +143,6 @@ pub type Query = QueryAnalyzed;
147143
pub struct QueryAnalyzed {
148144
query_parsed: QueryParsed,
149145
pub symbol_table: SymbolTableOwned,
150-
dependency_analysis: DependencyAnalysisOwned,
151146
arity_table: ExprArityTable,
152147
}
153148

‎crates/plotnik-lib/src/query/source_map.rs‎

Lines changed: 42 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,24 @@
1-
//! Arena-based source storage for unified lifetimes.
1+
//! Source storage for query compilation.
22
//!
3-
//! All sources are stored in a single contiguous buffer, allowing all string slices
4-
//! to share the same lifetime as `&SourceMap`. This eliminates lifetime complexity
5-
//! when multiple sources need to be analyzed together.
6-
7-
use std::ops::Range;
3+
//! Stores sources as owned strings, providing a simple interface for
4+
//! multi-source compilation sessions.
85
96
/// Lightweight handle to a source in a compilation session.
107
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Default)]
118
pub struct SourceId(u32);
129

1310
/// Describes the origin of a source.
14-
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
15-
pub enum SourceKind<'a> {
11+
#[derive(Clone, Debug, Eq, PartialEq)]
12+
pub enum SourceKind {
1613
/// A one-liner query passed directly (e.g., CLI `-q` argument).
1714
OneLiner,
1815
/// Input read from stdin.
1916
Stdin,
2017
/// A file with its path.
21-
File(&'a str),
18+
File(String),
2219
}
2320

24-
impl SourceKind<'_> {
21+
impl SourceKind {
2522
/// Returns the display name for diagnostics.
2623
pub fn display_name(&self) -> &str {
2724
match self {
@@ -33,11 +30,10 @@ impl SourceKind<'_> {
3330
}
3431

3532
/// A borrowed view of a source: id, kind, and content.
36-
/// All string slices share the lifetime of the `SourceMap`.
37-
#[derive(Copy, Clone, Debug)]
33+
#[derive(Clone, Debug)]
3834
pub struct Source<'a> {
3935
pub id: SourceId,
40-
pub kind: SourceKind<'a>,
36+
pub kind: &'a SourceKind,
4137
pub content: &'a str,
4238
}
4339

@@ -48,32 +44,16 @@ impl<'a> Source<'a> {
4844
}
4945
}
5046

51-
/// Internal representation of source kind, storing ranges instead of slices.
52-
#[derive(Clone, Debug)]
53-
enum SourceKindEntry {
54-
OneLiner,
55-
Stdin,
56-
/// Stores the byte range of the filename in the shared buffer.
57-
File {
58-
name_range: Range<u32>,
59-
},
60-
}
61-
62-
/// Metadata for a source in the arena.
47+
/// Metadata for a source.
6348
#[derive(Clone, Debug)]
6449
struct SourceEntry {
65-
kind: SourceKindEntry,
66-
/// Byte range of content in the shared buffer.
67-
content_range: Range<u32>,
50+
kind: SourceKind,
51+
content: String,
6852
}
6953

70-
/// Arena-based registry of all sources. Owns a single buffer.
71-
///
72-
/// All content slices returned have the same lifetime as `&SourceMap`,
73-
/// eliminating the need for separate lifetimes per source file.
54+
/// Registry of all sources.
7455
#[derive(Clone, Debug, Default)]
7556
pub struct SourceMap {
76-
buffer: String,
7757
entries: Vec<SourceEntry>,
7858
}
7959

@@ -84,30 +64,17 @@ impl SourceMap {
8464

8565
/// Add a one-liner source (CLI `-q` argument, REPL, tests).
8666
pub fn add_one_liner(&mut self, content: &str) -> SourceId {
87-
let content_range = self.push_content(content);
88-
self.push_entry(SourceKindEntry::OneLiner, content_range)
67+
self.push_entry(SourceKind::OneLiner, content)
8968
}
9069

9170
/// Add a source read from stdin.
9271
pub fn add_stdin(&mut self, content: &str) -> SourceId {
93-
let content_range = self.push_content(content);
94-
self.push_entry(SourceKindEntry::Stdin, content_range)
72+
self.push_entry(SourceKind::Stdin, content)
9573
}
9674

9775
/// Add a file source with its path.
9876
pub fn add_file(&mut self, path: &str, content: &str) -> SourceId {
99-
let name_start = self.buffer.len() as u32;
100-
self.buffer.push_str(path);
101-
let name_end = self.buffer.len() as u32;
102-
103-
let content_range = self.push_content(content);
104-
105-
self.push_entry(
106-
SourceKindEntry::File {
107-
name_range: name_start..name_end,
108-
},
109-
content_range,
110-
)
77+
self.push_entry(SourceKind::File(path.to_owned()), content)
11178
}
11279

11380
/// Create a SourceMap with a single one-liner source.
@@ -122,23 +89,23 @@ impl SourceMap {
12289
pub fn content(&self, id: SourceId) -> &str {
12390
self.entries
12491
.get(id.0 as usize)
125-
.map(|e| self.slice(&e.content_range))
92+
.map(|e| e.content.as_str())
12693
.expect("invalid SourceId")
12794
}
12895

12996
/// Get the kind of a source by ID.
130-
pub fn kind(&self, id: SourceId) -> SourceKind<'_> {
97+
pub fn kind(&self, id: SourceId) -> &SourceKind {
13198
self.entries
13299
.get(id.0 as usize)
133-
.map(|e| self.resolve_kind(&e.kind))
100+
.map(|e| &e.kind)
134101
.expect("invalid SourceId")
135102
}
136103

137104
/// Get the file path if this source is a file, None otherwise.
138105
pub fn path(&self, id: SourceId) -> Option<&str> {
139106
let entry = self.entries.get(id.0 as usize).expect("invalid SourceId");
140107
match &entry.kind {
141-
SourceKindEntry::File { name_range } => Some(self.slice(name_range)),
108+
SourceKind::File(path) => Some(path),
142109
_ => None,
143110
}
144111
}
@@ -158,53 +125,28 @@ impl SourceMap {
158125
let entry = self.entries.get(id.0 as usize).expect("invalid SourceId");
159126
Source {
160127
id,
161-
kind: self.resolve_kind(&entry.kind),
162-
content: self.slice(&entry.content_range),
128+
kind: &entry.kind,
129+
content: &entry.content,
163130
}
164131
}
165132

166133
/// Iterate over all sources as `Source` views.
167134
pub fn iter(&self) -> impl Iterator<Item = Source<'_>> {
168135
self.entries.iter().enumerate().map(|(idx, entry)| Source {
169136
id: SourceId(idx as u32),
170-
kind: self.resolve_kind(&entry.kind),
171-
content: self.slice(&entry.content_range),
137+
kind: &entry.kind,
138+
content: &entry.content,
172139
})
173140
}
174141

175-
/// Reserve additional capacity in the buffer.
176-
/// Useful when loading multiple files to avoid reallocations.
177-
pub fn reserve(&mut self, additional: usize) {
178-
self.buffer.reserve(additional);
179-
}
180-
181-
fn push_content(&mut self, content: &str) -> Range<u32> {
182-
let start = self.buffer.len() as u32;
183-
self.buffer.push_str(content);
184-
let end = self.buffer.len() as u32;
185-
start..end
186-
}
187-
188-
fn push_entry(&mut self, kind: SourceKindEntry, content_range: Range<u32>) -> SourceId {
142+
fn push_entry(&mut self, kind: SourceKind, content: &str) -> SourceId {
189143
let id = SourceId(self.entries.len() as u32);
190144
self.entries.push(SourceEntry {
191145
kind,
192-
content_range,
146+
content: content.to_owned(),
193147
});
194148
id
195149
}
196-
197-
fn slice(&self, range: &Range<u32>) -> &str {
198-
&self.buffer[range.start as usize..range.end as usize]
199-
}
200-
201-
fn resolve_kind(&self, kind: &SourceKindEntry) -> SourceKind<'_> {
202-
match kind {
203-
SourceKindEntry::OneLiner => SourceKind::OneLiner,
204-
SourceKindEntry::Stdin => SourceKind::Stdin,
205-
SourceKindEntry::File { name_range } => SourceKind::File(self.slice(name_range)),
206-
}
207-
}
208150
}
209151

210152
#[cfg(test)]
@@ -217,7 +159,7 @@ mod tests {
217159
let id = SourceId(0);
218160

219161
assert_eq!(map.content(id), "hello world");
220-
assert_eq!(map.kind(id), SourceKind::OneLiner);
162+
assert_eq!(map.kind(id), &SourceKind::OneLiner);
221163
assert_eq!(map.len(), 1);
222164
}
223165

@@ -227,7 +169,7 @@ mod tests {
227169
let id = map.add_stdin("from stdin");
228170

229171
assert_eq!(map.content(id), "from stdin");
230-
assert_eq!(map.kind(id), SourceKind::Stdin);
172+
assert_eq!(map.kind(id), &SourceKind::Stdin);
231173
}
232174

233175
#[test]
@@ -236,7 +178,7 @@ mod tests {
236178
let id = map.add_file("main.ptk", "Foo = (bar)");
237179

238180
assert_eq!(map.content(id), "Foo = (bar)");
239-
assert_eq!(map.kind(id), SourceKind::File("main.ptk"));
181+
assert_eq!(map.kind(id), &SourceKind::File("main.ptk".to_owned()));
240182
}
241183

242184
#[test]
@@ -253,10 +195,10 @@ mod tests {
253195
assert_eq!(map.content(c), "inline");
254196
assert_eq!(map.content(d), "piped");
255197

256-
assert_eq!(map.kind(a), SourceKind::File("a.ptk"));
257-
assert_eq!(map.kind(b), SourceKind::File("b.ptk"));
258-
assert_eq!(map.kind(c), SourceKind::OneLiner);
259-
assert_eq!(map.kind(d), SourceKind::Stdin);
198+
assert_eq!(map.kind(a), &SourceKind::File("a.ptk".to_owned()));
199+
assert_eq!(map.kind(b), &SourceKind::File("b.ptk".to_owned()));
200+
assert_eq!(map.kind(c), &SourceKind::OneLiner);
201+
assert_eq!(map.kind(d), &SourceKind::Stdin);
260202
}
261203

262204
#[test]
@@ -268,10 +210,10 @@ mod tests {
268210
let items: Vec<_> = map.iter().collect();
269211
assert_eq!(items.len(), 2);
270212
assert_eq!(items[0].id, SourceId(0));
271-
assert_eq!(items[0].kind, SourceKind::File("a.ptk"));
213+
assert_eq!(items[0].kind, &SourceKind::File("a.ptk".to_owned()));
272214
assert_eq!(items[0].content, "aaa");
273215
assert_eq!(items[1].id, SourceId(1));
274-
assert_eq!(items[1].kind, SourceKind::OneLiner);
216+
assert_eq!(items[1].kind, &SourceKind::OneLiner);
275217
assert_eq!(items[1].content, "bbb");
276218
}
277219

@@ -282,7 +224,7 @@ mod tests {
282224

283225
let source = map.get(id);
284226
assert_eq!(source.id, id);
285-
assert_eq!(source.kind, SourceKind::File("test.ptk"));
227+
assert_eq!(source.kind, &SourceKind::File("test.ptk".to_owned()));
286228
assert_eq!(source.content, "hello");
287229
assert_eq!(source.as_str(), "hello");
288230
}
@@ -291,7 +233,10 @@ mod tests {
291233
fn display_name() {
292234
assert_eq!(SourceKind::OneLiner.display_name(), "<query>");
293235
assert_eq!(SourceKind::Stdin.display_name(), "<stdin>");
294-
assert_eq!(SourceKind::File("foo.ptk").display_name(), "foo.ptk");
236+
assert_eq!(
237+
SourceKind::File("foo.ptk".to_owned()).display_name(),
238+
"foo.ptk"
239+
);
295240
}
296241

297242
#[test]
@@ -301,20 +246,6 @@ mod tests {
301246
let _ = map.content(SourceId(999));
302247
}
303248

304-
#[test]
305-
fn shared_buffer_lifetime() {
306-
let mut map = SourceMap::new();
307-
map.add_file("a", "first");
308-
map.add_file("b", "second");
309-
310-
// Both slices have the same lifetime as &map
311-
let a_content = map.content(SourceId(0));
312-
let b_content = map.content(SourceId(1));
313-
314-
// Can use both simultaneously
315-
assert_eq!(format!("{} {}", a_content, b_content), "first second");
316-
}
317-
318249
#[test]
319250
fn multiple_stdin_sources() {
320251
let mut map = SourceMap::new();
@@ -323,7 +254,7 @@ mod tests {
323254

324255
assert_eq!(map.content(a), "first stdin");
325256
assert_eq!(map.content(b), "second stdin");
326-
assert_eq!(map.kind(a), SourceKind::Stdin);
327-
assert_eq!(map.kind(b), SourceKind::Stdin);
257+
assert_eq!(map.kind(a), &SourceKind::Stdin);
258+
assert_eq!(map.kind(b), &SourceKind::Stdin);
328259
}
329260
}

0 commit comments

Comments
 (0)