v0.4.1: Type Coercion Controls, Recursive Type Aliases, Generic Structs#20
v0.4.1: Type Coercion Controls, Recursive Type Aliases, Generic Structs#20joshcramer merged 10 commits intomainfrom
Conversation
…(DD-009 Phase 4) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
….2/7.4) - Recursive type aliases: insert Named placeholder before resolving so self-references (e.g. JsonValue -> ... | [JsonValue]) resolve to Type::Named instead of Any; same placeholder added in interpreter.rs - Generic structs: store type_params per struct in TypeContext and FileExports; StructLiteral infer builds type param bindings from field values and returns Type::Generic with resolved args; FieldAccess infer substitutes type params when accessing fields of a Generic-typed struct - Module imports: struct_type_params exported and imported alongside struct fields so cross-file generic structs work - 6 new tests covering recursive alias type-check, no-infinite-loop, generic struct declaration, construction, field inference, and mismatch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ride for tests std::env::set_var is unsafe in multi-threaded contexts (Rust 1.83+, CI uses 1.94). This caused SIGABRT on macOS CI. Added set_test_type_mode() with RAII guard that uses a thread-local override instead of mutating process env.
… macOS tests - get_type_mode() in test builds no longer falls back to std::env::var() - macOS CI runs tests with --test-threads=1 to prevent env var race conditions - Targets SIGABRT on macOS CI (Rust 1.94, std::env is not thread-safe)
…t for macOS CI macOS aarch64 has 512KB default thread stack vs 8MB on Linux. The recursive interpreter tests cause real stack overflow (SIGABRT) despite the language-level recursion limit, because each interpreter call frame is many Rust stack frames. Also: suppress dead_code warnings for env-reading functions in test builds, disable fail-fast so all platforms report results independently.
There was a problem hiding this comment.
Pull request overview
Adds runtime TypeMode gating for implicit type coercions (arithmetic, string concat, boolean conditions/operators), building on the existing TypeMode/LintMode infrastructure, and updates version/docs/CI accordingly.
Changes:
- Enforce TypeMode-controlled behavior for mixed Int↔Float arithmetic, non-string concatenation, and non-bool conditions / logical ops in the interpreter.
- Extend the typechecker with recursive type-alias handling (placeholder) and generic-struct type parameter tracking + field inference.
- Improve test stability (thread-local TypeMode override; CI stack sizing; mutex poisoning handling) and bump docs/package version to v0.4.1.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/interpreter.rs | Adds strict/warn/forgiving gates for implicit coercions + related tests and test-mode configuration changes. |
| src/typechecker.rs | Tracks generic struct type params, substitutes them on field access, and adds recursive type-alias placeholder handling + tests. |
| src/config.rs | Introduces thread-local TypeMode override for tests and removes env-based reads in cfg(test) paths. |
| .github/workflows/ci.yml | Disables fail-fast and increases minimum stack size for recursion-heavy tests. |
| Cargo.toml / Cargo.lock | Version bump to 0.4.1. |
| docs/*.md | Updates “Last updated” metadata to v0.4.1. |
| CLAUDE.md / .github/copilot-instructions.md | Sync date update. |
src/typechecker.rs
Outdated
| // If expected type is a generic type param, record the binding | ||
| if let Type::Named(tp_name) = expected { | ||
| if tp_names.contains(tp_name) { | ||
| bindings | ||
| .entry(tp_name.clone()) | ||
| .or_insert_with(|| actual.clone()); | ||
| // Skip compatibility check — type param accepts any type |
There was a problem hiding this comment.
Generic struct field binding in struct literals doesn't enforce consistency when the same type parameter appears in multiple fields. Because the binding uses or_insert_with(...) and then continue, a conflicting second field (e.g., struct Pair<A> { a: A, b: A } with { a: 1, b: "x" }) would not produce a type error. Consider checking an existing binding for compatibility (and erroring on mismatch) rather than skipping validation unconditionally once the type param is recognized.
| // If expected type is a generic type param, record the binding | |
| if let Type::Named(tp_name) = expected { | |
| if tp_names.contains(tp_name) { | |
| bindings | |
| .entry(tp_name.clone()) | |
| .or_insert_with(|| actual.clone()); | |
| // Skip compatibility check — type param accepts any type | |
| // If expected type is a generic type param, record/check the binding | |
| if let Type::Named(tp_name) = expected { | |
| if tp_names.contains(tp_name) { | |
| if let Some(bound) = bindings.get(tp_name) { | |
| // A binding for this type parameter already exists; ensure it is compatible | |
| if !self.compatible(&actual, bound) | |
| && !matches!(actual, Type::Any) | |
| && !matches!(bound, Type::Any) | |
| { | |
| let line = self.find_line_near(name); | |
| self.error( | |
| format!( | |
| "In struct '{}', generic type parameter '{}' has incompatible bindings: {} and {}", | |
| name, | |
| tp_name, | |
| bound.name(), | |
| actual.name() | |
| ), | |
| line, | |
| None, | |
| ); | |
| } | |
| } else { | |
| bindings.insert(tp_name.clone(), actual.clone()); | |
| } | |
| // Skip further field-vs-struct compatibility check: handled via type param binding |
There was a problem hiding this comment.
Fixed in bd0742c. Now checks existing bindings for compatibility — Pair<A> { a: 1, b: "x" } correctly errors with "generic type parameter 'A' has incompatible bindings: Int and String".
| // TypeMode gate (DD-009 Phase 4): Strict requires Bool operands for && and ||. | ||
| match operator { | ||
| BinaryOp::And => { | ||
| if !matches!(lhs, Value::Bool(_)) { | ||
| match get_type_mode() { | ||
| TypeMode::Strict => { | ||
| return Err(IntentError::type_error(format!( | ||
| "Non-boolean condition in if/while. Got {}. Use explicit comparison (e.g., value != None, len(arr) > 0).", | ||
| lhs.type_name() | ||
| ))); |
There was a problem hiding this comment.
The strict-mode error/warn message used for &&/|| operands says "Non-boolean condition in if/while", which is misleading in this context (these are logical operators, not control-flow conditions). Consider tailoring the message to mention the operator (e.g., "Non-boolean operand for &&" / "||") and the expected type (Bool) to make runtime errors actionable.
There was a problem hiding this comment.
Fixed in bd0742c. Messages now say "Non-boolean operand for &&" / "Non-boolean operand for ||" instead of the misleading "if/while" text.
| // TypeMode gate (DD-009 Phase 4): Strict requires Bool operand for !. | ||
| if !matches!(val, Value::Bool(_)) { | ||
| match get_type_mode() { | ||
| TypeMode::Strict => { | ||
| return Err(IntentError::type_error(format!( | ||
| "Non-boolean condition in if/while. Got {}. Use explicit comparison (e.g., value != None, len(arr) > 0).", | ||
| val.type_name() | ||
| ))); |
There was a problem hiding this comment.
The strict-mode error/warn message for unary ! currently says "Non-boolean condition in if/while", which doesn't match the operator being evaluated. Consider changing this to a !-specific message (e.g., "Non-boolean operand for !") so users can quickly find and fix the source expression.
There was a problem hiding this comment.
Fixed in bd0742c. Now says "Non-boolean operand for !".
| #[cfg(test)] | ||
| pub fn get_type_mode() -> TypeMode { | ||
| read_type_mode_from_env() | ||
| // In tests, use thread-local override only (no env var reads). | ||
| // std::env::var is unsafe to call concurrently with set_var on macOS | ||
| // (Rust 1.83+ / POSIX getenv is not thread-safe on all platforms). | ||
| TYPE_MODE_OVERRIDE.with(|cell| (*cell.borrow()).unwrap_or(TypeMode::Warn)) |
There was a problem hiding this comment.
get_type_mode() in test builds now uses a thread-local override and does not read NTNT_TYPE_MODE from the environment, but the surrounding documentation (above this function) still describes per-call env reads for tests. Please update the docs/comments to match the new thread-local override behavior so tests don’t attempt to rely on std::env::set_var("NTNT_TYPE_MODE", ...).
There was a problem hiding this comment.
Fixed in bd0742c. Doc comment now describes the thread-local override behavior and points to set_test_type_mode.
src/interpreter.rs
Outdated
| // Insert placeholder first to support recursive type aliases | ||
| // (self-references during resolution will find the placeholder) | ||
| self.type_aliases | ||
| .insert(name.clone(), TypeExpr::Named(name.clone())); |
There was a problem hiding this comment.
In Statement::TypeAlias, a placeholder is inserted and then immediately overwritten with the real target, so the placeholder never survives in self.type_aliases. Since the interpreter currently doesn’t resolve type aliases anywhere else, this is effectively a no-op and the comment about recursive resolution is misleading. Consider removing the placeholder insertion (or implementing actual resolution where the placeholder is needed) to keep the runtime code clear.
| // Insert placeholder first to support recursive type aliases | |
| // (self-references during resolution will find the placeholder) | |
| self.type_aliases | |
| .insert(name.clone(), TypeExpr::Named(name.clone())); |
There was a problem hiding this comment.
Fixed in bd0742c. Removed the no-op placeholder — only the typechecker needs it (where resolve_type_expr actually resolves the self-reference). The interpreter just stores the raw TypeExpr.
| #[cfg(test)] | ||
| pub fn get_lint_mode() -> LintMode { | ||
| read_lint_mode_from_env() | ||
| // In tests, always return Default (no env var reads for thread safety). | ||
| LintMode::Default |
There was a problem hiding this comment.
get_lint_mode() in test builds now always returns LintMode::Default, which diverges from the documented behavior elsewhere that test builds re-read NTNT_LINT_MODE each call. If this change is intentional for thread-safety, consider updating the surrounding docs/comments to avoid confusion, or adding a thread-local override similar to TypeMode so lint-mode behavior can still be tested deterministically.
There was a problem hiding this comment.
Fixed in bd0742c. Doc comment now explains that test builds return LintMode::Default for thread safety, and suggests adding a thread-local override (like TypeMode) if lint-mode testing is needed in the future.
1. Generic struct type param consistency: detect conflicting bindings
(e.g., Pair<A> { a: 1, b: "x" } now errors)
2. Error messages for &&/|| say 'Non-boolean operand for &&/||' not 'if/while'
3. Error message for ! says 'Non-boolean operand for !' not 'if/while'
4. Updated config.rs docs for get_type_mode() to describe thread-local override
5. Removed no-op placeholder in interpreter TypeAlias (only typechecker needs it)
6. Updated config.rs docs for get_lint_mode() to note test-build behavior
What's in this release
Type Coercion Controls
Strict mode now catches implicit type coercions that can mask bugs:
3 + 2.5requiresfloat(3) + 2.5in strict mode. Prevents silent precision loss. Mixed numeric comparisons (3 == 3.0) are unaffected."count: " + 42requiresstr(42)in strict mode. String + String always works.if count {}requires explicit bool in strict mode (if count > 0 {}).&&,||,!require Bool operands in strict mode with operator-specific error messages.All three TypeMode tiers work as expected: strict (error), warn (log + continue), forgiving (silent).
Recursive Type Aliases
Self-referencing type aliases now resolve correctly:
Uses a placeholder-first approach in the type checker so self-references find the alias during resolution.
Generic Struct Support
Generic structs now have full type checking:
Pair<A> { a: 1, b: "x" }errors)CI & Test Infrastructure
std::env::set_varwith thread-local override for test isolation.set_varis unsafe in multi-threaded contexts (Rust 1.83+) and caused SIGABRT on macOS CI.RUST_MIN_STACK=8MB— macOS aarch64 has 512KB default thread stacks; recursive interpreter tests need more.fail-fast: false— all 3 platforms report results independently.Stats
src/interpreter.rs,src/typechecker.rs,src/config.rs