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

Fix sign error in BigInt::leftShift. #63

Closed
wants to merge 1 commit into from
Closed

Fix sign error in BigInt::leftShift. #63

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,7 @@ <h1>BigInt::subtract (_x_, _y_)</h1>
<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_ &lt; 0,
1. Return a BigInt representing _x_ divided by 2<sup>_y_</sup>, rounding down to the nearest integer, including for negative numbers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I need to read this when I have more time, but to answer your question: yes.

1. Return a BigInt representing _x_ multiplied by 2<sup>_y_</sup>.
1. Return a BigInt representing _x_ multiplied by 2<sup>_y_</sup>, rounding down to the nearest integer, including for negative numbers.
</emu-alg>
<emu-note>Semantics here should be equivalent to a bitwise shift, treating the BigInt as an infinite length string of binary two's complement digits.</emu-note>
</emu-clause>
Expand All @@ -324,7 +322,7 @@ <h1>BigInt::leftShift (_x_, _y_)</h1>
<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_).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me, but then the same should be done in the other definitions in 1.1 (that's 5 or 6 changes).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, really there's no way that this could possibly throw. I'll take your change here.

</emu-alg>
</emu-clause>

Expand Down