Repository navigation
fix(compiler): defer inferred generic argument diagnostics - #4160
codeshaunted wants to merge 1 commit into
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):
|
📝 WalkthroughWalkthroughThe compiler changes generic argument handling by distinguishing directly inferable type variables from substituted types containing variables. A diagnostic test program adds mismatched generic-call scenarios covering return context, ignored results, concrete parameters, and explicit type arguments. ChangesGeneric argument checking
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler2_tir/src/builder.rs (1)
3776-3789: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared "rigid vs. inferable typevar" predicate.
direct_inference_varhere is logically identical to therigid/defers_typevarcheck later in this same function (~lines 4132-4138): applying De Morgan's law to!(rigid_self_var == Some(name) || (self.generic_params.contains(name) && !generic_params.contains(name)))yields exactlyrigid_self_var != Some(name) && (!self.generic_params.contains(name) || generic_params.contains(name))— the same formula. Duplicating this subtle boolean (which encodes "is this typevar the caller's own rigid generic vs. one the callee can still infer") risks the two copies silently drifting apart in a future edit and reintroducing the cascading-diagnostic bug this PR fixes.♻️ Suggested consolidation
+ fn is_directly_inferable_typevar( + &self, + name: &Name, + callee_generic_params: &[Name], + rigid_self_var: Option<&Name>, + ) -> bool { + rigid_self_var != Some(name) + && (!self.generic_params.iter().any(|p| p == name) + || callee_generic_params.iter().any(|p| p == name)) + }Then use it at both call sites (here, and in the
defers_typevarclosure) instead of re-deriving the formula.🤖 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/builder.rs` around lines 3776 - 3789, Extract the shared rigid-versus-inferable type-variable predicate from the current direct_inference_var logic into a local helper or closure within the function. Replace both the direct_inference_var calculation and the later defers_typevar check with this shared predicate, preserving the existing inputs and behavior.Source: Linters/SAST tools
🤖 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_compiler2_tir/src/builder.rs`:
- Around line 3776-3789: Extract the shared rigid-versus-inferable type-variable
predicate from the current direct_inference_var logic into a local helper or
closure within the function. Replace both the direct_inference_var calculation
and the later defers_typevar check with this shared predicate, preserving the
existing inputs and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f19aeddc-6e2e-4fad-8f7a-2d924818f123
⛔ Files ignored due to path filters (6)
baml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_union_argument_no_cascade/baml_tests__diagnostic_errors__generic_union_argument_no_cascade__01_lexer__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_union_argument_no_cascade/baml_tests__diagnostic_errors__generic_union_argument_no_cascade__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_union_argument_no_cascade/baml_tests__diagnostic_errors__generic_union_argument_no_cascade__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_union_argument_no_cascade/baml_tests__diagnostic_errors__generic_union_argument_no_cascade__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_union_argument_no_cascade/baml_tests__diagnostic_errors__generic_union_argument_no_cascade__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_union_argument_no_cascade/baml_tests__diagnostic_errors__generic_union_argument_no_cascade__10_formatter__main.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
baml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_tests/projects/diagnostic_errors/generic_union_argument_no_cascade/main.baml
Binary size checks passed✅ 7 passed
Generated by |
Summary
Tests
Linear: https://linear.app/boundaryml2/issue/B-932/union-inferred-generic-call-incorrectly-rejects-a-contributing-argument
Summary by CodeRabbit
Bug Fixes
Tests