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

Change machine word constructors to have no default argument. #1938

Merged
merged 1 commit into from
Jun 3, 2017

Conversation

jemc
Copy link
Member

@jemc jemc commented Jun 1, 2017

This is meant to prevent confusion/surprise with respect to other primitives
whose value can be compared using is ${PrimitiveName}. Machine words
are special primitives that can have multiple values, and comparing
with is ${MachineWord} is currently equivalent to comparing to zero.

Incidentally, fixing this also alerts us to the bug in #1920.

We should hold off on merging this PR until after #1927 is merged.

@jemc jemc added changelog - changed Automatically add "Changed" CHANGELOG entry on merge do not merge This PR should not be merged at this time labels Jun 1, 2017
This is meant to prevent confusion with respect to other primitives
whose value can be compared using `is ${PrimitiveName}`. Machine words
are special primitives that can have multiple values, and comparing
with `is ${MachineWord}` is currently equivalent to comparing to zero.

Incidentally, fixing this also alerts us to the bug in #1920.

We should hold off on merging this PR until after #1927 is merged.
@jemc jemc force-pushed the change/machine-word-constructor-no-default-arg branch from f8cb52f to 6a57bfb Compare June 3, 2017 06:28
@jemc jemc removed the do not merge This PR should not be merged at this time label Jun 3, 2017
@jemc
Copy link
Member Author

jemc commented Jun 3, 2017

Rebased after merging #1927. This is ready to merge.

@SeanTAllen SeanTAllen merged commit f922d69 into master Jun 3, 2017
@SeanTAllen SeanTAllen deleted the change/machine-word-constructor-no-default-arg branch June 3, 2017 11:09
ponylang-main added a commit that referenced this pull request Jun 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants