Fix sign error in BigInt::leftShift. #63
Conversation
If y is negative, we need to divide by 2^(-y), i.e., multiply by 2^y.
| @@ -324,7 +322,7 @@ | |||
| <h1>BigInt::signedRightShift (_x_, _y_)</h1> | |||
| <p>The abstract operation BigInt::signedRightShift with arguments _x_ and _y_ of type BigInt:</p> | |||
| <emu-alg> | |||
| 1. Return ? BigInt::leftShift(_x_, -_y_). | |||
| 1. Return BigInt::leftShift(_x_, -_y_). | |||
There was a problem hiding this comment.
Nit: Rather than eliminating the ? , better to replace it with ! because the definition of leftShift may throw for certain types (though we assert it won't). For example, for Number, it will call ToInt32, which could throw from ToNumber. That won't throw, since it's already a Number, however.
There was a problem hiding this comment.
Fine with me, but then the same should be done in the other definitions in 1.1 (that's 5 or 6 changes).
There was a problem hiding this comment.
Never mind, really there's no way that this could possibly throw. I'll take your change here.
| @@ -313,9 +313,7 @@ | |||
| <h1>BigInt::leftShift (_x_, _y_)</h1> | |||
| <p>The abstract operation BigInt::leftShift with two arguments _x_ and _y_ of BigInt:</p> | |||
| <emu-alg> | |||
| 1. If _y_ < 0, | |||
| 1. Return a BigInt representing _x_ divided by 2<sup>_y_</sup>, rounding down to the nearest integer, including for negative numbers. | |||
There was a problem hiding this comment.
I'd actually like to keep the original, more wordy phrasing, since I believe we'll work towards a specification with no mathematical values and instead all numeric values in spec-land being either Numbers or BigInts (see #10). The small fraction in your spec text is not always representable as either. We could fix the issue more minimally by just inserting a - here, right?
There was a problem hiding this comment.
Okay, I need to read this when I have more time, but to answer your question: yes.
If y is negative, we need to divide by 2^(-y), i.e., multiply by 2^y.