feat: add multi file support, Implement parser context queue and external symbol registration #102
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements multi-file support for the compiler by introducing a parser context queue system and enabling external symbol registration across modules. The changes allow parsing multiple source files connected via mod declarations and properly registering their definitions in the symbol table.
Changes:
- Added
ParserContextwith queue-based multi-file parsing - Extended symbol table to register external functions, constants, and modules
- Modified code generation to traverse module definitions recursively
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| core/wasm-codegen/src/lib.rs | Removed multi-file restriction and added recursive module compilation |
| core/type-checker/src/type_checker.rs | Refactored type registration and inference to handle nested module definitions |
| core/type-checker/src/symbol_table.rs | Extended external definition registration to support constants, external functions, and modules |
| core/inference/src/lib.rs | Added parse_file entry point for multi-file projects |
| core/cli/src/parser.rs | Updated CLI documentation to reflect multi-file support |
| core/cli/src/main.rs | Changed to use parse_file instead of parse |
| core/ast/src/parser_context.rs | Implemented queue-based parsing with module scanning and submodule resolution |
| core/ast/src/nodes_impl.rs | Changed module body to use RefCell for delayed initialization |
| core/ast/src/nodes.rs | Updated ModuleDefinition.body type to RefCell<Option<Vec<Definition>>> |
| core/ast/src/lib.rs | Updated documentation to indicate ParserContext is no longer WIP |
| core/ast/src/builder.rs | Added location base tracking and made several methods public for parser context |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| fn infer_definitions(&mut self, definitions: &[Definition], ctx: &mut TypedContext, path: &[String]) { |
There was a problem hiding this comment.
The path parameter is declared but never used in this function or passed to nested calls. Either use it for module path tracking or remove it if not needed.
| .set_language(&tree_sitter_inference::language()) | ||
| .map_err(|_| anyhow::anyhow!("Error loading Inference grammar"))?; | ||
| let mut builder = Builder::new(); | ||
| let source_file_id = Builder::next_node_id(); |
There was a problem hiding this comment.
Using Builder::next_node_id() outside of the Builder context is confusing. Consider using self.next_node_id() for consistency with how IDs are generated elsewhere in this context.
| let source_file_id = Builder::next_node_id(); | |
| let source_file_id = builder.next_node_id(); |
| LocationBase { | ||
| offset: offset as u32, | ||
| line: line.saturating_sub(1), | ||
| column: column.saturating_sub(1), |
There was a problem hiding this comment.
The column is being decremented by 1, but on line 276 it's incremented by 1, and on line 1587 of builder.rs it's also incremented by 1. This could lead to off-by-one errors in column numbering. Verify the intended column indexing convention (0-based vs 1-based).
| if start_position.row == 0 { | ||
| start_column += self.location_base.column; | ||
| } | ||
| if end_position.row == 0 { | ||
| end_column += self.location_base.column; | ||
| } |
There was a problem hiding this comment.
The column offset is only applied when row == 0, but rows are being offset unconditionally. This asymmetry could cause incorrect locations for multi-line nodes in nested contexts. Consider whether column offset should apply to all rows or only the first line of the source.
| if start_position.row == 0 { | |
| start_column += self.location_base.column; | |
| } | |
| if end_position.row == 0 { | |
| end_column += self.location_base.column; | |
| } | |
| start_column += self.location_base.column; | |
| end_column += self.location_base.column; |
refcell_eq
Outdated
2 participants
resolves #66