runtime: mint type identity — TypeValue/MintId + unified eq (BEP-066 s1, PR 4) - #4331
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds ChangesMinted Type Identity
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
baml_language/crates/bex_vm/src/vm.rs (1)
756-765: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider bounding
static_mint_cachefor long-lived VMs.
static_mint_cachehas no eviction policy.LoadTyperuns on every generic call, and each distinct spelledRealizedTygets its own cache entry for the life of theBexVm. A long-lived VM (reused across many calls, or exposed to dynamically compiled/grafted packages with many concrete type instantiations) can grow this cache without bound.Cache entries are pure memoization (a stale/evicted entry only costs a re-walk, never a correctness issue), so a size cap or periodic clear is safe to add.
Also applies to: 1300-1312
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/bex_vm/src/vm.rs` around lines 756 - 765, Bound the `static_mint_cache` used by `BexVm` so repeated `LoadType` calls cannot grow it without limit. Add a fixed capacity or periodic eviction/clear policy at the cache insertion path near the `LoadType` handling around the referenced symbols, while preserving cache hits and recomputing evicted digests through the existing canonicalization flow.baml_language/crates/baml_tests/baml_src/ns_type_reflection/type_reflection.baml (1)
111-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the stale
syntactictest names.These tests now assert canonical mint equality, not syntactic inequality. Rename
union_order_deep_equals_syntactic_in_functionandunion_order_deep_equals_syntactic_in_test_blockso test names match the behavior.Also applies to: 119-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_tests/baml_src/ns_type_reflection/type_reflection.baml` around lines 111 - 113, Rename the stale test functions union_order_deep_equals_syntactic_in_function and union_order_deep_equals_syntactic_in_test_block to remove “syntactic” and reflect canonical mint equality, while preserving their assertions and behavior.baml_language/crates/bex_vm/tests/load_type.rs (1)
139-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the static mint value, not only its variant.
The test description says
LoadType(int)produces a deterministic static mint.matches!(type_value.mint(), MintId::Static(_))only checks the variant. Capture a secondLoadType(int)result and assert that bothMintIdvalues are equal, preferably across two VM instances.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/bex_vm/tests/load_type.rs` around lines 139 - 146, Strengthen the LoadType(int) test around the Object::Type branch by executing the same load in a second VM instance, capturing both results from type_value.mint(), and asserting the complete MintId values are equal. Retain the existing RealizedTy::int() assertion while replacing the variant-only MintId::Static(_) check with a deterministic-value comparison.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@baml_language/crates/baml_tests/baml_src/ns_type_reflection/type_reflection.baml`:
- Around line 111-113: Rename the stale test functions
union_order_deep_equals_syntactic_in_function and
union_order_deep_equals_syntactic_in_test_block to remove “syntactic” and
reflect canonical mint equality, while preserving their assertions and behavior.
In `@baml_language/crates/bex_vm/src/vm.rs`:
- Around line 756-765: Bound the `static_mint_cache` used by `BexVm` so repeated
`LoadType` calls cannot grow it without limit. Add a fixed capacity or periodic
eviction/clear policy at the cache insertion path near the `LoadType` handling
around the referenced symbols, while preserving cache hits and recomputing
evicted digests through the existing canonicalization flow.
In `@baml_language/crates/bex_vm/tests/load_type.rs`:
- Around line 139-146: Strengthen the LoadType(int) test around the Object::Type
branch by executing the same load in a second VM instance, capturing both
results from type_value.mint(), and asserting the complete MintId values are
equal. Retain the existing RealizedTy::int() assertion while replacing the
variant-only MintId::Static(_) check with a deterministic-value comparison.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a8541de1-8d02-407b-be2b-717621b73c1a
📒 Files selected for processing (19)
baml_language/crates/baml_tests/baml_src/ns_type_reflection/type_reflection.bamlbaml_language/crates/baml_tests/tests/type_value_equality.rsbaml_language/crates/baml_type/src/normalize.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_heap/src/accessor.rsbaml_language/crates/bex_heap/src/gc.rsbaml_language/crates/bex_heap/src/heap.rsbaml_language/crates/bex_heap/src/tlab.rsbaml_language/crates/bex_vm/src/package_baml/ops.rsbaml_language/crates/bex_vm/src/package_baml/reflect.rsbaml_language/crates/bex_vm/src/package_baml/root.rsbaml_language/crates/bex_vm/src/package_baml/type_class.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm/tests/load_type.rsbaml_language/crates/bex_vm/tests/method_class_type_args.rsbaml_language/crates/bex_vm_types/src/types.rsbaml_language/crates/bex_vm_types/src/types/object.rsbaml_language/crates/bex_vm_types/src/types/type_value.rs
Binary size checks passed✅ 7 passed
Generated by |
…e) (#4334) **BEP-066 slice-1 stack, PR 5 of 5 — the capstone.** Chained on #4331. With this green, the slice-1 stack is complete: the reflection read API (K/V/N rule families) is fully live. ## What - **Grammar carve-out**: `class`/`enum`/`interface`/`function` legal as path segments after `.` across type parsing, expression paths, patterns, map entries, generic lookahead, AST lowering, and formatting — bare keywords still rejected. - **The nine sealed kind classes** (`reflect.class.Type` … `reflect.function.Type`) + the closed `baml.reflect.TypeKind` union alias; identity-preserving `kind()` (K-5), all nine nullable `as_*()` (K-6 — never throw), `as_type()`. - **The one type-system fact**: kind class `<: type` sealed edge in shared normalization; `Object::Type` reports its precise kind class while keeping the physical TYPE tag; `implement … for type` preserved by teaching impl resolution to follow the sealed edge (`to_string()` verified on all nine kinds). - **Read-back (C-15)**: fields/values/member_types/element_type/params natives for all nine kinds; generic class fields substituted before read-back; function.Type has ordered params + return, no throws (K-11). New `docstring` + string-valued `other` columns on Class/Field/Enum/Variant; emit preserves aliases, descriptions, docstrings, and custom annotations (found + fixed + pinned a pre-existing hoist bug where the custom-attribute path consumed `stream.*`). - Kind classes non-constructible with a dedicated diagnostic. ## Oracle coverage Exhaustive nine-arm `TypeKind` match + missing-arm diagnostics · every `as_*` positive/null path · mint identity through `kind().as_type()` (I-2 × K-5) · non-throwing accessor contracts · full metadata read-back · conformance queries · recursive type walking · `of_value` precision. ## Gates Full corpus + parser + LSP (reviewed) + all-features baml_cli + tir + project green; fmt/clippy/rustdoc clean; snapshot accepts grouped in the commit (new type_kinds corpus + describe-listing growth). Implemented by a Codex (gpt-5.6-sol) worker under stack-manager review. **Both BEP-066 foundation stacks are now complete at 5/5.** <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added comprehensive runtime reflection for classes, enums, unions, literals, arrays, maps, interfaces, primitives, and functions. * Type values now expose their kind and provide kind-specific views and nested type information. * Added access to fields, parameters, enum values, metadata, aliases, documentation, and custom annotations. * Added validation preventing direct construction of reflection-kind values. * Expanded support for keyword-based names in qualified paths. * **Bug Fixes** * Improved type alias handling, metadata propagation, and reflection type matching. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 participant
BEP-066 slice-1 stack, PR 4 of 5 — chained on #4330. Implements the ratified identity design (I-1/I-2/I-4/I-9):
typevalues carry a mint token, and all three equality routes compare mints.Design
Object::Type→Box<TypeValue { ty: RealizedTy, mint: MintId }>(64-byte Object assert intact; no anchors/defs — slice 2).MintId::Static(u64): FNV-1a-64 (fixed offset/prime, big-endian numeric tokens) overNormalTy::canonicalwith the VM's program facts as TypeContext + a per-VM spelling-keyed digest cache — sotype.of<string?>() == type.of<string | null>()and every reference to a static declaration shares one mint (I-2), process-independently.MintId::Runtime(u64): shared AtomicU64 onBexHeap(spawned VMs covered). No producer yet — the constructors land in slice 2; distinctness pinned at the Rust level.BexVm::alloc_static_typeis the single constructor — no site can forget the mint. Wire/FFI strip mints outbound, re-derive inbound (H-4: identity never crosses);Programborsh unchanged.The three eq routes, unified atomically
CmpOp::{Eq,NotEq}(vm.rs) ·EqualsDriver(ops.rs) ·baml.deep_equals(root.rs) — all mint-only, plusTypeValue::{PartialEq,Eq,Hash}. Observable consequence, pinned by the flipped PR-1 characterization tests: permuted unions stay equal under==anddeep_equalsflips false→true (was syntactic). GC/deep-copy preserve the full TypeValue.Gates
Full baml_tests (1,486 lib + all binaries + 2,633 CLI cases) · bex_vm 21 + load_type 5 · bex_heap · bex_engine 109 · bex_vm_types 42 · project 82 — all green. Touched-crate clippy
-D warnings, fmt, rustdoc-D warningsclean. Zero snapshot churn (mint is invisible to compilation, as designed).Implemented by a Codex (gpt-5.6-sol) worker under stack-manager review.
Summary by CodeRabbit
New Features
typevalues.Bug Fixes
Tests