Conversation
Owner
harehare
commented
Mar 6, 2026
harehare
commented
- Move keyword filtering logic to get_non_keyword_children for reuse
- Add InferenceContext::report_mismatch and report_no_matching_overload for error reporting
- Add format_type_list for error message formatting
- Refactor unify to use report_mismatch for cleaner error handling
- Minor normalization and early exit improvements
…lpers, improve type formatting - Move keyword filtering logic to get_non_keyword_children for reuse - Add InferenceContext::report_mismatch and report_no_matching_overload for error reporting - Add format_type_list for error message formatting - Refactor unify to use report_mismatch for cleaner error handling - Minor normalization and early exit improvements
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors mq-check’s type inference/type-checking internals by extracting shared helper logic (non-keyword child extraction, error reporting helpers, and type list formatting) and applying small performance/clarity improvements while keeping behavior largely the same.
Changes:
- Centralize mismatch / overload error construction via new
InferenceContexthelper methods and use them from unification. - Extract keyword-filtered child collection into
get_non_keyword_childrenand reuse it in constraint generation and narrowing. - Minor optimizations: pre-allocate union normalization, early-return in
apply_substfor empty substitutions, and early-exit overload selection on perfect matches.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/mq-check/src/unify.rs | Uses InferenceContext::report_mismatch to reduce duplicated mismatch error construction. |
| crates/mq-check/src/types.rs | Adds format_type_list and minor perf improvements to union normalization and substitution application. |
| crates/mq-check/src/narrowing.rs | Switches to shared non-keyword child extraction helper for call argument analysis. |
| crates/mq-check/src/infer.rs | Adds error reporting helpers and reuses shared type-list formatting; adds overload-resolution early exit. |
| crates/mq-check/src/constraint.rs | Introduces get_non_keyword_children and refactors call-argument extraction to use it; minor import/path cleanups. |
crates/mq-check/src/types.rs
Outdated
Comment on lines
+575
to
+581
| pub fn format_type_list(types: &[Type]) -> String { | ||
| types | ||
| .iter() | ||
| .map(|t| t.display_renumbered()) | ||
| .collect::<Vec<_>>() | ||
| .join(", ") | ||
| } |
There was a problem hiding this comment.
New helper format_type_list is user-visible via error message formatting but currently has no unit test coverage in types.rs's existing test module. Adding a small test for empty / single / multiple types would help prevent regressions in diagnostics formatting.
Co-authored-by: Copilot <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
crates/mq-check/src/infer.rs:407
- The early-exit condition treats
100 * params.len()as a “perfect”/maximum overload score, butType::match_scorecan exceed 100 for structural matches (e.g., arrays add +20; functions sum param+ret scores). This can causeresolve_overloadto break before evaluating a later overload with a higher score, potentially selecting the wrong overload. Consider removing this early break, or computing a true upper bound for the current overload’s maximum possible score (e.g., summingparam.match_score(param)for each param) before early exiting.
None => {
best_match = Some((overload.clone(), total_score));
}
Some((_, best_score)) if total_score > *best_score => {
best_match = Some((overload.clone(), total_score));
}
_ => {}
}
}
}
}
best_match.map(|(ty, _score)| self.instantiate_fresh(&ty))
}
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.