Repository navigation
Interface fixes and comparison operators - #3788
Merged
Merged
Conversation
Along with implementations for primitives+builtin containers. This commit this does not do any operator overloading, they are simply interfaces.
Adds `type_implements_with_deps` in the TIR that allows us to search if a given type implements a given interface in dependencies, not just the current package.
This is what powers `==` and it works for *any* types, not just ones that implement `baml.ops.Equals`. It is basically `deep_equals` except it delegates to `baml.ops.Equals` whenever it reaches a value that implements it.
The previous one only looked up by `TypeName` so did not work correctly for blanket implementations.
Following Rust's RFC 2451 per BEP-044
The previous interface implementation did not check for overlapping implements blocks across files. We now do a full cross-package coherece check including blanket implementations to ensure there can never be more than one implementation of a realized interface for a realized concrete type.
While we don't really have user package dependencies, we should use the full dependency tree for resolving interfaces rather than just direct dependencies.
Since BAML has unions, checking for overlap between to `implements` blocks is much harder than in Rust. In fact, it's an [NP-hard problem](https://arxiv.org/abs/1611.05672) in the general case. We now have a far more comprehensive overlap checker which should allow a better analysis given BAML's union-equivalence rules. It will first try a number of common/trivial fast-path cases, as well as reducing the total search space. Then it will fall back to the general-case solver which will only be run if the search space is within an acceptable upper bound.
There will never be overlap between the interface-existentials `Foo<Item=string>` and `Foo<Item=int>` because each concrete type may only implement one. This change only increases permissiveness and does not affect soundness.
`==` is now syntax sugar for the function, though some primitives will emit builtin bytecode for a fast-path comparison. If the types of each operand are provably disjoint (will never be equal) then we emit a warning.
Adds the `VirtualCall` VM instruction which, given a `Self` receiver, an iterface, a method name, and method arguments, will dispatch a call to the correct concrete implementation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2kai2kai2
enabled auto-merge
June 17, 2026 18:36
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (20)
📒 Files selected for processing (21)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughAdds comparison primitives, virtual interface dispatch, runtime registry wiring, type inference changes, and interface coherence validation. ChangesComparison, dispatch, and coherence
Node.js protobuf encodeDelimited fix
Sequence Diagram(s)sequenceDiagram
participant BexVm
participant resolve_implements_rule
participant realize_frame
participant execute_call_from_locals_offset
BexVm->>resolve_implements_rule: concrete_ty, iface, iface_args
resolve_implements_rule-->>BexVm: RuntimeImplRule + bound_args
BexVm->>realize_frame: method.frame, bound_args
realize_frame-->>BexVm: realized type_args
BexVm->>execute_call_from_locals_offset: callee FQN, type_args
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
⏭️ 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):
|
|
No description provided. |
2kai2kai2
force-pushed
the
kai/comparison
branch
from
June 17, 2026 18:55
633304c to
be4fb65
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/bex_vm_types/src/bytecode.rs (1)
185-186:⚠️ Potential issue | 🟠 MajorPreserve the serialized
Instructiondiscriminants.
Instructionis Borsh-serialized without explicit discriminant pinning (unlikeOpCodewhich uses#[borsh(use_discriminant = true)]), and insertingVirtualCallbeforeThrowshifts the ordinal for every later variant. Existing packed bytecode (embedded in executables viabaml pack) will deserialize incorrectly: bytes that previously decoded toThrowwill now decode toVirtualCall, silently corrupting instruction interpretation. MoveVirtualCallto the end of the enum to match theOpCodepattern, or add an explicit version field toProgram/Bytecodewith migration logic.🤖 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_types/src/bytecode.rs` around lines 185 - 186, The Instruction enum lacks explicit discriminant pinning for Borsh serialization, so adding VirtualCall before Throw shifts the ordinal values of all subsequent variants, breaking deserialization of existing packed bytecode. Move the VirtualCall variant to the end of the Instruction enum definition to preserve the existing discriminant values for all other variants and maintain backward compatibility with bytecode already embedded in executables.
🧹 Nitpick comments (5)
baml_language/crates/baml_compiler2_mir/src/builder.rs (1)
350-353: ⚡ Quick winClarify the receiver position when method type args are present.
Line 351 says the receiver is the first
argsentry, but the API reservesargs[..ntypeargs]for method-level type arguments, so the receiver isargs[ntypeargs].Proposed documentation fix
- /// resolved at runtime from the receiver's concrete type (the first `args` - /// entry) against `iface`. Used for statically-undetermined receivers + /// resolved at runtime from the receiver's concrete type (the first value + /// argument, `args[ntypeargs]`) against `iface`. Used for statically-undetermined receivers🤖 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_compiler2_mir/src/builder.rs` around lines 350 - 353, The documentation comment for the open-world virtual interface-method call function needs to be updated to correctly describe the receiver position. Currently, the comment states the receiver is the first args entry, but this is only accurate when there are no method-level type arguments. Update the comment to clarify that the API reserves args[..ntypeargs] for method-level type arguments, and therefore the receiver is located at args[ntypeargs], not at args[0]. This will prevent confusion when method type arguments are present.baml_language/crates/bex_vm_types/src/bytecode.rs (1)
2069-2073: ⚡ Quick winAdd a unit test for
VirtualCallcompact encoding.This opcode has the new
u16 + u16operand shape; cover size, operand order,TryFrom, and display sonargs/ntypeargscannot be swapped silently. As per coding guidelines, Rust changes should prefer unit tests where possible and runcargo test --lib.Proposed unit test
+ #[test] + fn encode_virtual_call_operands() { + let instr = Instruction::VirtualCall { + nargs: 3, + ntypeargs: 2, + }; + let bc = make_bytecode(vec![instr], vec![]); + let compact = bc.lower_to_compact(); + + assert_eq!(compact.code.len(), 5); + assert_eq!(compact.code[0], OpCode::VirtualCall as u8); + assert_eq!(u16::from_le_bytes([compact.code[1], compact.code[2]]), 3); + assert_eq!(u16::from_le_bytes([compact.code[3], compact.code[4]]), 2); + assert_eq!(OpCode::try_from(compact.code[0]), Ok(OpCode::VirtualCall)); + assert_eq!( + instr.to_string(), + "VIRTUAL_CALL nargs=3 ntypeargs=2" + ); + }Also applies to: 2399-2399
🤖 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_types/src/bytecode.rs` around lines 2069 - 2073, Add a unit test for the VirtualCall instruction encoding to verify the compact bytecode representation. The test should create a VirtualCall instance with specific nargs and ntypeargs values, encode it through the existing bytecode serialization logic, and verify that the resulting bytes have the correct length (4 bytes for two u16 values), that operands are encoded in the correct order (nargs first, then ntypeargs as shown in the encoding block), and that TryFrom deserialization correctly reconstructs the original instruction without swapping the operand values. Include assertions validating the Display trait implementation to ensure the instruction is formatted correctly for debugging purposes.Source: Coding guidelines
baml_language/crates/baml_compiler2_emit/src/emit.rs (1)
2112-2112: 💤 Low valueConsider adding a debug assertion for arithmetic safety.
The subtraction
args.len() - ntypeargswill panic on underflow in debug builds (or wrap in release). While MIR should always be well-formed withargs.len() >= ntypeargs, a debug assertion documents the invariant and aids debugging if upstream MIR construction has a bug.🛡️ Proposed defensive assertion
+ debug_assert!( + args.len() >= *ntypeargs, + "VirtualCall: args.len() ({}) must be >= ntypeargs ({})", + args.len(), + ntypeargs + ); let nargs = args.len() - ntypeargs;🤖 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_compiler2_emit/src/emit.rs` at line 2112, The subtraction operation `args.len() - ntypeargs` lacks a debug assertion to verify the invariant that args.len() must be greater than or equal to ntypeargs. Add a debug assertion immediately before this subtraction to ensure this precondition holds, which will document the expected invariant and help catch any upstream MIR construction bugs during development.baml_language/crates/bex_vm/src/vm.rs (1)
5483-5487: 🏗️ Heavy liftAvoid global name scans in the
VirtualCallhot path.
find_function_by_namescans all globals for every virtual dispatch. Sincemethod.fqnis known when building the runtime impl registry, store a resolvedGlobalIndex/function pointer inMethodImpland jump directly here.🤖 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 5483 - 5487, The VirtualCall hot path is performing an expensive global name scan via find_function_by_name for every virtual dispatch. Instead, modify the MethodImpl struct to store a pre-resolved GlobalIndex or function pointer during runtime impl registry construction. Then replace the find_function_by_name call in the VirtualCall handling block with a direct access to this pre-resolved reference stored in the method field. This eliminates the repeated global scans and moves the lookup cost to initialization time.baml_language/crates/bex_vm/tests/interface_registry.rs (1)
26-29: ⚡ Quick win
contains-based impl-rule matching is too fuzzy for a white-box assertion helper.Substring matching can pick the wrong rule as more types are added, which can make these tests pass for the wrong reason.
Proposed fix
.find(|rule| { let pat = rule.for_ty_pattern.to_string(); - pat == for_type || (pat.contains(for_type) && !pat.contains("$stream")) + let normalized = pat.strip_suffix("$stream").unwrap_or(&pat); + normalized == for_type + || normalized.rsplit('.').next() == Some(for_type) })🤖 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/interface_registry.rs` around lines 26 - 29, The `find` closure that matches rules based on `rule.for_ty_pattern` uses substring matching with `contains`, which is too imprecise and can match the wrong rule as more types are added. Replace the fuzzy substring-based matching logic with exact pattern matching that properly compares the pattern against the intended type without relying on partial string containment checks. Consider implementing a more precise matching strategy that handles the pattern matching semantics correctly rather than just checking if one string contains another.
🤖 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.
Inline comments:
In `@baml_language/crates/baml_builtins2_codegen/src/codegen.rs`:
- Around line 1733-1753: The is_container closure only detects top-level List,
Map, and Uint8Array types but fails to detect containers wrapped in Optional
types like Option<List<T>>. This causes the function to undercount container
parameters and potentially miss deadlock scenarios. Update the is_container
closure to recursively check if a type is Optional and if so, inspect the
wrapped inner type to detect containers that are nested within Option wrappers,
ensuring all container parameters are properly counted regardless of whether
they are directly or indirectly wrapped in Optional.
In `@baml_language/crates/baml_builtins2_codegen/src/extract.rs`:
- Around line 512-517: The generic parameters collection in the code around
impl_def.generic_params is only capturing implementation-level generics but
missing method-level generics. When collecting impl_generics using
impl_def.generic_params, also iterate through and include any generic parameters
that exist at the method level to ensure all generics (both implementation and
method level) are properly propagated to NativeBuiltin.generics. Apply this same
fix to all similar locations where generics are being collected (including the
areas mentioned at lines 550-577) so that method generic parameters are not
treated as concrete types and are included in the final generics list.
- Around line 614-626: The function `type_expr_to_baml_type_with_self` only
handles bare `Self` and falls through to `type_expr_to_baml_type` for all other
cases, which leaves nested forms like `Self[]`, `map<string, Self>`, or `Self?`
unresolved. To fix this, instead of just checking for top-level `Self`, you need
to recursively process the entire TypeExpr structure to replace all occurrences
of `Self` with `self_baml` at any nesting level. For container types, extract
their inner types and recursively call `type_expr_to_baml_type_with_self` on
those inner types before reconstructing the container with the resolved inner
types. Apply the same recursive logic for optional or wrapped types before
applying outer type wrappers.
In `@baml_language/crates/baml_builtins2/baml_std/baml/ns_ops/comparison.baml`:
- Around line 147-156: In the Equals implementations for T[] and map<K, V>,
replace the calls to baml.ops.equals_equals(self, other) with direct comparison
logic. For the array implementation, compare elements sequentially using the
element type's Equals trait. For the map implementation, compare key-value pairs
directly. This avoids the circular dispatch where equals_equals would call back
to Equals.eq, preventing infinite recursion.
In `@baml_language/crates/baml_compiler2_emit/src/lib.rs`:
- Around line 352-357: The rules are currently being bucketed and sorted using
only the for_ty_pattern as the key, but when a class implements the same
interface at multiple instantiations (e.g., Converter<int> and
Converter<float>), this creates ties that rely on insertion order from unordered
maps, breaking deterministic output. Update the sort key to include the full
interface instantiation alongside for_ty_pattern, not just the pattern alone.
This applies to the rule sorting/keying logic referenced in the comment block
and also to any other places where similar bucketing occurs (around the
locations mentioned in the comment).
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 8923-8938: The iface_template is currently built using
TyTemplate::Concrete with RuntimeTy::Interface and convert_tir_ty_for_runtime
conversions, which preserves caller generics as bare RuntimeTy::TypeVar instead
of TypeArgRef, causing unresolved interface keys in virtual calls on generic
Converter types. Replace the TyTemplate::Concrete(RuntimeTy::Interface(...))
construction with a call to ty_to_template on the TIR interface view to ensure
caller frame type arguments are properly substituted at runtime.
- Around line 8941-8949: The virtual_call method is receiving the destination
directly, but it should normalize the destination to a local variable first,
similar to how regular calls are handled. Instead of passing dest.clone() as the
destination to the virtual_call invocation, create a temporary local variable as
the destination for the virtual_call, and then after the resume block completes,
add a copy operation from that temporary local to the actual dest projection if
dest is not already a local. This ensures virtual interface calls handle complex
destinations like fields, indices, or captures consistently with regular call
constraint handling.
- Around line 5861-5876: The issue is that `eq_dest` can be a complex Place like
`Place::Field` or `Place::Index` for both `Eq` and `Ne` operations, but the
`call` instruction expects a `Place::Local` destination. Currently, a temporary
is only created for `Ne` operations. Modify the logic to check whether `dest` is
a `Place::Local` regardless of the operator type: if it is not a local, create a
temporary as the call destination for both `Eq` and `Ne` cases, then add a copy
instruction after the call to transfer the result from the temporary back to the
original `dest` place.
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 4550-4560: The `expand_type_alias` call in the map operation on
line 4558 only expands top-level aliases and does not handle nested aliases
within compound types like Container<List<A>>. Modify the expansion logic to
recursively expand all nested type aliases so that compound types with aliased
type parameters are fully resolved. This ensures that semantically equivalent
types like Container<List<A>> and Container<List<int>> (where A = int) generate
identical keys when used in the lower_path_generic_args function and subsequent
type_arg_key generation.
- Around line 4453-4457: The loops iterating over members.required_methods are
only checking if methods are in provided_method_names before skipping them, but
they also need to check if a default implementation exists in
members.default_methods. Modify the condition where you check if
provided_method_names.contains(req_name) to also check if req_name exists in
members.default_methods, so that requirements are properly satisfied by either
explicit implementations or default implementations from parent types. Apply
this fix to all similar loops that validate required methods (note the comment
mentions this also applies to additional locations around line 4794-4801).
In `@baml_language/crates/baml_tests/tests/interfaces.rs`:
- Around line 7640-7648: The test malformed_impls_do_not_spuriously_overlap only
asserts that there are no interface errors, which could allow the test to pass
vacuously if unresolved-type validation regresses. Add additional assertions
after the assert_no_interface_errors call to verify that unresolved-type
diagnostics are actually present for the Nonexistent1 and Nonexistent2 types,
and ensure that error E0132 (the spurious overlap error) is absent from the
diagnostics. This will prevent the test from passing silently when type
validation logic changes.
In `@baml_language/crates/bex_vm_types/src/bytecode.rs`:
- Around line 958-963: The comment for the VirtualCall instruction incorrectly
describes the stack order. The interface type (Object::Type) and method-name
string are actually placed above the value args on the stack and are popped
first, not below as the current comment states. Update the VirtualCall comment
to accurately reflect that iface_type and method_name are positioned above the
args on the stack and are popped before processing the arguments.
In `@baml_language/crates/bex_vm_types/src/types.rs`:
- Around line 187-203: The new `interface_impls` field added to the `Program`
struct is a non-skipped Borsh-serialized field, which breaks deserialization of
previously compiled binaries. Determine which of the three approaches to take:
(1) add a version field to PackEnvelope and implement migration logic in the
extract_envelope() function to handle old wire formats, (2) document that old
compiled binaries are no longer supported, or (3) mark the interface_impls field
with #[borsh(skip)] if the field can be populated at runtime initialization
instead of deserialization. Verify your choice is compatible with how
extract_envelope() deserializes PackEnvelope and implement the necessary changes
to prevent deserialization failures.
In `@baml_language/crates/bex_vm/src/package_baml/ops.rs`:
- Around line 303-307: The cycle detection logic in the visited set
normalization is preventing both orderings of a comparison pair from being
checked separately. Instead of normalizing the key to a canonical form using the
comparison operator (if pa < pb), preserve the actual operand order directly as
the key. This ensures that both (pa, pb) and (pb, pa) are treated as distinct
entries in the visited set, allowing both orderings to be evaluated even if the
user-defined Equals.eq function is non-symmetric. Apply this same change to all
locations where similar key normalization occurs (also in the section mentioned
at lines 541-549).
In `@baml_language/crates/bex_vm/src/package_baml/resolve.rs`:
- Around line 116-145: The fallback mechanism in resolve_implements_rule is
incorrectly storing a mismatched rule as a fallback even when iface_args is
non-empty and the rule's interface_args do not match. The fallback is then
returned, allowing a mismatch like resolving Iface<string> through an Iface<int>
implementation. Only set the fallback to the first applicable rule when
iface_args.is_empty() is true; otherwise, if no rule's interface_args exactly
match the requested iface_args, the function should return None instead of
falling back to a mismatched rule.
In `@baml_language/crates/bex_vm/src/vm.rs`:
- Around line 5467-5477: The resolve_implements_rule call accepts a fallback
match even when iface_args is non-empty and the returned rule's interface
arguments don't match the requested iface_args, leading to incorrect impl
dispatch. After the resolve_implements_rule call succeeds and returns a rule and
bound_args, add validation to verify that the returned rule's realized interface
arguments match the provided iface_args. If they don't match, treat this as an
unresolved virtual call error instead of silently using the mismatched impl.
Alternatively, constrain the resolver to only use fallback candidates when
iface_args is empty or erased.
- Around line 1399-1414: The match arm in the virtual call resolution logic
handling Object::Array and Object::Map kinds currently uses unimplemented!()
which panics when container receivers are dispatched on, but the stdlib now
supports container interface methods. Replace the unimplemented!() call with
proper error handling by either implementing container virtual dispatch with
runtime type tags to track element types at the Value level, carrying the
realized receiver type information into the VirtualCall structure, or adding
validation during the lowering phase to reject unsupported container receiver
calls before they reach the VM. Choose the approach that aligns best with the
existing architecture.
In `@baml_language/crates/bex_vm/tests/comparison_driver.rs`:
- Around line 28-33: The infinite loop in the `run_bool` function can hang
indefinitely if the VM repeatedly returns `VmExecState::EarlyYield`, which
stalls the test suite instead of producing a clear failure. Add an iteration
counter to the loop and establish a maximum iteration limit (e.g., 10000
iterations). When the loop exceeds this limit, panic with a descriptive message
indicating that the VM execution did not complete within the expected number of
iterations, which will provide clear test failure feedback instead of hanging.
In `@baml_language/crates/bex_vm/tests/ops_comparison.rs`:
- Around line 85-90: The test currently verifies that ordering comparisons with
NaN on the left operand (nan, one) return false for FLOAT_LT, FLOAT_GT,
FLOAT_GE, and FLOAT_LE operations, but does not test NaN on the right operand.
Add additional assertions after the existing ones that mirror these tests but
with the operands reversed (one, nan) to ensure the float comparison operations
correctly handle NaN as the right-hand operand according to the IEEE standard.
---
Outside diff comments:
In `@baml_language/crates/bex_vm_types/src/bytecode.rs`:
- Around line 185-186: The Instruction enum lacks explicit discriminant pinning
for Borsh serialization, so adding VirtualCall before Throw shifts the ordinal
values of all subsequent variants, breaking deserialization of existing packed
bytecode. Move the VirtualCall variant to the end of the Instruction enum
definition to preserve the existing discriminant values for all other variants
and maintain backward compatibility with bytecode already embedded in
executables.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler2_emit/src/emit.rs`:
- Line 2112: The subtraction operation `args.len() - ntypeargs` lacks a debug
assertion to verify the invariant that args.len() must be greater than or equal
to ntypeargs. Add a debug assertion immediately before this subtraction to
ensure this precondition holds, which will document the expected invariant and
help catch any upstream MIR construction bugs during development.
In `@baml_language/crates/baml_compiler2_mir/src/builder.rs`:
- Around line 350-353: The documentation comment for the open-world virtual
interface-method call function needs to be updated to correctly describe the
receiver position. Currently, the comment states the receiver is the first args
entry, but this is only accurate when there are no method-level type arguments.
Update the comment to clarify that the API reserves args[..ntypeargs] for
method-level type arguments, and therefore the receiver is located at
args[ntypeargs], not at args[0]. This will prevent confusion when method type
arguments are present.
In `@baml_language/crates/bex_vm_types/src/bytecode.rs`:
- Around line 2069-2073: Add a unit test for the VirtualCall instruction
encoding to verify the compact bytecode representation. The test should create a
VirtualCall instance with specific nargs and ntypeargs values, encode it through
the existing bytecode serialization logic, and verify that the resulting bytes
have the correct length (4 bytes for two u16 values), that operands are encoded
in the correct order (nargs first, then ntypeargs as shown in the encoding
block), and that TryFrom deserialization correctly reconstructs the original
instruction without swapping the operand values. Include assertions validating
the Display trait implementation to ensure the instruction is formatted
correctly for debugging purposes.
In `@baml_language/crates/bex_vm/src/vm.rs`:
- Around line 5483-5487: The VirtualCall hot path is performing an expensive
global name scan via find_function_by_name for every virtual dispatch. Instead,
modify the MethodImpl struct to store a pre-resolved GlobalIndex or function
pointer during runtime impl registry construction. Then replace the
find_function_by_name call in the VirtualCall handling block with a direct
access to this pre-resolved reference stored in the method field. This
eliminates the repeated global scans and moves the lookup cost to initialization
time.
In `@baml_language/crates/bex_vm/tests/interface_registry.rs`:
- Around line 26-29: The `find` closure that matches rules based on
`rule.for_ty_pattern` uses substring matching with `contains`, which is too
imprecise and can match the wrong rule as more types are added. Replace the
fuzzy substring-based matching logic with exact pattern matching that properly
compares the pattern against the intended type without relying on partial string
containment checks. Consider implementing a more precise matching strategy that
handles the pattern matching semantics correctly rather than just checking if
one string contains another.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 91640b55-6760-4013-94cb-5a54db88fe94
⛔ Files ignored due to path filters (40)
baml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/bigints.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/csv.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/floats.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/inferred_generic_type_args.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/iter.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/iter_impl_generics_only.core.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/iter_impl_generics_only.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_to_from_string.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/optional_function_parameters.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/reflect_type_of.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/reflect_type_of_generic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/type_reflection.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/typed_inputs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiler2_mir/baml_tests__compiler2_mir__optional_named_gap_and_explicit_null.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__assert_std__/baml_tests__compiles____assert_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__assert_std__/baml_tests__compiles____assert_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/byte_string_literals/baml_tests__compiles__byte_string_literals__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/byte_string_literals/baml_tests__compiles__byte_string_literals__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/optional_function_parameters/baml_tests__compiles__optional_function_parameters__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/optional_function_parameters/baml_tests__compiles__optional_function_parameters__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/numeric_invariance/baml_tests__diagnostic_errors__numeric_invariance__01_lexer__numeric_invariance.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/numeric_invariance/baml_tests__diagnostic_errors__numeric_invariance__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/numeric_invariance/baml_tests__diagnostic_errors__numeric_invariance__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/numeric_invariance/baml_tests__diagnostic_errors__numeric_invariance__10_formatter__numeric_invariance.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/simple_type_error/baml_tests__diagnostic_errors__simple_type_error__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/simple_type_error/baml_tests__diagnostic_errors__simple_type_error__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_name_type_error/baml_tests__diagnostic_errors__test_expr_name_type_error__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_name_type_error/baml_tests__diagnostic_errors__test_expr_name_type_error__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_textual.snapis excluded by!**/*.snapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**
📒 Files selected for processing (46)
baml_language/crates/baml_builtins2/baml_std/baml/ns_ops/comparison.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_builtins2_codegen/src/codegen.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_emit/src/analysis.rsbaml_language/crates/baml_compiler2_emit/src/emit.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_emit/src/stack_carry.rsbaml_language/crates/baml_compiler2_hir/src/package.rsbaml_language/crates/baml_compiler2_mir/src/builder.rsbaml_language/crates/baml_compiler2_mir/src/ir.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_mir/src/optimize.rsbaml_language/crates/baml_compiler2_mir/src/pretty.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/infer_context.rsbaml_language/crates/baml_compiler2_tir/src/interfaces.rsbaml_language/crates/baml_compiler_diagnostics/src/diagnostic.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_tests/baml_src/ns_bigints/bigints.bamlbaml_language/crates/baml_tests/baml_src/ns_floats/floats.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/numeric_invariance/numeric_invariance.bamlbaml_language/crates/baml_tests/src/compiler2_tir/inference.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a.rsbaml_language/crates/baml_tests/tests/interfaces.rsbaml_language/crates/baml_tests/tests/interfaces_associated_types.rsbaml_language/crates/baml_type/src/interface.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/baml_type/src/normalize.rsbaml_language/crates/baml_type/src/normalize/tests.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_vm/src/debug.rsbaml_language/crates/bex_vm/src/package_baml/json.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/ops.rsbaml_language/crates/bex_vm/src/package_baml/resolve.rsbaml_language/crates/bex_vm/src/package_baml/type_class.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm/tests/comparison_driver.rsbaml_language/crates/bex_vm/tests/interface_registry.rsbaml_language/crates/bex_vm/tests/ops_comparison.rsbaml_language/crates/bex_vm_types/src/bytecode.rsbaml_language/crates/bex_vm_types/src/errors.rsbaml_language/crates/bex_vm_types/src/types.rsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js
- Path callees should also be able to do virtual calls - Class inherent method cannot override interface `implements` methods - Class inherent methods cannot satisfy an interface contract: ONLY methods inside the `implements` block (or defaulted in) can do so. - Methods from required interfaces of interface-existentials should dispatch with the declaring interface not the requiring interface. - Coderabbit suggestions - Better diagnostic messages for invalid operator usage - Add some regression tests
2kai2kai2
force-pushed
the
kai/comparison
branch
from
June 17, 2026 20:55
be4fb65 to
19161cf
Compare
codeshaunted
added a commit
that referenced
this pull request
Jun 18, 2026
…olation
Resolves 4 post-merge test failures (no source conflicts — all in generated
snapshots / test expectations):
- numeric_invariance lexer snapshot: regenerated — backtick now lexes as a
`Backtick` token (this branch's feature) instead of `Error`.
- bytecode_display_{expanded,unoptimized}: regenerated — canary grew the
builtin function table (comparison ops / interface fixes) so call indices
shifted, plus this branch's auto-derived `to_string` methods now appear.
- phase3a {new_mode_failures_have_good_diagnostics, nested_lambda_diagnostic_
has_real_span}: updated the asserted substring from `operator \`Add\`` to
`operator \`+\`` to match canary's improved arithmetic diagnostic (#3788),
which now prints the user-facing operator symbol.
Full suite green: 6263 passed, 0 snapshot drift.
This was referenced Jun 25, 2026
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
baml.ops.Equalsinterface +baml.ops.equals_equalsdriver for==baml.ops.Compareinterface directly corresponds to</>/<=/>=implementsrule registryimplementsrule (and thus concrete method implementation) at compile time, then we do a virtual method call. This is implemented as a new VM instruction, instead of the previous strategy which (unsoundly) did a switch over the receiver's type tag.Summary by CodeRabbit
New Features
Equals/Compareprimitives (==,!=, and ordering) with structural equality for arrays/maps and a runtime fallback for dynamic operands.Bug Fixes
floatNaN comparison behavior: all ordering and equality with NaN behaves as expected.