-
Notifications
You must be signed in to change notification settings - Fork 1
Remove source_code from node location
#70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…pdate constructor
…lated functionality
…hMap and add comprehensive tests for parent lookup functionality
…rce_file_for_node` and `get_node_source`
…and add test for Location's Clone trait
…imizations and key features
- Updated CONTRIBUTING.md for clarity and consistency in test writing guidelines. - Enhanced error messages in Arena's add_node method for better debugging. - Added comprehensive tests for the Arena structure, including edge cases and type definitions. - Expanded tests for the Builder functionality, ensuring proper AST construction from source code. - Improved coverage for various node types, including structs, functions, and external function definitions.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the source_code field from the Location struct to optimize memory usage and improve performance. The source text is now stored once in SourceFile.source instead of being duplicated in every node's location.
Changes:
- Removed
source: Stringfield fromLocationstruct and made itCopy - Added
source: Stringfield toSourceFileto store the complete source text - Replaced
Vec<NodeRoute>withFxHashMapfor O(1) parent/child lookups in the Arena - Added convenience methods
get_node_source()andfind_source_file_for_node()to the Arena API
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| core/ast/src/nodes.rs | Removed source field from Location, added Copy derive, and added source field to SourceFile |
| core/ast/src/builder.rs | Updated to store source in SourceFile instead of each Location |
| core/ast/src/arena.rs | Replaced linear parent lookup with hash map, added source text retrieval methods |
| core/ast/src/nodes_impl.rs | Updated SourceFile::new() to accept source parameter |
| core/type-checker/src/type_checker.rs | Removed .clone() calls on Location (now uses Copy) |
| core/type-checker/src/errors.rs | Removed source field from test location construction |
| tests/src/ast/nodes.rs | Updated tests to remove source parameter, added comprehensive Copy tests |
| tests/src/ast/builder.rs | Added extensive tests for source retrieval and location offsets |
| tests/src/ast/arena.rs | New file with comprehensive tests for Arena parent/child lookup and source retrieval |
| tests/src/type_checker/type_info_tests.rs | Updated dummy_location() to remove source parameter |
| CONTRIBUTING.md | Minor grammar and formatting corrections |
| CHANGELOG.md | New file documenting all changes in the project |
| core/ast/README.md | New comprehensive documentation for the AST crate |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #69