Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

fixes #3363: modulo operator evaluation order

  • Loading branch information...
commit a2c0106b3f2e0edce7de1a648d80b38ec4f44d51 1 parent 4dfc75d
@michaelficarra michaelficarra authored
View
2  lib/coffee-script/nodes.js
@@ -3075,7 +3075,7 @@
return "[].indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; }";
},
modulo: function() {
- return "function(a, b) { b = +b; return (a % b + b) % b; }";
+ return "function(a, b) { return (+a % (b = +b) + b) % b; }";
},
hasProp: function() {
return '{}.hasOwnProperty';
View
2  src/nodes.coffee
@@ -2194,7 +2194,7 @@ UTILITIES =
"
modulo: -> """
- function(a, b) { b = +b; return (a % b + b) % b; }
+ function(a, b) { return (+a % (b = +b) + b) % b; }
@xixixao
xixixao added a note

The +a is not redundant anymore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
"""
# Shortcuts to speed up the lookup time for native functions.
View
7 test/operators.coffee
@@ -363,3 +363,10 @@ test "#3361: Modulo operator coerces right operand once", ->
res = 42 %% valueOf: -> count += 1
eq 1, count
eq 0, res
+
+test "#3363: Modulo operator coercing order", ->
+ count = 2
+ a = valueOf: -> count *= 2
+ b = valueOf: -> count += 1
+ eq 4, a %% b
+ eq 5, count

3 comments on commit a2c0106

@davidchambers

Nice fix. The test is clever, though the following would be more explicit:

log = ''
a = valueOf: -> log += 'a'; 42
b = valueOf: -> log += 'b'; 13
eq 3, a %% b
eq 'ab', log
@michaelficarra
Collaborator

Sorry, I don't know if I find that any more clear/explicit, especially not significantly enough to make a change.

@davidchambers

It's fine as is. There's no harm in making the reader do some arithmetic. :)

Please sign in to comment.
Something went wrong with that request. Please try again.