Error message for missing parentheses on static dispatch call#9245
Error message for missing parentheses on static dispatch call#9245fpsvogel wants to merge 5 commits intoroc-lang:mainfrom
Conversation
jaredramirez
left a comment
There was a problem hiding this comment.
Overall this approach looks good! Just have one note about performance that needs to be figured out, then this is good to merge!
| const is_method = try self.isMethodOnNominalType(receiver_var, dot_access.field_name); | ||
|
|
||
| // Then, unify the actual receiver type with the expected record | ||
| _ = try self.unifyInContext(record_being_accessed, receiver_var, env, .{ .record_access = .{ | ||
| .field_name = dot_access.field_name, | ||
| .field_region = dot_access.field_name_region, | ||
| .is_method = is_method, | ||
| } }); |
There was a problem hiding this comment.
This approach makes sense to me, but it would be ideal to only pay for checking self.isMethodOnNominalType if the unification fails. Running the check for all record fields could get expensive.
Currently, self.unifyInContext is not set up for that, but I think it's worth exploring if there's a nice way to do set the context on error or something!
There was a problem hiding this comment.
|
Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked |
Resolves #9193
Improves the error message when a static dispatch call with zero arguments is missing empty parentheses.
It may make the most sense to review this PR one commit at a time: two tweaks to the error message are in separate commits, because they involve nontrivial logic that I didn't want to lump into the principal changes in the first commit. These are the two tweaks:
[1, 2, 3]but.len).Use `len`() insteadbutUse `len()` instead).The new error message:
Source for the above example:
The current error message, before these changes: