[codex] implement iterable for loops#3705
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Array adapter methods and Iterator.for_each; generalizes Chain error typing; implements Iterable-based for-loop type inference and MIR lowering; refactors interface dispatch/blanket-impl resolution; refines runtime template matching; and adds iterator tests. ChangesIterable-based Iterator Methods and For Loop Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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):
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
aaronvg
marked this pull request as ready for review
June 6, 2026 04:36
|
No description provided. |
![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: 3
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/baml_compiler2_mir/src/lower.rs (1)
6003-6018:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRun the formatter on this file before merging.
CI's pre-commit step is already rewriting these hunks, so this file will keep the pipeline red until the generated formatting diff is committed.
Also applies to: 9054-9084, 9146-9158, 9246-9285
🤖 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/lower.rs` around lines 6003 - 6018, Formatting is inconsistent in the receiver_class_type_arg_operands block (and other hunks listed) causing CI pre-commit to rewrite the file; run the Rust formatter (rustfmt) or your repository's pre-commit formatter on baml_language/crates/baml_compiler2_mir/src/lower.rs and commit the resulting changes so the code around receiver_class_type_arg_operands, enclosing_generic_params, ty_to_template, builder.temp, Place::local and Rvalue::LoadType matches the project's style; ensure the same formatting is applied to the other affected ranges (around lines mentioned in the review) before pushing.Source: Pipeline failures
🧹 Nitpick comments (2)
baml_language/crates/bex_vm/src/vm.rs (1)
47-50: ⚡ Quick winAdd unit coverage for the new subtype and union guard semantics.
These branches now define
IsTypebehavior for templated runtime checks, but the file-local#[cfg(test)]module still doesn't exercise the two new edge cases: accepting subtype matches throughTypeArgRefOrWildcard, and rejecting actual unions with an uncovered member. Please add focused unit tests here and runcargo test --liblocally before merge.As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible" and "Always run
cargo test --libif you changed any Rust code".Also applies to: 79-88
🤖 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 47 - 50, Add two focused unit tests inside the file-local #[cfg(test)] module in vm.rs to cover the new templated runtime check branches: (1) a test that constructs a frame_type_args entry and uses TyTemplate::TypeArgRefOrWildcard(n) to verify an actual type that is a subtype of the referenced expected type is accepted (exercise the actual.is_subtype_of(expected) path), and (2) a test that builds an actual union type containing a member not covered by the union guard to verify it is rejected (exercise the union guard rejection path referenced around the same code and lines 79-88). Locate the checks via TyTemplate::TypeArgRefOrWildcard, frame_type_args, and Ty::BuiltinUnknown, assert the expected boolean result from the IsType/runtime check, and run cargo test --lib to verify both tests pass.Source: Coding guidelines
baml_language/crates/baml_tests/baml_src/ns_iter/iter.baml (1)
59-62: ⚡ Quick winAdd one failure-path test for non-iterable
for-ininputs.These tests cover iterable success paths well, but they currently miss the contract where
for (let x in value)should panic ifvalueis not iterable. Adding one negative test here would harden regressions in lowering/type-dispatch changes.Suggested test addition
+test "iter_for_in_non_iterable_panics" { + // Adjust to the project's panic/assertion primitive for runtime failures. + assert.throws(() -> void { + let out: int[] = []; + for (let x in 123) { + out.push(x); + } + }) +}🤖 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_iter/iter.baml` around lines 59 - 62, Add a negative test alongside iter_for_in_range that verifies a for-in loop panics on non-iterable input: create a new test named like "iter_for_in_non_iterable" in the same file (baml_language/crates/baml_tests/baml_src/ns_iter/iter.baml) which calls the same lowering/iteration routine (mirror how iter_for_in_range() is used) but passes a non-iterable value (e.g., a number or null) and asserts that the call throws/panics; ensure the assertion uses the project test helper for expecting failures (parallel to how successful cases use baml.deep_equals) so regressions in the for (let x in value) non-iterable path are caught.
🤖 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_compiler2_mir/src/lower.rs`:
- Around line 8941-8945: The early-return that treats any requested_ty with
contains_assoc_projection(...) or Tir2Ty::TypeVar as unconstrained should be
removed; instead, in the logic around the match that currently returns true,
perform a structural walk/match of requested_ty (including nested nodes like
Map<Self.Item, int>) and attempt to match/substitute associated projections at
their concrete positions rather than short-circuiting the whole type; update the
handling in the same function where requested_ty is inspected so that
contains_assoc_projection(...) only triggers per-node projection processing
(recursing into Map, Tuple, Path, etc.) and do not treat a partially-open
associated type as a full wildcard that bypasses assoc matching or request
rewriting (also apply the same structural fix to the similar block around lines
handling the other occurrence noted in the review).
- Around line 1569-1618: The interface lookup in interface_view_from_registry
builds a packages list that omits dependency packages so implementations in
dependencies are missed; update interface_view_from_registry to expand the
package set the same way registry_dispatch_target_for_concrete does (include
dependency packages for the current file package and the target package) before
iterating registries—i.e., replace the current packages collection logic with
the dependency-expanded lookup used by registry_dispatch_target_for_concrete (or
call that helper) so PackageId::new /
baml_compiler2_tir::interfaces::package_implements_registry is consulted for all
dependency packages as well.
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 5427-5430: Run the formatter (rustfmt / project pre-commit
formatter) to fix the formatting of the diagnostic call: reformat the
self.context.report_simple(...) invocation that reports
TirTypeError::NotIterable { ty: coll_ty } with *collection so it matches the
project's style (proper line breaks, indentation and trailing commas) — locate
the call to self.context.report_simple and the TirTypeError::NotIterable
construct in builder.rs and re-run the formatter so CI/pre-commit hooks no
longer rewrite this block.
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 6003-6018: Formatting is inconsistent in the
receiver_class_type_arg_operands block (and other hunks listed) causing CI
pre-commit to rewrite the file; run the Rust formatter (rustfmt) or your
repository's pre-commit formatter on
baml_language/crates/baml_compiler2_mir/src/lower.rs and commit the resulting
changes so the code around receiver_class_type_arg_operands,
enclosing_generic_params, ty_to_template, builder.temp, Place::local and
Rvalue::LoadType matches the project's style; ensure the same formatting is
applied to the other affected ranges (around lines mentioned in the review)
before pushing.
---
Nitpick comments:
In `@baml_language/crates/baml_tests/baml_src/ns_iter/iter.baml`:
- Around line 59-62: Add a negative test alongside iter_for_in_range that
verifies a for-in loop panics on non-iterable input: create a new test named
like "iter_for_in_non_iterable" in the same file
(baml_language/crates/baml_tests/baml_src/ns_iter/iter.baml) which calls the
same lowering/iteration routine (mirror how iter_for_in_range() is used) but
passes a non-iterable value (e.g., a number or null) and asserts that the call
throws/panics; ensure the assertion uses the project test helper for expecting
failures (parallel to how successful cases use baml.deep_equals) so regressions
in the for (let x in value) non-iterable path are caught.
In `@baml_language/crates/bex_vm/src/vm.rs`:
- Around line 47-50: Add two focused unit tests inside the file-local
#[cfg(test)] module in vm.rs to cover the new templated runtime check branches:
(1) a test that constructs a frame_type_args entry and uses
TyTemplate::TypeArgRefOrWildcard(n) to verify an actual type that is a subtype
of the referenced expected type is accepted (exercise the
actual.is_subtype_of(expected) path), and (2) a test that builds an actual union
type containing a member not covered by the union guard to verify it is rejected
(exercise the union guard rejection path referenced around the same code and
lines 79-88). Locate the checks via TyTemplate::TypeArgRefOrWildcard,
frame_type_args, and Ty::BuiltinUnknown, assert the expected boolean result from
the IsType/runtime check, and run cargo test --lib to verify both tests pass.
🪄 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: fbb3cd82-a8d3-4840-a85b-3a735056afd8
⛔ Files ignored due to path filters (20)
baml_language/crates/baml_tests/snapshots/baml_src/closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/for_loops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/fs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/glob.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_auto_derive.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/lambdas.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/lexical_scoping.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/maps.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/type_error_repro.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/typed_inputs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/watch.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!**/*.snap
📒 Files selected for processing (6)
baml_language/crates/baml_builtins2/baml_std/baml/containers.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_iter/iter.bamlbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_tests/baml_src/ns_iter/iter.bamlbaml_language/crates/bex_vm/src/vm.rs
aaronvg
enabled auto-merge
June 6, 2026 05:32
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 094e684. Configure here.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_tests/tests/interfaces_associated_types.rs (1)
130-160: 💤 Low valueWell-structured helper for multi-file runtime tests.
The helper correctly compiles multiple BAML files, locates the entry function by name suffix, constructs a
BexEnginewith native ops, and executes the function with no arguments. The Arc wrapping is necessary sincecall_function_bound_argsrequires&Arc<Self>.Optional: improve error message clarity
BexEngine::new( program, Arc::new(sys_ops::SysOps::native()), None, Vec::new(), ) - .expect("engine") + .expect("BexEngine::new should succeed for compiled multi-file program")🤖 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/tests/interfaces_associated_types.rs` around lines 130 - 160, The panic when no function is found in run_multi_file_no_args is terse; update the unwrap_or_else used to select entry to include the requested entry_suffix and a short listing of available function names (from program.function_indices.keys()) to aid debugging so the panic message reads something like "function ending with `{entry_suffix}` not found; available functions: ..."; locate the selection/unwrap in run_multi_file_no_args and replace the panic with a formatted message that joins the keys into a string.
🤖 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/tests/interfaces_associated_types.rs`:
- Around line 130-160: The panic when no function is found in
run_multi_file_no_args is terse; update the unwrap_or_else used to select entry
to include the requested entry_suffix and a short listing of available function
names (from program.function_indices.keys()) to aid debugging so the panic
message reads something like "function ending with `{entry_suffix}` not found;
available functions: ..."; locate the selection/unwrap in run_multi_file_no_args
and replace the panic with a formatted message that joins the keys into a
string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2be4a73c-45e4-4a65-ae2a-b5f89346c25a
📒 Files selected for processing (2)
baml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_tests/tests/interfaces_associated_types.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml_compiler2_mir/src/lower.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/baml_compiler2_mir/src/lower.rs (1)
8408-8420:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUnify default-method owner-frame seeding.
These paths don't agree on where a default method's owner interface args live. Class-backed defaults are emitted with
Static(Vec::new()), type-backed defaults returncurrent_iface_args, andemit_method_candidate_switch()prependsowner_opsonly when theItemRefmatches the originally requested interface. That means same-interface type implementor defaults get interface args twice, while defaults inherited from arequiresparent get no provider associated-type frame args at all. In both cases the default body'senclosing_generic_params()layout is violated and generic/associated-type reads resolve incorrectly at runtime. Carry the provider interface view on the candidate and build the owner frame exactly once from that view. Please add unit coverage for both a same-interface primitive default and a required-interface inherited default.As per coding guidelines,
**/*.rs: Prefer writing Rust unit tests over integration tests where possible.Also applies to: 8530-8559, 9053-9067, 9874-9884
🤖 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/lower.rs` around lines 8408 - 8420, The code is seeding default-method owner frames inconsistently: when building InterfaceMethodCandidate in resolve_type_implementor_method you must carry the provider-interface view (the interface/type whose generic/associated params the default body uses) on the candidate instead of sometimes using CalleeFrameSeed::Static(Vec::new()) or current_iface_args; update InterfaceMethodCandidate to include a provider_iface_args (or reuse an existing field) and set it on all paths (class-backed and type-backed defaults and inherited requires), then modify emit_method_candidate_switch to build the owner frame once from that provider view (prepend owner_ops exactly once when ItemRef equals the originally requested interface using the new provider info) so enclosing_generic_params() and associated-type reads resolve consistently; add unit tests covering a same-interface primitive default and a required-interface inherited default to assert correct generic/associated-type layout at runtime.Source: Coding guidelines
🤖 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_compiler2_mir/src/lower.rs`:
- Around line 3306-3361: source_param_interface_view_for_name/current fast-path
uses only param.name and can incorrectly match a shadowed local or closure arg;
change the shortcut to first resolve the binding at the expression site and only
return the enclosing function-parameter's interface view when that resolved
binding is the same parameter binding from the function signature. Concretely,
inside source_param_interface_view_for_expr and
source_param_interface_view_for_name use the existing resolver/lookup used
elsewhere in this crate to map the Path/Name at expr_id to its Binding/VarId,
check that the binding corresponds to the function parameter (not a
local/closure shadow), and only then compute the param type and call
interface_dispatch_target_for_tir_ty; add a unit test that declares a function
parameter shadowed by a local or closure (e.g. fn f(x: T){ let x = ...;
x.method() } ) to assert the method resolves to the inner type, not the outer
parameter.
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 8408-8420: The code is seeding default-method owner frames
inconsistently: when building InterfaceMethodCandidate in
resolve_type_implementor_method you must carry the provider-interface view (the
interface/type whose generic/associated params the default body uses) on the
candidate instead of sometimes using CalleeFrameSeed::Static(Vec::new()) or
current_iface_args; update InterfaceMethodCandidate to include a
provider_iface_args (or reuse an existing field) and set it on all paths
(class-backed and type-backed defaults and inherited requires), then modify
emit_method_candidate_switch to build the owner frame once from that provider
view (prepend owner_ops exactly once when ItemRef equals the originally
requested interface using the new provider info) so enclosing_generic_params()
and associated-type reads resolve consistently; add unit tests covering a
same-interface primitive default and a required-interface inherited default to
assert correct generic/associated-type layout at runtime.
🪄 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: f5c52657-c3bd-4034-90ea-ab6acaab52cd
⛔ Files ignored due to path filters (21)
baml_language/crates/baml_tests/snapshots/baml_src/for_loops.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/lambdas.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/lexical_scoping.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/typed_inputs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/watch.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____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/closure_loop_variable/baml_tests__compiles__closure_loop_variable__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_advanced/baml_tests__compiles__lambda_advanced__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lexical_scoping/baml_tests__compiles__lexical_scoping__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/parser_statements/baml_tests__compiles__parser_statements__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__06_codegen.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!**/*.snap
📒 Files selected for processing (4)
baml_language/crates/baml_compiler2_emit/src/analysis.rsbaml_language/crates/baml_compiler2_emit/src/emit.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_tests/baml_src/ns_iter/iter.baml
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_compiler2_emit/src/analysis.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml_tests/baml_src/ns_iter/iter.baml
antoniosarosi
added a commit
that referenced
this pull request
Jun 8, 2026
Canary advanced 2 commits after round 4: 0a91ec3 [codex] iterable for loops (#3705) and 4a866e7 (windows lld build chore). The compiler source (builder.rs, lower.rs, emit.rs, analysis.rs) and vm.rs auto-merged cleanly against #3705 — only 4 generated snapshots conflicted; took canary's and regenerated to fold in our changes. Also fixes a class of flaky timing assertions exposed by cold-build runs. Three tests timed wall-clock around a call that COMPILES the (growing) BAML stdlib before running — so a cold/large compile blew past their promptness bounds, which actually guard against multi-second cancellation HANGS, not compile time: - task_group::task_group_cancel_cancels_members - cancel_cascade::cancel_unblocks_await_on_non_descendant - spawn_semantics::parent_throw_cancels_running_children Fix: compile OUTSIDE the timed region and measure only the engine run. Added a public `run_compiled(program, entry, args, show_auto_derive)` to the baml_tests engine harness (split out of run_test_with_options) and a `build_engine`/`run_engine` split in the bex_engine task_group test, so the assertions time execution alone. Bounds restored to tight values (2s) well under the sleeps they guard. Verified: cargo test --workspace --all-features (sdk_test_* excluded as in CI) — all suites green, including the three above and baml_src on a warm build; prek run --all-files clean. rustc 1.93.0.
This was referenced Jun 11, 2026
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
1 participant
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.
Summary
Iterableas the interface used to turn values into iterators, with arrays implementing it throughArrayIterator.for (let x in value)throughIterable.iter()andIterator.next()instead of the previous array/index path.Why
BAML
for insyntax should work like the Rust/JS iterable model: arrays and iterator adapters participate through the same interface machinery, so users do not need to writeIterator.new(...)or rely on compiler special cases.Validation
cargo check -p bex_vm -p baml_compiler2_mir -p baml_compiler2_tirINSTA_UPDATE=always cargo nextest run -p baml_tests __baml_std__cargo nextest run -p baml_tests --test baml_src baml_testINSTA_UPDATE=always cargo nextest run -p baml_tests --test baml_src bytecodegit diff --check.snap.newfilesNote
Medium Risk
Touches core MIR lowering, interface dispatch, and bytecode branch emission; behavior changes for all
for inloops and many interface method calls.Overview
for innow goes throughIterable/Iteratorinstead of index-based array lowering. MIR callsiter()andnext()untilDone; non-iterable collections panic at runtime. TIR infers loop variables fromIterable.Itemand foldsIterable.Errorinto throw analysis.Stdlib:
Arraygains fluent helpers (for_each,filter,filter_map,step_by,chain,peekable,collect,count) built onArrayIterator.Iteratoraddsfor_each;chainaccepts anyIterableand unions error types via a third generic onChain.Compiler fixes supporting dispatch: Interface lowering/dispatch is expanded—
tir_type_satisfies_dispatch_request, registry lookup across packages, iterableforlowering, associated-type frame args for interface defaults,Iterableparams on function signatures, and explicitObjecttype args. Bytecode emission no longer skips branch conditions when the else block is unreachable (regression test added). Closure/mapsnapshots pick up extraload_typefor element type args.Reviewed by Cursor Bugbot for commit eb8e1e0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Tests