Make int.gcd accept zero operands.

Make int.gcd accept zero as an operand, including both operands being zero.

R=floitsch@google.com, regis@google.com, sgjesse@google.com

Committed: https://github.com/dart-lang/sdk/commit/3c976019cc84f01fa03974bf628bb4f9b3f2e418

[Lasse Reichstein Nielsen, 2015-06-24 08:51:59]

lrn@google.com changed reviewers:
+ floitsch@google.com, regis@google.com, sgjesse@google.com

[Lasse Reichstein Nielsen, 2015-06-24 08:51:59]

lrn@google.com changed required reviewers:
+ sgjesse@google.com

[Lasse Reichstein Nielsen, 2015-06-24 08:52:00]

There is no reason to restrict gcd from working when only one of the operands is
zero.

[Søren Gjesse, 2015-06-24 08:58:37]

lgtm

[floitsch, 2015-06-24 12:18:13]

LGTM.

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart
File runtime/lib/bigint.dart (right):

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart#new...
runtime/lib/bigint.dart:1563: "at least one operand must not be zero");
Wolfram Alpha goes as far as to say that gcd(0, 0) => 0

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart#new...
runtime/lib/bigint.dart:1806: // Returns gcd of abs(this) and abs(other), with
this != 0 and other !=0.
comment is not correct anymore.

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart#new...
runtime/lib/bigint.dart:1811: if (other == 0) {
Are we sure that bigints cannot be 0?
(If yes assert it, and add comment).

[Lasse Reichstein Nielsen, 2015-06-24 12:44:10]

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart
File runtime/lib/bigint.dart (right):

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart#new...
runtime/lib/bigint.dart:1563: "at least one operand must not be zero");
It's a usable definition, as long as you can explain to the user that 0 is the
greatest divisor of all (by being the top element in the lattice of integers
ordered by divisibility, and gcd being the meet operation on that lattice).
Well, or we can just say it's a special case :)

On the other hand, it does avoid throwing a not very helpful error message.

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart#new...
runtime/lib/bigint.dart:1806: // Returns gcd of abs(this) and abs(other), with
this != 0 and other !=0.
Good catch.

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart#new...
runtime/lib/bigint.dart:1811: if (other == 0) {
They can be 0, at least internally. The `other._toBigInt()` call below would be
used for 0 before this change. 
I don't think an externally visible BigInt will be 0. The call to _toValidInt()
above is there to prevent that.

[regis, 2015-06-24 16:07:16]

lgtm

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart
File runtime/lib/bigint.dart (right):

https://codereview.chromium.org/1211473002/diff/1/runtime/lib/bigint.dart#new...
runtime/lib/bigint.dart:1811: if (other == 0) {
On 2015/06/24 12:44:10, Lasse Reichstein Nielsen wrote:
> They can be 0, at least internally. The `other._toBigInt()` call below would
be
> used for 0 before this change. 
> I don't think an externally visible BigInt will be 0. The call to
_toValidInt()
> above is there to prevent that.

Correct. At this point, 'other' has not yet been forced to Bigint (either here
or in the forwarding gcd of integers.dart), and 0 is still Smi. We would assert
fault when using '== 0' otherwise.

[Lasse Reichstein Nielsen, 2015-07-06 10:23:31]

Changed to accept two zeros, PTAL

[Lasse Reichstein Nielsen, 2015-07-06 10:23:32]

Changed to accept two zeros, PTAL

[Søren Gjesse, 2015-07-06 11:25:07]

lgtm

[regis, 2015-07-06 15:08:12]

lgtm after removing 2 lines.

https://codereview.chromium.org/1211473002/diff/40001/runtime/lib/integers.dart
File runtime/lib/integers.dart (right):

https://codereview.chromium.org/1211473002/diff/40001/runtime/lib/integers.da...
runtime/lib/integers.dart:399: if (other == 0) return this.abs();
Remove these 2 lines, since you check for zero below.

[Lasse Reichstein Nielsen, 2015-07-07 07:51:19]

Committed patchset #4 (id:60001) manually as
3c976019cc84f01fa03974bf628bb4f9b3f2e418 (presubmit successful).