Skip to content

Fix issue #288 #292

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.

Sign up for GitHub

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

Merged
heyLu merged 6 commits into from
Apr 22, 2015
Merged

Fix issue #288 #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abe9791
Make remainder return Integer if both arguments are integers
Apr 20, 2015
e53412c
Add note for Mac OS X building
Apr 20, 2015
a07f9e3
Wrap _quot in a check whether its arguments are not both ints
Apr 20, 2015
7423184
Added tests to confirm changes
Apr 21, 2015
8871f30
Change test to remove stringification
Apr 21, 2015
103d367
Fix to not use stringification for rem tests
Apr 22, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Some planned and implemented features:
make build_with_jit
./pixie-vm

Note: Mac OS X does not come with the build tools required by default. Install the XCode Command Line tools ([Apple Developer Site](http://developer.apple.com)) or install them independently.


## Running the tests

Expand Down
5 changes: 3 additions & 2 deletions pixie/vm/numbers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ def define_num_ops():
if op == "_div" and c1 == Integer and c2 == Integer:
continue
extend_num_op(op, c1, c2, conv1, sym, conv2)
extend_num_op("_quot", c1, c2, conv1, "/", conv2, wrap_start = "rt.wrap(math.floor(", wrap_end = "))")
extend_num_op("_rem", c1, c2, conv1, ",", conv2, wrap_start = "rt.wrap(math.fmod(", wrap_end = "))")
if c1 != Integer or c2 != Integer:
Copy link
Copy Markdown
Member

@heyLu heyLu Apr 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be an and, not an or, otherwise some operations are not defined.

I think _quot could be included in this if-statement as well, because it also has a special case defined above.

Copy link
Copy Markdown
Author

@MatthewWest MatthewWest Apr 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually doesn't though. See, it's checking for whether either is not an integer. That is functionally identical to checking whether it is not the case that both are integers. Unless there's something else I'm not thinking of.

Copy link
Copy Markdown
Member

@heyLu heyLu Apr 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, sorry.

Can you include _quot as well, though?

Copy link
Copy Markdown
Author

@MatthewWest MatthewWest Apr 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

extend_num_op("_rem", c1, c2, conv1, ",", conv2, wrap_start = "rt.wrap(math.fmod(", wrap_end = "))")
extend_num_op("_quot", c1, c2, conv1, "/", conv2, wrap_start = "rt.wrap(math.floor(", wrap_end = "))")
for (op, sym) in [("_num_eq", "=="), ("_lt", "<"), ("_gt", ">"), ("_lte", "<="), ("_gte", ">=")]:
extend_num_op(op, c1, c2, conv1, sym, conv2,
wrap_start = "true if ", wrap_end = " else false")
Expand Down
13 changes: 13 additions & 0 deletions tests/pixie/tests/test-numbers.pxi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,16 @@
(t/deftest test-float
(doseq [[x f] [[1 1.0] [3 3.0] [3.333 3.333] [3/2 1.5] [1/7 (/ 1.0 7.0)]]]
(t/assert= (float x) f)))

(t/deftest rem-types
(t/assert= Integer (type (rem 5 3)))
(t/assert= Float (type (rem 5.0 3)))
(t/assert= Ratio (type (rem 7/2 3)))
(t/assert= Float (type (rem 7/2 3.0))))

(t/deftest quot-types
(t/assert= Integer (type (quot 5 3)))
(t/assert= Float (type (quot 5.0 3)))
(t/assert= Integer (type (quot 7/2 3/7)))
(t/assert= Float (type (quot 7/2 3.0))))