Repository navigation
Intersection bounds - #4291
Merged
Merged
Intersection bounds#4291
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ 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):
|
Contributor
📝 WalkthroughWalkthroughGeneric parameters now store all ChangesGeneric intersection bounds
Estimated code review effort: 5 (Critical) | ~120 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 |
Binary size checks passed✅ 7 passed
Generated by |
![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
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler2_ast/src/lib.rs (1)
979-1022: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the remaining declaration forms for intersection bounds.
This exercises function and interface declarations, but not classes, required interface methods, or free
implementsblocks, which each use the same new extractor. Add AST unit cases forT extends A & Bin those forms, then runcargo test --libfor this crate.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”.🤖 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_ast/src/lib.rs` around lines 979 - 1022, The existing test ast_preserves_every_bound_in_a_generic_param_intersection covers only interfaces and functions; extend AST unit coverage to include class declarations, required interface methods, and free implements blocks using T extends Named & Sized. Assert that every bound survives lowering in each form via the existing rendering helpers, then run cargo test --lib for the crate.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_tir/src/builder.rs`:
- Around line 11401-11421: In the unresolved-member fallback of resolve_member
for rigid receiver bounds, return Ty::Unknown instead of Ty::Error after
reporting TirTypeError::UnresolvedMember. Keep the existing diagnostic and
resolution flow unchanged, matching the neighboring AssociatedTypeProjection and
other unresolved-member arms.
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 968-983: Deduplicate the pooled sources in the generic-bounds
branch before ambiguity checks. Update the collection around
`associated_type_projection_sources_for_interface_bound` so identical realized
interface sources from overlapping conjuncts are retained once, while distinct
sources still trigger ambiguity; preserve the existing unknown and single-source
behavior.
In
`@baml_language/crates/baml_tests/projects/compiles/generic_intersection_bounds/generic_intersection_bounds.baml`:
- Around line 51-70: Add a function that directly returns item.name from a
bounded generic type variable, exercising the Return terminator path, and invoke
it from main alongside the existing field_into_call_arg and field_into_switch
consumers. Keep the test focused on the same Named & Sized constraint and
virtual field access behavior.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler2_ast/src/lib.rs`:
- Around line 979-1022: The existing test
ast_preserves_every_bound_in_a_generic_param_intersection covers only interfaces
and functions; extend AST unit coverage to include class declarations, required
interface methods, and free implements blocks using T extends Named & Sized.
Assert that every bound survives lowering in each form via the existing
rendering helpers, then run cargo test --lib for the crate.
🪄 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 Plus
Run ID: 6b011a87-2ba5-446d-8b29-8a6b72327067
⛔ Files ignored due to path filters (12)
baml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/intersection_bounds.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__10_formatter__generic_intersection_bounds.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__10_formatter__generic_intersection_bounds.snapis excluded by!**/*.snap
📒 Files selected for processing (38)
baml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_cli/src/bytecode_cache.rsbaml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/companions.rsbaml_language/crates/baml_compiler2_ast/src/lib.rsbaml_language/crates/baml_compiler2_ast/src/lower_cst.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_emit/src/stack_carry.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/item_tree.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/builder.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/classes.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/common.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/functions.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/interfaces.rsbaml_language/crates/baml_compiler2_hir/src/signature.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/classes.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/common.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/functions.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/impls.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/interfaces.rsbaml_language/crates/baml_compiler2_ppir/src/lib.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/builder/associated_projection.rsbaml_language/crates/baml_compiler2_tir/src/builder/interface_resolution.rsbaml_language/crates/baml_compiler2_tir/src/generic_env.rsbaml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/interfaces/impl_rules.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_lsp2_actions/src/type_info.rsbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/crates/baml_tests/baml_src/ns_intersection_bounds/intersection_bounds.bamlbaml_language/crates/baml_tests/projects/compiles/generic_intersection_bounds/generic_intersection_bounds.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/generic_intersection_bounds/generic_intersection_bounds.bamlbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase5.rsbaml_language/crates/baml_tests/src/incremental/scenarios.rs
💤 Files with no reviewable changes (2)
- baml_language/crates/baml_compiler2_ast/src/companions.rs
- baml_language/crates/baml_compiler2_hir/src/item_tree/builder.rs
There was a bug in the LSP one so I deleted the LSP one.
2kai2kai2
force-pushed
the
kai/intersection-bounds
branch
from
July 30, 2026 21:47
6ee9ac7 to
04d03ae
Compare
2kai2kai2
enabled auto-merge
July 30, 2026 21:47
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
baml_language/crates/baml_tests/projects/compiles/generic_intersection_bounds/generic_intersection_bounds.baml (1)
51-70: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComment claims three stack-carry routes; only two are exercised.
No function here returns a bare interface-field read off a bounded type variable —
describeconcatenates,label_ofreturns a method call, andholder.item.name(line 121) is a concrete class field. Either add the missing case or dropReturnfrom the comment.🧪 Proposed addition
+function field_into_return<T extends Named & Sized>(item: T) -> string { + item.name +} + function field_into_call_arg<T extends Named & Sized>(item: T) -> string {…then thread it through
mainalongside the other two consumers.🤖 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/projects/compiles/generic_intersection_bounds/generic_intersection_bounds.baml` around lines 51 - 70, Add a test function in the generic intersection-bounds fixture that directly returns an interface-field read from a bounded type variable, then invoke it from main alongside the existing call-argument and switch consumers. Update the surrounding comments only as needed to accurately describe all three exercised stack-carry routes.
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler2_mir/src/lower.rs (1)
99-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid the unnecessary
bounds_mapclone when constructingGlobalTypeContext.
TypeVarBoundsMapaliasesFxHashMap<ParamTy, Vec<baml_type::Interface>>, andGlobalTypeContext::boundsonly borrows that map, so passboundsdirectly instead of cloning the per-call bounds vectors.♻️ Proposed change
- let bounds_map: baml_compiler2_tir::lower_type_expr::TypeVarBoundsMap = bounds.clone(); let res_ctx = baml_compiler2_tir::package_interface::package_resolution_context(db, pkg_id); let ctx = baml_compiler2_tir::type_context::GlobalTypeContext { db, res_ctx, aliases, - bounds: &bounds_map, + bounds, };🤖 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 99 - 102, Remove the unnecessary bounds_map clone in the surrounding helper and pass the existing bounds reference directly when constructing GlobalTypeContext. Update the constructor call to match GlobalTypeContext::bounds borrowing semantics, preserving all other type-context behavior.
🤖 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_ppir/src/item_data/common.rs`:
- Around line 23-27: Update the GenericParamData struct to derive salsa::Update
alongside its existing traits, ensuring Vec<GenericParamData> satisfies the
Salsa Update requirement used by ClassData and InterfaceData.
In `@baml_language/crates/baml_compiler2_tir/src/interfaces.rs`:
- Around line 954-965: Update the deduplication in the interface-declarer lookup
around interface_closure_locs and the seen set so distinct realized interface
instantiations, including their type arguments, remain separate instead of
collapsing by InterfaceLoc. Use the same realized-interface identity as TIR
dispatch, or change the helper to receive and deduplicate realized bounds while
preserving associated-type filtering and output behavior.
---
Duplicate comments:
In
`@baml_language/crates/baml_tests/projects/compiles/generic_intersection_bounds/generic_intersection_bounds.baml`:
- Around line 51-70: Add a test function in the generic intersection-bounds
fixture that directly returns an interface-field read from a bounded type
variable, then invoke it from main alongside the existing call-argument and
switch consumers. Update the surrounding comments only as needed to accurately
describe all three exercised stack-carry routes.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 99-102: Remove the unnecessary bounds_map clone in the surrounding
helper and pass the existing bounds reference directly when constructing
GlobalTypeContext. Update the constructor call to match
GlobalTypeContext::bounds borrowing semantics, preserving all other type-context
behavior.
🪄 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 Plus
Run ID: c78b9e5c-8b4f-4640-8ac8-e316938b1d46
⛔ Files ignored due to path filters (12)
baml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/intersection_bounds.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_intersection_bounds/baml_tests__compiles__generic_intersection_bounds__10_formatter__generic_intersection_bounds.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_intersection_bounds/baml_tests__diagnostic_errors__generic_intersection_bounds__10_formatter__generic_intersection_bounds.snapis excluded by!**/*.snap
📒 Files selected for processing (39)
baml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_cli/src/bytecode_cache.rsbaml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/companions.rsbaml_language/crates/baml_compiler2_ast/src/lib.rsbaml_language/crates/baml_compiler2_ast/src/lower_cst.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_emit/src/stack_carry.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/item_tree.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/builder.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/classes.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/common.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/functions.rsbaml_language/crates/baml_compiler2_hir/src/item_tree/interfaces.rsbaml_language/crates/baml_compiler2_hir/src/signature.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/classes.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/common.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/functions.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/impls.rsbaml_language/crates/baml_compiler2_ppir/src/item_data/interfaces.rsbaml_language/crates/baml_compiler2_ppir/src/lib.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/builder/associated_projection.rsbaml_language/crates/baml_compiler2_tir/src/builder/interface_resolution.rsbaml_language/crates/baml_compiler2_tir/src/generic_env.rsbaml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/interfaces.rsbaml_language/crates/baml_compiler2_tir/src/interfaces/impl_rules.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_lsp2_actions/src/type_info.rsbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/crates/baml_tests/baml_src/ns_intersection_bounds/intersection_bounds.bamlbaml_language/crates/baml_tests/projects/compiles/generic_intersection_bounds/generic_intersection_bounds.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/generic_intersection_bounds/generic_intersection_bounds.bamlbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase5.rsbaml_language/crates/baml_tests/src/incremental/scenarios.rs
💤 Files with no reviewable changes (2)
- baml_language/crates/baml_compiler2_hir/src/item_tree/builder.rs
- baml_language/crates/baml_compiler2_ast/src/companions.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- baml_language/crates/baml_compiler2_tir/src/builder/associated_projection.rs
- baml_language/crates/baml_compiler2_ast/src/lib.rs
Comment on lines
+23
to
+27
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct GenericParamData { | ||
| pub name: Name, | ||
| pub bounds: Vec<TypeRefId>, | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cd baml_language
cargo test --libRepository: BoundaryML/baml
Length of output: 145
🏁 Script executed:
#!/bin/bash
set -u
echo "== locate common.rs and salsa dependency/version =="
git ls-files | rg '(^|/)common\.rs$|cargo\.toml|baml_compiler2_ppir' | head -200
echo
echo "== relevant struct definitions =="
for f in baml_language/crates/baml_compiler2_ppir/src/item_data/common.rs baml_language/crates/baml_compiler2_ppir/src/item_data/*.rs; do
if [ -f "$f" ]; then
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view compact 2>/dev/null | head -120 || true
echo
fi
done
echo
echo "== search for salsa::Update and GenericParamData =="
rg -n "salsa::Update|struct GenericParamData|Vec<GenericParamData>|GenericParamData" baml_language/crates/baml_compiler2_ppir -S
echo
echo "== cargo workspace salsa dependency =="
rg -n 'name = "salsa"|salsa =|\.salsa' baml_language -SRepository: BoundaryML/baml
Length of output: 145
🏁 Script executed:
#!/bin/bash
set -u
echo "== lockfile salsa entry =="
fd -a 'Cargo.lock' . | while read -r lock; do
echo "--- $lock"
rg -n 'name = "salsa"|version = ".*salsa|salsa ' "$lock" | head -80
done
echo
echo "== read relevant file slices =="
sed -n '1,90p' baml_language/crates/baml_compiler2_ppir/src/item_data/common.rs
for f in baml_language/crates/baml_compiler2_ppir/src/item_data/*.rs; do
if rg -q "struct ClassData|struct InterfaceData|Vec<GenericParamData>" "$f"; then
echo "--- $f"
rg -n -C 8 "struct ClassData|struct InterfaceData|Vec<GenericParamData>" "$f"
fi
doneRepository: BoundaryML/baml
Length of output: 8611
🏁 Script executed:
#!/bin/bash
set -u
echo "== source salsa package metadata =="
cd baml_language
cargo metadata --no-deps --format-version 1 2>/dev/null | jq -r '.packages[] | select(.name == "salsa") | {name, version, source, dependencies}'
echo
echo "== lockfile salsa dependency lines =="
sed -n '6506,6545p' Cargo.lock
echo
echo "== local salsa API text, if available =="
for d in baml_language/target/registry/src baml_language/target/debug/build baml_language/target/debug/deps; do
[ -d "$d" ] && echo "FOUND $d"
done
rg -n 'impl<T: salsa::Update|struct Update|enum Update|derive\(salsa::Update\)' "$HOME"/.cargo/registry/src/cache/*/salsa-*/src 2>/dev/null | head -80 || true
echo
echo "== local crate salsa usage =="
rg -n "salsa =|salsa::Update|salsa::\([" baml_language/crates/baml_compiler2_ppir/Cargo.toml baml_language/crates/baml_compiler2_ppir/src -SRepository: BoundaryML/baml
Length of output: 145
🏁 Script executed:
#!/bin/bash
set -u
echo "== metadata salsa package (targeted) =="
cargo metadata --format-version 1 2>/dev/null | \
python3 - <<'PY' >/tmp/salsa_pkg.json
import json, sys
data=json.load(sys.stdin)
for p in data.get('packages', []):
if p.get('name') == 'salsa':
print(json.dumps({
'name': p['name'],
'version': p['version'],
'source': p.get('source'),
}, indent=2))
break
else:
print('not found')
PY
cat /tmp/salsa_pkg.json
echo
echo "== lockfile salsa block =="
sed -n '6506,6525p' Cargo.lock
echo
echo "== salsa source availability and Update derive trait bounds =="
for base in \
"${CARGO_USER_HOME:-"$HOME"}/.cargo/registry/src/cache" \
"${CARGO_HOME:-"$HOME/.cargo"}/registry/src/cache" \
"$HOME/.cargo/registry/src" \
"$CARGO_HOME/registry/src"; do
if [ -d "$base" ]; then
echo "-- searching $base"
find "$base" -type d -name 'salsa-*' 2>/dev/null | head -5 | while read -r dir; do
echo "=== $dir"
rg -n -C 4 'impl<T: .*Update|impl Update for Vec|derive\(Update\)|struct Update' "$dir/src" "$dir" 2>/dev/null | head -120 || true
done
break
fi
done
echo
echo "== compiler2_ppir salsa imports/usages =="
cat baml_language/crates/baml_compiler2_ppir/Cargo.toml
rg -n "salsa::Update|salsa::|struct TypeRefStore|Vec<GenericParamData>" baml_language/crates/baml_compiler2_ppir/src -SRepository: BoundaryML/baml
Length of output: 145
🌐 Web query:
salsa 0.19 derive Update Vec<T> T: Update
💡 Result:
In Salsa, the salsa::Update trait is responsible for determining if a value has changed between database revisions [1]. When using #[derive(salsa::Update)] on a struct or enum, the derive macro automatically generates an implementation [2][3]. For types like Vec<T>, the default behavior of #[derive(salsa::Update)] typically requires that the element type T also implements salsa::Update to satisfy trait bounds [4]. However, Salsa provides mechanisms to handle cases where this requirement might be inconvenient or where a different update strategy is preferred: 1. Escape Hatch: Recent versions of Salsa have introduced improvements to the Update derive macro, such as helper attributes, which allow you to relax the T: Update bound [4]. 2. Fallback Helper: You can use the #[salsa::update(fallback)] attribute [5]. This is suitable for fully owned types that implement Eq, where the update logic reverts to checking for equality (updating only if the new value is not equal to the old one) [5]. 3. Manual Implementation: If your specific Vec<T> usage needs custom logic, you can implement the salsa::Update trait manually, provided you adhere to the safety requirements for unsafe fn maybe_update [1]. This method must correctly compare the old and new values to determine if an update is necessary [1]. It is important to note that salsa::Update cannot be implemented for types containing references tied to the database (e.g., &'db T), as these may point to memory that was modified or freed in a newer revision, making Eq comparisons invalid [1]. Top results: [1][2][5][4]
Citations:
- 1: https://docs.rs/salsa/latest/salsa/trait.Update.html
- 2: https://docs.rs/salsa/latest/salsa/derive.Update.html
- 3: https://docs.rs/salsa-macros/latest/salsa_macros/derive.Update.html
- 4: salsa-rs/salsa@9422bda
- 5: https://docs.rs/salsa/latest/salsa/index.html
🌐 Web query:
github salsa-rs salsa derive Update Vec T element T: salsa::Update
💡 Result:
In the salsa-rs library, the salsa::Update trait is designed to allow data structures to be efficiently updated or reused across revisions within a Salsa database [1]. For standard library types, including Vec<T>, the trait is already implemented in the salsa crate [1]. Specifically, salsa provides an implementation for Vec<T> where T satisfies salsa::Update: impl Update for Vec where T: Update {... } [1] This means that if your Vec contains elements that also implement salsa::Update, you do not need to write a manual implementation; it is handled automatically [1]. If you are using a custom struct and wish to derive salsa::Update for it, you can use the #[derive(Update)] macro provided by the salsa-macros crate [2][3]. When using this derive macro, the compiler will generate the necessary implementation. If your struct contains fields that are not automatically Update compatible, or if you need to customize the behavior, you can use helper attributes such as #[update] to control the implementation [2][3]. Recent updates to the library have also improved the ergonomics of these derive helpers, including support for custom bounds and escape hatches for generic types [4]. In summary, you do not need to manually implement salsa::Update for a Vec<T> if T already implements it [1]. For your own data structures, simply add #[derive(salsa::Update)] to your type definition [2][5].
Citations:
- 1: https://docs.rs/salsa/latest/salsa/trait.Update.html
- 2: https://docs.rs/salsa/latest/salsa/derive.Update.html
- 3: https://docs.rs/salsa-macros/latest/salsa_macros/derive.Update.html
- 4: salsa-rs/salsa@9422bda
- 5: https://docs.rs/salsa/latest/salsa/index.html
Derive salsa::Update for GenericParamData.
ClassData and InterfaceData retain salsa::Update, but they contain Vec<GenericParamData>, and Salsa’s Update implementation for Vec<T> requires T: salsa::Update, so this will fail to compile until GenericParamData also derives the trait.
Proposed fix
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, salsa::Update)]
pub struct GenericParamData {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[derive(Debug, Clone, PartialEq, Eq)] | |
| pub struct GenericParamData { | |
| pub name: Name, | |
| pub bounds: Vec<TypeRefId>, | |
| } | |
| #[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] | |
| pub struct GenericParamData { | |
| pub name: Name, | |
| pub bounds: Vec<TypeRefId>, | |
| } |
🤖 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_ppir/src/item_data/common.rs` around
lines 23 - 27, Update the GenericParamData struct to derive salsa::Update
alongside its existing traits, ensuring Vec<GenericParamData> satisfies the
Salsa Update requirement used by ClassData and InterfaceData.
Source: Coding guidelines
Comment on lines
+954
to
+965
| let mut seen: FxHashSet<baml_compiler2_hir::loc::InterfaceLoc<'db>> = FxHashSet::default(); | ||
| for root in roots { | ||
| for loc in interface_closure_locs(db, root) { | ||
| if !seen.insert(loc) { | ||
| continue; | ||
| } | ||
| if baml_compiler2_ppir::item_data::interface_data(db, loc) | ||
| .associated_types | ||
| .iter() | ||
| .any(|assoc| assoc.name == *member) | ||
| { | ||
| out.push(loc); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve interface instantiations when deduplicating declarers.
InterfaceLoc collapses distinct bounds such as I<int> and I<string>, so T.Item can be treated as unambiguous even though those are distinct associated-type projections. TIR dispatch deduplicates by realized interface identity, including arguments; this LSP-facing helper should use the same identity or accept realized bounds rather than bare locations.
🤖 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_tir/src/interfaces.rs` around lines 954 -
965, Update the deduplication in the interface-declarer lookup around
interface_closure_locs and the seen set so distinct realized interface
instantiations, including their type arguments, remain separate instead of
collapsing by InterfaceLoc. Use the same realized-interface identity as TIR
dispatch, or change the helper to receive and deduplicate realized bounds while
preserving associated-type filtering and output behavior.
codeshaunted
added a commit
that referenced
this pull request
Aug 13, 2026
Brings the branch up to date with canary (146 conflicted files) and ports every TIR-side change onto the hir_ty substrate: - #4311 equirecursive types: canary's mu-automaton (phase-typed NormalTy<Named|Canonical>, de Bruijn binders, canonical_bottom_up) merged with this branch's B-1091 co-inductive assumption threading; the interned entry re-phased through the same pipeline. Same-shape recursive aliases now provably overlap (coherence twin updated). - #4320 tooling surface: hir_ty gains callable::function_signature_ty (a view over function_signature; own generics only; Self resolved for interface-default and free-impl methods), interfaces:: {InterfaceDeclScope, resolve_interface_fields, resolve_interface_required_methods, interfaces_declaring_associated_type}, and package_interface::exported_function (one place pairing the signature query with the effective-throws oracle). baml_surface is re-pointed tir -> hir_ty with its facts.rs contract intact. - #4308 catch-arm interface facts: reachability respects implements in both directions (the semantic contract compiles with zero diagnostics); the arm lowering keeps the modular Interface narrow rather than the closed-world concrete residual. - #4291 intersection bounds, consumption side: GenericParamData conjunction shapes adopted end to end; class constructor sites register one Implements obligation per declared bound conjunct (register_class_bounds, rustc's ADT well-formedness discipline); interface-member lookup pools declarers across the bound conjunction, dedupes by realized identity through requires, and reports E0121/E0122 ambiguity (rustc's MethodError::Ambiguity shape) instead of first-conjunct-wins; a failing blanket-impl bound names the unsatisfied conjunct (BlanketBoundNotSatisfied) instead of a bare mismatch. - #4352 @spec/ai builtins compile and emit on hir_ty. Four pre-existing engine gaps they exposed are fixed: * a bare AnyFunction's unpinned members read as their declared `unknown` defaults at the oracle and in the engine's same-interface unification (BEP-062 lazy default; no eager fill at lowering); * binding a bounded inference var by direct unification now REPLAYS its accumulated VarBounds against the solution (take_solved_class_bounds + replay in the finish fixpoint) instead of dropping them - the map-lambda + future.all shape no longer strands its type args; * an empty container literal flowing into a ground `unknown` demand commits its establishment slots to the top type (the demand is a consuming use); a literal with NO demand keeps the strict uninferrable-container error; * required interface methods (bodyless items under the unified method model) are excluded from the codegen symbol pool. - Coherence: canary's PreparedImpl refactor adopted on Facts::with_bounds - the subject-validity gate now judges the same normalized spelling E0138 judges, memoized per impl. - baml_project CFG dispatch (canary's interface virtual-call resolution in the visualization) re-pointed onto hir_ty's InferenceResult and impls_for_type. - check.rs: the legacy jinja prompt checker is deleted with canary's `client<llm>`/jinja removal; the associated-type declarer walk uses the conjunction-deduping interfaces_declaring_associated_type. - Tests: fixtures migrated off removed builtins (baml.deep_equals -> ops.Equals `==`; a local generic pair fn where the fixture probes inference); snapshots re-blessed for the unified required-method model ([missing] items), canary's builtin/std content, and canonical union order.
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.
Summary by CodeRabbit
New Features
T extends A & B.Bug Fixes
Tests