Conversation
semantic: replace string-based type checks with token-driven metadata and decl handling
- checker.go:
* Replaced raw string comparisons (e.g. objType == "string") with token-based
type info, fixing issues with variants like `string?` and aliases.
* Added a small type-info cache to avoid reparsing identical strings; legacy
is*Name checks and metadata inference reuse the cached info.
* Numeric type checks now operate on parsed token types instead of strings.
- tokens/Token.go:
* Metadata lookup now normalizes case.
* Introduced MetadataTokenFromIdentifier so consumers can operate on tokens
end-to-end instead of fragile string matches.
- metadata.go:
* Centralized metadata typing via MetadataReturn + metadataReturnFor so both
checker and runtime share one source of truth.
* Consolidated return-type bookkeeping, removing duplicate blocks.
- interpreter.go:
* Forwarded member tokens through runtime metadata access.
* Added an environment-frame pool so function/method calls reuse cleared maps
instead of allocating new ones each recursion; reduces GC churn.
- ast/decls.go:
* FuncDecl, ClassDecl, StructDecl, EnumDecl, and MethodDecl now retain their
name tokens, allowing parser and checker to compare tokens directly.
* Builtin symbols now populate the new decl-name field.
- parser/headers.go:
* Constructors and methods captured via identifierMatches and name tokens
instead of raw text equality, avoiding fragile identifier string matching.
- builtins.go:
* Added a single factory helper to generate builtin ast.FuncDecls, removing
repeated boilerplate and simplifying future builtin additions.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the semantic analysis system to replace string-based type checking with a token-driven approach and introduces performance optimizations including environment pooling. The changes improve type safety by eliminating fragile string comparisons and reduce garbage collection pressure through object reuse.
- Replaced string-based type checking with token-driven metadata and type information handling
- Added environment pooling for function/method calls to reduce allocations
- Consolidated metadata return type handling and simplified builtin function definitions
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/third/Fib.co | Updated test parameters and return type for fibonacci function |
| src/semantic/metadata.go | Consolidated metadata return type handling with helper functions |
| src/semantic/interpreter.go | Added environment pooling and updated function call handling |
| src/semantic/checker.go | Introduced type info caching and replaced string-based type checks |
| src/semantic/builtins.go | Simplified builtin function definitions with factory helper |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pub func fib(n: i32): i32 { | ||
| pub func fib(n: i32): u64 { | ||
| if (n <= 1) { | ||
| ret n |
There was a problem hiding this comment.
The function returns u64 but returns parameter n which is of type i32. This creates a type mismatch that will cause compilation errors.
| ret n | |
| ret n as u64 |
| pub func fib(n: i32): i32 { | ||
| pub func fib(n: i32): u64 { | ||
| if (n <= 1) { | ||
| ret n |
There was a problem hiding this comment.
The function returns u64 but returns parameter n which is of type i32. This creates a type mismatch that will cause compilation errors.
| ret n | |
| ret n as u64 |
| func acquireEnv(size int) map[string]any { | ||
| if v := envPool.Get(); v != nil { | ||
| m := v.(map[string]any) | ||
| clearEnv(m) | ||
| return m | ||
| } | ||
| return make(map[string]any, size) | ||
| } |
There was a problem hiding this comment.
The function ignores the size parameter when creating a new map. It should use size for the capacity hint: return make(map[string]any, size).
| } | ||
|
|
||
| func releaseEnv(m map[string]any) { | ||
| clearEnv(m) |
There was a problem hiding this comment.
The clearEnv call is redundant since acquireEnv already calls clearEnv after retrieving from the pool. Consider removing this call to avoid double-clearing.
| clearEnv(m) |
| inst := make(map[string]any, 8) | ||
| inst["__type"] = typeName |
There was a problem hiding this comment.
The magic number 8 should be defined as a named constant like defaultInstanceCapacity to improve code readability and maintainability.
semantic: replace string-based type checks with token-driven metadata and decl handling
checker.go:
string?and aliases.tokens/Token.go:
metadata.go:
interpreter.go:
ast/decls.go:
parser/headers.go:
builtins.go: