Major refactoring of Type / DIF Type structs #512
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a major refactoring of the Type and DIF Type structs, reorganizing the codebase structure and updating type representations across multiple modules. The changes affect core functionality including the AST structure, compiler components, DIF format, and various utilities.
Key Changes:
- Restructured type representation from
TypeContainerto separateTypeandTypeDefinitionstructures - Reorganized AST into separate
structsmodules for expressions, types, and apply operations - Refactored DIF (Datex Interface Format) to use new type representations
- Updated compiler components to work with restructured types
- Added no_std support infrastructure with custom stdlib module
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lsp/type_hint_collector.rs | New file for collecting type hints in LSP |
| src/lsp/mod.rs | New LSP module implementation with language server capabilities |
| src/lsp/errors.rs | Error handling for LSP compiler errors |
| src/logger.rs | Updated to use AtomicBool and added esp_logger support |
| src/libs/core.rs | Major refactor changing type storage and r#type field rename to ty |
| src/lib.rs | Added no_std support with stdlib/collections modules |
| src/global/* | Updated to use core prelude and removed std dependencies |
| src/fmt/* | New formatter module for DATEX code formatting |
| src/dif/* | Updated DIF format to use new type representations |
| src/decompiler/* | Updated decompiler to work with restructured AST |
| src/crypto/* | Updated to use stdlib abstractions |
| src/core_compiler/* | New core compiler module for value/type compilation |
| src/compiler/* | Updated compiler components for new type system |
| src/ast/structs/* | New AST structure with separate modules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type CoreLibTypes = HashMap<CoreLibPointerId, Type>; | ||
|
|
||
| #[cfg_attr(not(feature = "embassy_runtime"), thread_local)] | ||
| pub static mut CORE_LIB_TYPES: Option<CoreLibTypes> = None; |
There was a problem hiding this comment.
Using mutable static variables with unsafe access creates potential race conditions in multi-threaded contexts. Consider using OnceLock or another thread-safe initialization mechanism instead of raw mutable static.
src/dif/value.rs
Outdated
| match type_container { | ||
| ) -> Option<DIFTypeDefinition> { | ||
| match type_definition { | ||
| TypeContainer::TypeReference(inner) => { |
There was a problem hiding this comment.
The match arm references TypeContainer but the function parameter is type_definition: &TypeDefinition. This appears to be a mismatch - the code should match on TypeDefinition variants, not TypeContainer.
| TypeContainer::TypeReference(inner) => { | |
| TypeDefinition::TypeReference(inner) => { |
src/dif/representation.rs
Outdated
| ) -> Result<Value, DIFReferenceNotFoundError> { | ||
| Ok(match type_container { | ||
| Ok(match type_definition { | ||
| DIFTypeContainer::Reference(r) => { |
There was a problem hiding this comment.
The function parameter is type_definition: &DIFTypeDefinition but the match arms reference DIFTypeContainer. This appears to be incorrect - should be matching on DIFTypeDefinition variants instead.
| // // add pointer to memory if not there yet | ||
| // append_value(buffer, &reference.collapse_to_value().borrow()) | ||
| // } | ||
| _ => unreachable!() |
There was a problem hiding this comment.
Using unreachable!() without explanation in a match arm makes it unclear what cases should never occur. Consider using a more descriptive panic message or documenting why this is unreachable.
| _ => unreachable!() | |
| _ => unreachable!("append_type_container: encountered unexpected Type variant. Only variants handled above are expected here. If you see this, Type may have changed or this function is being misused.") |
Test Results590 tests 590 ✅ 34s ⏱️ Results for commit 0f2bc13. |
No description provided.