Draft
Conversation
…d new flags to main.rs, fixed typo in readme
… and lifetimes for visitors
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Switch AST + Typechecker to arena-backed data structures
Our parser and typechecker moved a lot of data around (Boxes, Vecs, Rc-like pointers), creating churn for the borrow checker and unnecessary heap traffic. This PR introduces an arena-backed AST and refactors large parts of the type checker to work with
&'areferences andBumpVec<'a>instead of owning containers. The goals:What’s in this PR
1) New arena AST and conversion from heap AST
arena_ast::...) with&'afields andBumpVec<'a>collections.clone_in(&Bump) -> Selfutility:derive(Clone)preserves allocations and lifetimes. For fields tied to an arena—e.g.,&'a TorBumpVec<'a, T>—Cloneeither copies the reference or allocates in the same arena. It cannot migrate data to a different arena.clone_in(&arena)rebinds to a (new) arena. Theclone_inimplementations re-allocate all arena-owned pieces in the givenarenaand recursively convert children, producing a value whose references andBumpVec<'a, _>now live off the target arena.Parser integration
This preserves the existing parser (which doesn’t support direct arena allocation) and converts the parsed tree into the arena.
2) Visitors and “mutating” updates in an arena world
We can’t mutably borrow fields behind shared references. So the
VisitMut-style passes follow a clone–edit–reassign pattern:This keeps the API “mutating” from the outside (callers pass
&mut), but internally:This same technique is applied throughout: data types, exec expressions, views, etc.
3) Type checking refactor (subty, unify, pre_decl, pl_expr, infer_kinded_args)
Replaced
Vec/BoxwithBumpVec<'a>/&'a Tconsistently.Added
clone_inmethods for types likeExecExpr,ExecExprKind,ExecPathElem,View,ArgKinded,FunDef, etc.Reworked lifetime-heavy substitution and unification:
arena.alloc(...)).bind_to, we allocate aterm_refin the arena and substitute into all map values with that reference (should avoid self-referential/lifetime pitfalls).PlaceExpr.tymoved fromOption<&'a Ty<'a>>toOnceCell<&'a Ty<'a>>, because we now compute types lazily while walking the AST and want a write-once set.pl_expr highlights
ty_check_and_passed_mems_prvs(and helpers) now take a&PlaceExprby default; we only require&mut PlaceExprwhen we truly need to rewrite that node. The computed type is stored via the node’sOnceCell(interior mutability), and we return the passed memories and provenances. This should avoid borrow conflicts and unnecessary AST mutation.ty_check_view_pl_exprtakes&PlaceExprand&Viewimmutably, clones theViewinto the arena (editablev_tmp), performs inference and substitution on the clone, and does not write back to the original AST. We only need the resulting type plus (mems, prvs), which are returned.infer_kinded_args
BumpVec<'a, ArgKinded<'a>>when called from arena contexts (added a helper returning bump vectors). This avoids mixingalloc::VecwithBumpVec.API changes
PlaceExpr.tyis nowOnceCell<&'a Ty<'a>>.&'a Bumpand/or returnBumpVec<'a, _>instead ofVec.clone_ininstead of plainclone()when a value must live in the arena.constrain/substitute/ty_check_*signatures now accept arena and work with references (no Boxes).Testing changes
Bumpand construct types viaDataTy::new(&arena, ...).&'a DataTy<'a>etc.), we allocatearena.alloc(...).constrain/substitute.Example adapted test:
Open items / follow-ups
ty_check/mod.rsborrows and mutabilityGlobalCtxholding&mut CompilUnitwhile also errors have to be emited (err.emit(compil_unit.source)).gl_ctxis dropped before emitting. Alternatively, makeemittake a snapshot or&Source, not&mut CompilUnit.GlobalCtx decl storage safety
compil_unit.items. Replace witharena.alloc_str(name)+clone_ininto the arena (already done in most places; a final pass should remove the remaining cases).More
clone_incoverageclone_infor many core types (e.g.ExecExprKind,ExecPathElem,View,ArgKinded,FunDef). Should finish the remaining node kinds for consistency.Benchmarks
TL;DR
clone_inutilities andinto_arenaconversions.&'arefs andBumpVec<'a>.PlaceExpr.tytoOnceCell<&'a Ty<'a>>.mod.rs, API unification).