Skip to content
This repository has been archived by the owner on Feb 19, 2018. It is now read-only.

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

Closed
Inve1951 opened this issue Sep 18, 2016 · 20 comments
Closed

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

Inve1951 opened this issue Sep 18, 2016 · 20 comments

Comments

@Inve1951
Copy link

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. 👍

@GeoffreyBooth
Copy link
Collaborator

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

@Inve1951
Copy link
Author

Inve1951 commented Sep 18, 2016

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?

@GeoffreyBooth
Copy link
Collaborator

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

In #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?

@Inve1951
Copy link
Author

// 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.

@GeoffreyBooth
Copy link
Collaborator

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 😄

@lydell
Copy link

lydell commented Sep 18, 2016

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 jashkenas/coffeescript#2887 (and the issues linked to from there) to see if this has been discussed before.

@Inve1951
Copy link
Author

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

@lydell
Copy link

lydell commented Sep 18, 2016

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

@alangpierce
Copy link

alangpierce commented Sep 18, 2016

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.

@Inve1951
Copy link
Author

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.

@Inve1951
Copy link
Author

Inve1951 commented Sep 18, 2016

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...

@GeoffreyBooth
Copy link
Collaborator

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?

@Inve1951
Copy link
Author

Inve1951 commented Sep 19, 2016

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 😄

@Inve1951 Inve1951 changed the title integer division operator // integer cast/truncation operator(s) Sep 19, 2016
@DomVinyard
Copy link

DomVinyard commented Sep 21, 2016

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.

@Inve1951
Copy link
Author

Inve1951 commented Sep 23, 2016

@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.

@GeoffreyBooth
Copy link
Collaborator

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.

@Inve1951
Copy link
Author

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

@GeoffreyBooth
Copy link
Collaborator

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

@Inve1951
Copy link
Author

yes

GeoffreyBooth added a commit to GeoffreyBooth/coffeescript that referenced this issue Dec 11, 2017
GeoffreyBooth added a commit to jashkenas/coffeescript that referenced this issue Dec 11, 2017
* Changelog for 2.1.0; remove text from objects section that is no longer valid for CS2/ES2015.

* Update packages

* 2.1.0 build

* Update output

* Correct reference to `//` division, per coffeescript6/discuss#39 (comment)
@coffeescriptbot coffeescriptbot changed the title integer cast/truncation operator(s) CS2 Discussion: Features: integer cast/truncation operator(s) Feb 19, 2018
@coffeescriptbot
Copy link
Collaborator

Migrated to jashkenas/coffeescript#4935

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants