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

CS2 Discussion: Features: integer cast/truncation operator(s) #4935

Closed
coffeescriptbot opened this issue Feb 19, 2018 · 19 comments
Closed

CS2 Discussion: Features: integer cast/truncation operator(s) #4935

coffeescriptbot opened this issue Feb 19, 2018 · 19 comments

Comments

@coffeescriptbot
Copy link
Collaborator

From @Inve1951 on 2016-09-18 11:44

I'm following your progress in this project and want to throw in another topic:

Using Math.floor() on negative numbers gives non-straight-forward results.

For example right now -5 // 3 compiles to Math.floor(-5 / 3) which results in -2 when -1 would be the result of proper "integer division".

I came across this stackoverflow question which displays some nice alterantives which even seem to perform better.

Keep up that good work of yours. 👍

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-18 15:27

@Inve1951 what would be your preference for what // should compile to?

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2016-09-18 17:33

I think the most elegant version is compiling a // b to (a / b)|0.

It seems ES6 got Math.trunc() but even with all major browsers supporting it I'd rather have (a / b)|0.

Switching //'s behavior is still a breaking change.

@GeoffreyBooth what are your thoughts on this topic?

@coffeescriptbot
Copy link
Collaborator Author

coffeescriptbot commented Feb 19, 2018

From @GeoffreyBooth on 2016-09-18 18:12

I’ve never used that operator so I have no thoughts 😄

In coffeescript6/discuss#36 we’re discussing a breaking-change version of CoffeeScript that outputs ESNext, so Math.trunc would be an option. Can you explain the pros and cons of that versus (a / b)|0?

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2016-09-18 18:36

// is to be considered broken as is.

(a / b)|0 is a really slick fix and should definitely go into 1.11.0

What variant you put in 2.0.0 is fully up to you guys.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-18 18:46

This sounds like a bugfix for //. Do you mind opening an issue (or even better, a pull request) on the main coffeescript repo?

You’ll need to explain what exactly (a / b)|0 does, and why it’s better than the current implementation. I’m not sure it’s a breaking change if the current behavior is incorrect, but you’ll need to explain why please 😄

@coffeescriptbot
Copy link
Collaborator Author

From @lydell on 2016-09-18 18:50

I have used // a few times, but never with negative numbers, so I’ve never really thought about how that should work.

Just as a note: Python seems to do the same thing as CoffeeScript here:

$ python3 -c 'print(-5 // 3)'
-2

Might be worth reading through #2887 (and the issues linked to from there) to see if this has been discussed before.

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2016-09-18 18:57

Just ran a speed test on both in chrome:

var t, i, dump;

t = Date.now();
for(i=0;i<10000000;++i) {
    dump = Math.trunc(Math.sqrt(i));
}
console.log( "Math.trunc: " + (Date.now()-t) );

t = Date.now();
for(i=0;i<10000000;++i) {
    dump = (Math.sqrt(i)|0);
}
console.log( "|0: " + (Date.now()-t) );

Output:

Math.trunc: 536
|0: 393

@coffeescriptbot
Copy link
Collaborator Author

From @lydell on 2016-09-18 19:09

I don’t think it’s worth discussing performance before the semantics are worked out.

@coffeescriptbot
Copy link
Collaborator Author

From @alangpierce on 2016-09-18 19:12

I don't think this is obviously a bug. Floor division and truncation division are both valid operations, and different languages make different decisions about what to use by default.

In Python, Guido intentionally chose the floor behavior. Here's a post explaining why:
http://python-history.blogspot.com/2010/08/why-pythons-integer-division-floors.html

Floor division and mod that returns a non-negative value (%% in CoffeeScript) are mathematically nicer, and I generally prefer them. I've been bitten a few times in C++ because of code breaking when given negative numbers, which wouldn't have happened with the more mathematically pure operators.

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2016-09-18 19:12

Sorta coming from C there which makes me expect dividing integers simply not resulting in floating point numbers. The float result (if there were one) is truncated at the decimal point (towards zero).
Same thing happens when casting floats to ints.

Python seems to do the same thing as CoffeeScript

I'd also blame python for that behavior.

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2016-09-18 19:49

As mentioned in a comment from @alangpierce's link this doesn't work: -5//3 * -1 == -1 * -5//3 the way // is implemented.

But I see now that there is a different way to think of "integer division".
My straight-forward interpretation was that it treats the expression like C does when doing a division with integers since CS's normal division operator / treats the operands as floats no matter what.

I still find the implemented behavior really odd.
If I'd want the result to be rounded down I'd intentionally use Math.floor()

Edit:
Just noticed that // is called the "floor division operator" in it's PR but is called the "integer division operator" at coffeescript.org.
With it's current implementation, calling it "floor division" in the docs would make it less missleading.

