Implement BigintOperations::ToDecimalCString.

Implement BigintOperations::ToDecimalCString.

Committed: https://code.google.com/p/dart/source/detail?r=5068

[siva, 2012-03-02 19:31:45]

lgtm

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
File runtime/vm/bigint_operations.cc (right):

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:342: // We guard against overflows by using 64
bit computations.
On 64 bit machines the signed_length is actually a 64 bit quantity, should you
restrict that to an int32_t value so that we are sure the 64 bit computation
does not result in overflows on a 64 bit machine?

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:354: if (required_size !=
static_cast<intptr_t>(required_size)) {
I think we should assert here instead of silently returning "".

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:359:
reinterpret_cast<char*>(allocator(static_cast<intptr_t>(required_size)));
ASSERT(result != NULL);

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:381: result[result_pos++] = '0' + part % 10;
'0' + (part % 10);
to make it clear for operator precedence challenged folks.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:387: // Move the resulting position back until
we don't have any zeroes anymore.
// This is done so that we can remove all leading zeros.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:403: }
ASSERt(result_pos >= 0);

[floitsch, 2012-03-05 14:28:15]

PTAL.
I'm now testing beforehand if there can be an overflow. Since this should never
happen I don't have a test for it. So please be extra careful when reviewing
that code.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
File runtime/vm/bigint_operations.cc (right):

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:342: // We guard against overflows by using 64
bit computations.
On 2012/03/02 19:31:45, asiva wrote:
> On 64 bit machines the signed_length is actually a 64 bit quantity, should you
> restrict that to an int32_t value so that we are sure the 64 bit computation
> does not result in overflows on a 64 bit machine?

Refactored.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:354: if (required_size !=
static_cast<intptr_t>(required_size)) {
On 2012/03/02 19:31:45, asiva wrote:
> I think we should assert here instead of silently returning "".

That's why there was a TODO. Now throwing an OOM-exception.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:359:
reinterpret_cast<char*>(allocator(static_cast<intptr_t>(required_size)));
On 2012/03/02 19:31:45, asiva wrote:
> ASSERT(result != NULL);

Done.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:381: result[result_pos++] = '0' + part % 10;
On 2012/03/02 19:31:45, asiva wrote:
> '0' + (part % 10);
> to make it clear for operator precedence challenged folks.

Done.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:387: // Move the resulting position back until
we don't have any zeroes anymore.
On 2012/03/02 19:31:45, asiva wrote:
> // This is done so that we can remove all leading zeros.

Done.

https://chromiumcodereview.appspot.com/9536016/diff/2001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:403: }
On 2012/03/02 19:31:45, asiva wrote:
> ASSERt(result_pos >= 0);

Done.

[siva, 2012-03-07 01:05:04]

lgtm

https://chromiumcodereview.appspot.com/9536016/diff/7001/runtime/vm/bigint_op...
File runtime/vm/bigint_operations.cc (right):

https://chromiumcodereview.appspot.com/9536016/diff/7001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:363: int64_t bit_length = length *
kDigitBitSize;
ASSERT(bit_length > length);

https://chromiumcodereview.appspot.com/9536016/diff/7001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:385: const Bigint& divisor =
Bigint::Handle(NewFromSmi(divisor_smi));
why not just do a
const Bigint& divisor = Bigint::Handle(NewFromInt64(kDivisorValue));

[floitsch, 2012-03-07 08:25:48]

https://chromiumcodereview.appspot.com/9536016/diff/7001/runtime/vm/bigint_op...
File runtime/vm/bigint_operations.cc (right):

https://chromiumcodereview.appspot.com/9536016/diff/7001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:363: int64_t bit_length = length *
kDigitBitSize;
On 2012/03/07 01:05:04, asiva wrote:
> ASSERT(bit_length > length);

Done.

https://chromiumcodereview.appspot.com/9536016/diff/7001/runtime/vm/bigint_op...
runtime/vm/bigint_operations.cc:385: const Bigint& divisor =
Bigint::Handle(NewFromSmi(divisor_smi));
On 2012/03/07 01:05:04, asiva wrote:
> why not just do a
> const Bigint& divisor = Bigint::Handle(NewFromInt64(kDivisorValue));

Done.

[sra1, 2012-04-02 20:03:29]

https://chromiumcodereview.appspot.com/9536016/diff/13001/runtime/vm/bigint_o...
File runtime/vm/bigint_operations.cc (right):

https://chromiumcodereview.appspot.com/9536016/diff/13001/runtime/vm/bigint_o...
runtime/vm/bigint_operations.cc:381: const int kPowerOfTen = 8;
It is funny that the constant called power-of-ten is not a power of 10. 
Compare: const int kTwo = 3;

I would call these kChunkDivisor and kChunkDigits

[floitsch, 2012-04-03 11:20:52]

https://chromiumcodereview.appspot.com/9536016/diff/13001/runtime/vm/bigint_o...
File runtime/vm/bigint_operations.cc (right):

https://chromiumcodereview.appspot.com/9536016/diff/13001/runtime/vm/bigint_o...
runtime/vm/bigint_operations.cc:381: const int kPowerOfTen = 8;
On 2012/04/02 20:03:29, sra1 wrote:
> It is funny that the constant called power-of-ten is not a power of 10. 
> Compare: const int kTwo = 3;
> 
> I would call these kChunkDivisor and kChunkDigits

done in https://chromiumcodereview.appspot.com/9959095/