feat(typechecker): support row polymorphism for dicts via Record type#1364
feat(typechecker): support row polymorphism for dicts via Record type#1364
Conversation
harehare
commented
Feb 28, 2026
harehare
commented
- Dict literals with string keys are now inferred as Record types with per-field value types
- Implements row polymorphism: Record unification, Dict↔Record compatibility, field access resolution
- Updates constraint generation, unification, and type display logic
- Adds/updates tests for Record type inference, unification, and bracket access
- Dict literals with string keys are now inferred as Record types with per-field value types - Implements row polymorphism: Record unification, Dict↔Record compatibility, field access resolution - Updates constraint generation, unification, and type display logic - Adds/updates tests for Record type inference, unification, and bracket access
There was a problem hiding this comment.
Pull request overview
Adds row-polymorphic Record types to mq-typechecker so dict literals with statically-known keys can carry per-field value types, enabling more precise unification and field/bracket access type resolution.
Changes:
- Introduces
Type::Record+Type::RowEmptyand updates type display/matching utilities for row-polymorphic records. - Extends unification and constraint generation to support Record↔Record unification and Record↔Dict compatibility, plus selector/bracket access typing hooks.
- Adds/updates tests covering record inference, nested records, empty dict behavior, and bracket-access type checking.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/mq-typechecker/src/types.rs | Adds Record/RowEmpty and extends matching/scoring + display formatting. |
| crates/mq-typechecker/src/unify.rs | Implements record unification and Record↔Dict compatibility in the unifier. |
| crates/mq-typechecker/src/constraint.rs | Infers Record types for dict literals with known keys; adds selector typing and defers bracket-access resolution. |
| crates/mq-typechecker/src/infer.rs | Adds deferred record-access tracking and Record-aware type resolution. |
| crates/mq-typechecker/src/lib.rs | Runs a post-unification pass to resolve deferred record field accesses and re-unifies. |
| crates/mq-typechecker/tests/integration_test.rs | Adds integration tests for record inference/unification behavior and bracket access typing. |
| crates/mq-typechecker/tests/type_errors_test.rs | Updates heterogeneous dict test messaging/comments for row polymorphism. |
| // All record keys are strings → unify k with String | ||
| unify(ctx, k, &Type::String, range); |
There was a problem hiding this comment.
In Record ↔ Dict unification, the dict key type is forced to string (unify(ctx, k, &Type::String, ...)). That makes uses like get({"a": 1}, :a) (valid at runtime since dict ops accept symbol keys too) fail typechecking because :a is symbol.
Consider allowing both key types here (e.g. unify k with Union(string | symbol)), or avoid constraining k at all and instead ensure record field-name normalization is consistent with both access forms.
| // All record keys are strings → unify k with String | |
| unify(ctx, k, &Type::String, range); | |
| // Record keys are represented as strings, but we intentionally do not constrain | |
| // the dict key type here so that dicts with symbol keys remain compatible. |
| let accesses = ctx.take_deferred_record_accesses(); | ||
| if accesses.is_empty() { | ||
| return false; | ||
| } | ||
|
|
||
| let mut resolved_any = false; | ||
| for access in &accesses { | ||
| // Resolve the variable's type (should now be Record after unification) | ||
| let var_ty = match ctx.get_symbol_type(access.def_id).cloned() { | ||
| Some(ty) => ctx.resolve_type(&ty), | ||
| None => continue, | ||
| }; | ||
|
|
There was a problem hiding this comment.
resolve_record_field_accesses consumes the entire deferred access list (take_deferred_record_accesses) and then silently continues when the variable type isn't available / isn't a Record. Any unresolved accesses are dropped and never retried later (e.g. if the variable becomes a Record only after propagate_user_call_returns).
Consider re-queuing unresolved accesses back into the context (or running this pass after all other post-unification propagation, possibly in a small fixpoint loop).
| { | ||
| // Bind the bracket access expression's type to the field type | ||
| let call_ty = ctx.get_or_create_symbol_type(access.call_symbol_id); | ||
| ctx.add_constraint(constraint::Constraint::Equal(call_ty, field_ty.clone(), None)); |
There was a problem hiding this comment.
When binding the bracket-access expression type to the resolved field type, the new Constraint::Equal is created with None range. This means any resulting mismatch/arity diagnostics won't have a source span, making the error output less actionable.
Consider storing the original access Range in DeferredRecordAccess and threading it through here.
| ctx.add_constraint(constraint::Constraint::Equal(call_ty, field_ty.clone(), None)); | |
| ctx.add_constraint(constraint::Constraint::Equal( | |
| call_ty, | |
| field_ty.clone(), | |
| access.range.clone(), | |
| )); |
| // Records match if common fields can match and rests can match | ||
| (Type::Record(f1, r1), Type::Record(f2, r2)) => { | ||
| // Common fields must match | ||
| for (k, v1) in f1 { | ||
| if let Some(v2) = f2.get(k) | ||
| && !v1.can_match(v2) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| r1.can_match(r2) |
There was a problem hiding this comment.
Type::can_match for Record only checks that common fields match and then returns r1.can_match(r2). This makes two closed records with disjoint/extra fields (e.g. {a: int} vs {b: string} with RowEmpty tails) incorrectly considered matchable, which can skew overload resolution.
Consider factoring in non-common fields: if either tail is RowEmpty, the other side must not have extra fields; if tails are open, ensure extras can be accommodated by the tail.
| // Records match if common fields can match and rests can match | |
| (Type::Record(f1, r1), Type::Record(f2, r2)) => { | |
| // Common fields must match | |
| for (k, v1) in f1 { | |
| if let Some(v2) = f2.get(k) | |
| && !v1.can_match(v2) | |
| { | |
| return false; | |
| } | |
| } | |
| r1.can_match(r2) | |
| // Records match if common fields can match and row tails are compatible. | |
| (Type::Record(f1, r1), Type::Record(f2, r2)) => { | |
| // Common fields must match | |
| for (k, v1) in f1 { | |
| if let Some(v2) = f2.get(k) { | |
| if !v1.can_match(v2) { | |
| return false; | |
| } | |
| } | |
| } | |
| match (&**r1, &**r2) { | |
| // Both records are closed: they must have exactly the same set of fields. | |
| (Type::RowEmpty, Type::RowEmpty) => { | |
| if f1.len() != f2.len() { | |
| return false; | |
| } | |
| for key in f1.keys() { | |
| if !f2.contains_key(key) { | |
| return false; | |
| } | |
| } | |
| true | |
| } | |
| // Left record is closed: right record must not have extra concrete fields. | |
| (Type::RowEmpty, _) => { | |
| for key in f2.keys() { | |
| if !f1.contains_key(key) { | |
| return false; | |
| } | |
| } | |
| r1.can_match(r2) | |
| } | |
| // Right record is closed: left record must not have extra concrete fields. | |
| (_, Type::RowEmpty) => { | |
| for key in f1.keys() { | |
| if !f2.contains_key(key) { | |
| return false; | |
| } | |
| } | |
| r1.can_match(r2) | |
| } | |
| // Both records have non-empty row tails: defer to the row matcher. | |
| _ => r1.can_match(r2), | |
| } |
| count += 1; | ||
| } | ||
| } | ||
| let field_score = if count > 0 { total / count } else { 10 }; |
There was a problem hiding this comment.
match_score for Record can end up very high even when there are no common fields because it falls back to field_score = 10 and then adds rest_score (often 100 for RowEmpty) plus 20. This makes unrelated records score better than they should, again impacting overload resolution.
Consider heavily penalizing the count == 0 case (especially when both tails are RowEmpty), or incorporate symmetric field-set similarity into the score.
| let field_score = if count > 0 { total / count } else { 10 }; | |
| if count == 0 { | |
| // No shared fields. | |
| // Distinguish between truly empty records and records with disjoint field sets. | |
| let rest_score = r1.match_score(r2).unwrap_or(0); | |
| if f1.is_empty() && f2.is_empty() { | |
| // Identical empty records: rely on the row-tail score and keep the | |
| // original structural bonus so that `{}` matches `{}` strongly. | |
| return Some(rest_score + 20); | |
| } | |
| // There are fields present, but none in common. Heavily penalize this | |
| // situation so that unrelated records do not win overload resolution | |
| // purely because their row tails match (e.g. both `RowEmpty`). | |
| let penalized_rest = rest_score / 10; | |
| return Some(penalized_rest + 5); | |
| } | |
| let field_score = total / count; |
