Design the checker crate skeleton (crates/twill-check) #1
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Stand up the new crate, decide how it consumes twill-parser's AST, and settle on the salsa-style incremental architecture called for in project-plan.md's Step 9 note -- retrofitting incrementality later is expensive, so the query structure needs to be right from this step.
Architecture decisions made while working through this:
Use the real
salsacrate, not a hand-rolled memoization layer. Its auto-discovered dependency tracking (recording what each tracked fn reads, invalidating exactly what's affected downstream) is the actual hard part this ticket's incrementality warning is about -- rust-analyzer is built on it for the same IDE-incremental-frontend shape. Tracked as #42.Lower twill-parser's borrowed
Expr<'src>into an owned/interned tree immediately after parsing, rather than trying to carry the'srclifetime through salsa's'db-bound tracked structs. Identifiers get interned as aSymboltype (viasalsa::interned) from the start, not deferred -- that's what gives salsa's early-cutoff a stable identity to compare across revisions, and Step 2's type checker will wantSymbol-shaped identifiers for its symbol table anyway. Tracked as #43.Next up: the concrete
#[salsa::db]trait andSourceFileinput skeleton -- what the Db trait looks like and who owns it, whetherSourceFileneeds to model a multi-file module graph from the start, and whetherparse/lowerare separate tracked fns so downstream queries can skip re-parsing on unrelated edits. Tracked as #44.Correction to the decision recorded above: 'parse and lower are separate #[salsa::tracked] fns' (tracked as #44/#45, now merged) turns out not to hold up.
A tracked fn's return value can never hold data borrowed with the source-text's lifetime -- that's the same SalsaValue restriction that forced #43's lowered tree to be owned/interned in the first place. It applies just as much to parse's success case: the actual Spanned<Module<'src>> tree can never cross a tracked-fn boundary either, so parse can only legally return something owned (diagnostics), never the tree itself.
That breaks the original reasoning: the point of splitting parse out was so lower could depend on parse's cached result instead of re-parsing. But since parse can't hand lower the tree, lower still has to independently re-run the actual lex+parse itself to get anything to walk -- the split doesn't save any recomputation, it just adds a query-graph node.
Going forward: lex+parse becomes a plain, non-tracked helper function (reusable, testable, just not itself a salsa query). The single #[salsa::tracked] fn (lower) calls that helper internally and returns something like Result<hir::Module<'db>, Vec> -- Diagnostic being plain owned data (message + span, no lifetime), trivially SalsaValue-safe.
If a real need for independently-cacheable syntax diagnostics shows up later (Step 9's live-diagnostics work, #39, is the likely candidate), the split can be revisited then with an actual consumer driving the shape, rather than guessed at now.