Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify wording for some subtyping errors #3933

Merged
merged 8 commits into from
Dec 16, 2021

Conversation

jasoncarr0
Copy link
Contributor

@jasoncarr0 jasoncarr0 commented Dec 14, 2021

A few subtyping errors were technically incorrect, or unnecessarily obtuse. This improves the wording to better clarify the meaning of these.

Fixes #3929 and #3928

Jason Carr added 6 commits December 14, 2021 00:35
While the usage of subtyping is technically correct, this makes it
clearer why the unaliasing appears (and aliasing in the other
formulation), we need need to assign it to a new name for the parameter.
@jasoncarr0
Copy link
Contributor Author

Fixes #3929

This also applies a light bandage to #3928, while we very much should indicate which variables to consume and test that we can consume, updating the message helps indicate that consuming is even an option one should try.

@SeanTAllen
Copy link
Member

Windows failure can be ignored. It's a CI error that is known and being looked into.

@SeanTAllen
Copy link
Member

@jasoncarr0 is this ready other than the request for some release notes?

@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Dec 14, 2021
@ponylang-main
Copy link
Contributor

Hi @jasoncarr0,

The changelog - fixed label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 3933.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

@SeanTAllen
Copy link
Member

This addresses my intent with #3928, @jasoncarr0 during sync noted why he didn't think this was a fix. He had an idea for a deeper change where it would point out the variable that needs to be consumed. We discussed and he will be opening an issue for that.

Note to self: when squashing this to merge, make sure Fixes #3929 and #3928 is part of the comment.

@jasoncarr0
Copy link
Contributor Author

@SeanTAllen Is this good to merge? Or is there anything else I missed

@SeanTAllen
Copy link
Member

@jasoncarr0 it's fine. in the future you can do one line per paragraph in the release notes.

@SeanTAllen SeanTAllen merged commit 6e5d702 into main Dec 16, 2021
@SeanTAllen SeanTAllen deleted the subtyping-error-wording-changes branch December 16, 2021 15:18
github-actions bot pushed a commit that referenced this pull request Dec 16, 2021
github-actions bot pushed a commit that referenced this pull request Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiler prints different reference capability on error: iso vs iso^
3 participants