🐛 fix(mq-check): handle union overloads with consistent return types#1386
🐛 fix(mq-check): handle union overloads with consistent return types#1386
Conversation
Fixes type inference for operators with union-typed arguments by checking if all union members yield the same return type. This prevents false-positive errors when unions are used with binary operators and improves Hindley-Milner inference robustness.
There was a problem hiding this comment.
Pull request overview
This PR fixes a false-positive type error in the mq-check type inference engine. Previously, using a union-typed value with a binary operator always triggered an error—even in cases where every member of the union would produce the same return type, making the operation safe. The fix adds a check (all_union_members_same_return) that tests each concrete member of the union against the operator's overloads; if all members resolve to the same return type, the operation is allowed.
Changes:
- Introduces the
all_union_members_same_returnblock (lines 470–503) that tests every concrete member of each union-typed operand against the operator's overloads to verify consistent return types. - Extends the condition at line 505 to also allow resolution when all union members yield the same return type, alongside the existing
union_params_are_varspath.
crates/mq-check/src/lib.rs
Outdated
| let all_union_members_same_return = { | ||
| let mut unique_ret: Option<types::Type> = None; | ||
| let mut all_match = true; | ||
| 'outer: for (i, arg_ty) in resolved_operands.iter().enumerate() { | ||
| if let types::Type::Union(members) = arg_ty { | ||
| // Reject unions containing unresolved type variables | ||
| if members.iter().any(|m| m.is_var()) { | ||
| all_match = false; | ||
| break 'outer; | ||
| } | ||
| for member in members { | ||
| let mut test_args = resolved_operands.clone(); | ||
| test_args[i] = member.clone(); | ||
| if let Some(types::Type::Function(_, member_ret)) = | ||
| ctx.resolve_overload(&d.op_name, &test_args) | ||
| { | ||
| let resolved_ret = ctx.resolve_type(&member_ret); | ||
| match &unique_ret { | ||
| None => unique_ret = Some(resolved_ret), | ||
| Some(prev) if prev == &resolved_ret => {} | ||
| _ => { | ||
| all_match = false; | ||
| break 'outer; | ||
| } | ||
| } | ||
| } else { | ||
| all_match = false; | ||
| break 'outer; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| all_match && unique_ret.is_some() | ||
| }; |
There was a problem hiding this comment.
The new all_union_members_same_return logic introduces a distinct success path that is not covered by any new tests. The existing test suite has tests for the union_params_are_vars path (e.g., to_number on a union type) and tests that union + arithmetic should fail, but no test exercises the new path where all union members yield the same return type from a concrete (non-Var-parameter) overload.
A test case such as an operator call with a union argument where every member of that union maps to the same return type would validate this new code path. Without it, a regression in this path would not be caught.
This is required by the project's testing conventions (see crates/mq-check/tests/integration_test.rs where all similar paths have comprehensive test coverage).
|
@copilot |
…lean helper function Co-authored-by: harehare <[email protected]>
♻️ refactor(mq-check): extract union_members_consistent_return into standalone helper
|
@copilot |
Fixes type inference for operators with union-typed arguments by checking if all union members yield the same return type. This prevents false-positive errors when unions are used with binary operators and improves Hindley-Milner inference robustness.