To me it seems that both implementations are reasonable and I'm clearly in favor of truncation towards zero since there is no int cast in js. (obviously one could just write a function _int = (x) -> x | 0 but one could also just use Math.floor() as desired)

Since it's not a bug please treat this as a proposal for changing the implementation. That's quite brazen, I know...

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-18 22:14

I think renaming it to ”floor division operator” in the docs is reasonable.

I’m less convinced that we should change the behavior of an operator when the current implementation is reasonable and might be preferred by people. Perhaps rather than changing //, a new operator should be created?

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2016-09-19 17:42

I did some thinking and some experimenting and came up with the following:

New operators: |/, |* and |
a |/ b becomes (a / b |0)
a |* b becomes (a * b |0)
And | used as a prefix makes |a * b become (a|0) * b and a = |a become a = (a|0) which is equivalent to a |= 0.

I wouldn't add |+ and |- since a |+ b is valid JS (meaning a | (+b)) and can be reproduced with |(a + b). It could however be implemented with a whitespace rule (described further down) and writing code like a |- b(c) could be considered bad practice.

Parentheses ( ) are required because | is very weak in the operator precedence order (MSDN, MDN).

What actually happens here is that before any binary operator (like our Bitwise OR |) is applied it's operands are converted/cast/truncated into 32-bit integers and a|0 just returns that 32-bit integer version of a without altering it. (MDN)
This means that the same result could also be achieved with other binary operations which don't alter their input. (e.g. a & -1)

I do see one drawback though:
a |b + c is valid CS (and JS) and means a | (b + c).
This could be resolved through requiring the prefix usage of | to not have whitespace towards it's operand and requiring the Binary OR syntax to have whitespace.

So above example a |b + c would compile to a((b|0) + c) whereas a | b + c would output as is.
This feels very simmilar to using @ where @somevar is totally different from @ somevar.
It's still breaking existing a |b + c and thus is a breaking change.

One could argue that an int-cast operator is dispensable since you can just write (a|0) yourself. I do deem it lacking though.

Also one could argue that the int-cast operator is all you need and the other operators are dispensable since |(a / b), |(a * b), |(a - b), or even (a * b |0) can also be hand-typed.

And of course the operator could look different to those described here in order to not bring a breaking change along (e.g. °a, a /° b, a *´ b, ).

Furthermore if those are implemented one could go crazy and demand versions of those which do the int-cast on the operands before calculation but that's just nonsense IMO.

I hope this sounds better to you guys 😄

@coffeescriptbot
Copy link
Collaborator Author

From @DomVinyard on 2016-09-21 16:08

a |/ b becomes (a / b | 0)

But why? to save a couple of keystrokes every so often? There's nothing coffeescript about it, jash would kill these suggestions without mercy.

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2016-09-23 23:24

@DomVinyard, thanks for your input.

a |/ b to save a couple of keystrokes every so often?

I had that exact same thought, which is what brought me to :

that the int-cast operator is all you need and the other operators are dispensable

I still do think that there should be the int-cast operator because there is no integer primitive in JS (and no float-to-int cast) and there's situations where you need ints.

a = |b -> a = ( b |0);
a |b -> a( b |0);
a = |(b * c) -> a = ( b * c |0)
and maybe even a =| b / c -> a = ( b / c |0);

It's reasonable to have that operator:

  • Not having to mind Math.floor a and Math.ceil a wheter a is negative.
  • (a | 0) feels kinda hacky and isn't intuitive at all. I even doubt many know about it.
  • Math.trunc a is ES6+ only and isn't currently supported on mobile devices.
  • parseInt "#{a}" is a twofold detour.
  • Giving CS another plus over JS.

@lydell @GeoffreyBooth
I found another usage case for said operator: (0| "-013.345") === -13, (0| "0x13") === 19
On strings it produces the same result as parseInt() would.
However negative hex representation is not supported (0| "-0x13") === 0.

For clarification:
In my previous post I described a whitespace rule which would be neccessary if the pipe character | or another already used operator gets another purpose with this int-cast.
I found FAQ#Grammar and odf's comment on #1036 which perfectly mirror the proposed behaviour.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2017-11-26 02:14

Closing as the consensus above on the original proposal, which suggested redefining the // operator, was “no change.” Any other suggestions should get their own issues.

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2017-11-27 10:44

docs still say // performs integer division when it probably should say floor division

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2017-11-27 16:11

Is it as simple as find and replace one phrase for the other?

@coffeescriptbot
Copy link
Collaborator Author

From @Inve1951 on 2017-11-30 11:17

yes

@coffeescriptbot coffeescriptbot changed the title integer cast/truncation operator(s) CS2 Discussion: Features: integer cast/truncation operator(s) Feb 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant