Skip to content
This repository has been archived by the owner on Oct 10, 2019. It is now read-only.

Shifting by negative number #96

Closed
binji opened this issue Oct 25, 2017 · 12 comments
Closed

Shifting by negative number #96

binji opened this issue Oct 25, 2017 · 12 comments

Comments

@binji
Copy link

binji commented Oct 25, 2017

The current spec says that shifting by a negative number results in a shift in the opposite direction. This seems pretty strange to me, and appears to conflict with many other languages and for Number. My apologies if this has been discussed before.

For Number, the shift amount is masked with 31 which produces a different result:

> 1 << -3
536870912
> 1n << -3n
0

In python the shift is not allowed:

>>> 1 << -3
ValueError: negative shift count

In PHP the shift is also not allowed:

> echo print(1 << -3);
PHP Warning:  Uncaught ArithmeticError: Bit shift by negative number in php shell code:1
  • C and C++ the result is undefined behavior.
  • In Go, the shift count must be an unsigned number: link
  • In Rust, the shift count also must be unsigned: link
  • In Java, the shift is masked: link
  • GMP uses the mp_bitcnt_t for the shift amount, which is unsigned: link link
@lars-t-hansen
Copy link

The meaning of negative shift probably comes from the Lisp family's arithmetic shift function, see eg http://clhs.lisp.se/Body/f_ash.htm. In that case there was just one shift operation (ASH) but here we have two - left and right - so the weirdness seems significantly less motivated.

I suppose the case could be made that certain computations will result in shift amounts that are naturally negative or positive depending on the shift direction and that user code is simpler as a result by not having to dispatch on that, but I agree it would be nice to see an argument for that.

@littledan
Copy link
Member

I don't have a strong argument for these semantics. It's not driven by any particular use case. I thought they'd be a reasonable generalization of the mathematical ideal of shifting, but if these are confusing for users, we could not provide this behavior. I'd be fine throwing a RangeError on negative shifts. See also #40.

littledan added a commit that referenced this issue Oct 26, 2017
@littledan
Copy link
Member

@binji Does the patch in 7470df1 do what you're imagining? I'm pretty sure we don't want to do masking for shifts on BigInts. This patch throws a RangeError on negative shifts; another possibility is to round to zero (like ToLength). I'm not really sure what the motivation for such a restriction is, though.

littledan added a commit that referenced this issue Oct 26, 2017
@binji
Copy link
Author

binji commented Oct 26, 2017

Yes, that looks reasonable to me, thanks!

@anba
Copy link

anba commented Oct 30, 2017

In Java, the shift is masked: link

I think for Java it makes more sense to compare it to java.math.BigInteger, which actually implements the current semantics. From BigInteger#shiftLeft​:

Returns a BigInteger whose value is (this << n). The shift distance, n, may be negative, in which case this method performs a right shift. (Computes floor(this * 2^n).)

@binji
Copy link
Author

binji commented Oct 30, 2017

Interesting! Thanks for this, @anba. Do you know of any examples where it this behavior is used?

@littledan
Copy link
Member

@binji I don't have any actual use cases in mind, but often in TC39, we sometimes do things out of consistency, if it seems well-defined, sensible for users and without any implementation concerns. Maybe that's not the best practice in all cases, but it's a way to look into this. Could you say anything more about the motivation to put in this restriction? Would it end up being slower in realistic implementations, or confusing for users who would've preferred to eagerly get the exception?

@binji
Copy link
Author

binji commented Oct 30, 2017

Could you say anything more about the motivation to put in this restriction?

The motivation for the restriction is the examples I listed above -- it's inconsistent with many languages' behavior, including JavaScript's behavior with Number.

Since it is an uncommonly provided feature, it is likely not meant to be used by the programmer, and perhaps not even known to exist. As a result, it could easily hide bugs that would have been caught earlier.

That's why I'd be curious to see use cases, since the bit-shifting algorithms I can think of off the top of my head often use fixed shift amounts (where a negative shift is pointless), and always use fixed shift directions.

Would it end up being slower in realistic implementations...

It might be marginally slower because it requires an additional check to determine whether the shift amount is negative and dispatch accordingly. The cost of the the actual shift will be more significant, so I don't think this is a big deal.

@jakobkummerow
Copy link
Collaborator

Would it end up being slower in realistic implementations

It's safe to assume virtually identical performance: implementations have to check for negative shift amount anyway, and then either throw or flip the shift direction.

@binji
Copy link
Author

binji commented Oct 31, 2017

Oh right. :-)

We can say that whatever you choose as the default will be slightly faster, since in either case if you want the other behavior you'll have to perform an additional check to throw or flip manually. Again, the difference is probably negligible.

@anba
Copy link

anba commented Oct 31, 2017

Do you know of any examples where it this behavior is used?

Except for one recent case where I was able to simplify some branching code to directly call shiftLeft, I don't know of any examples where it's useful to have this behaviour. But then again I don't work much with BigInteger in Java, so I probably can't give useful insights here. 😄
And the OpenJDK source code also doesn't give any hints why it's implemented this way in Java.

@littledan
Copy link
Member

littledan commented Dec 6, 2017

I raised this issue at TC39; with @anba's use case, it didn't seem like the committee had much interest in revisiting the semantics.

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

No branches or pull requests

5 participants