Make ~/, round, ceil, floor, truncate return ints. Remove toInt.

Make ~/, round, ceil, floor, truncate return ints. Remove toInt.

[floitsch, 2013-01-03 18:23:17]

https://codereview.chromium.org/11748016/diff/2001/runtime/lib/double.cc
File runtime/lib/double.cc (right):

https://codereview.chromium.org/11748016/diff/2001/runtime/lib/double.cc#newc...
runtime/lib/double.cc:83: if (isinf(result) || isnan(result)) {
From here on same as 'toInt' on the left (except that I changed the
error-message).

https://codereview.chromium.org/11748016/diff/2001/runtime/lib/double.cc#newc...
runtime/lib/double.cc:167: if (isinf(arg.value()) || isnan(arg.value())) {
same as "toInt" on the left. Only change: error-message.

[srdjan, 2013-01-03 19:08:17]

I am opposed to this change as it is a Dart VM performance killer: switching
between integers and doubles is costly in Dart (yes, in some cases you could
optimize conversions away, but APIs should not rely on that). The user should be
able to stay in double space when doing rounding, and switch to integers only
explicitly.

Operations with double receivers should always return doubles (add, multiply,
round, ...).

[Lasse Reichstein Nielsen, 2013-01-04 10:29:42]

LGTM

https://codereview.chromium.org/11748016/diff/2001/runtime/lib/double.cc
File runtime/lib/double.cc (right):

https://codereview.chromium.org/11748016/diff/2001/runtime/lib/double.cc#newc...
runtime/lib/double.cc:87: Exceptions::ThrowByType(Exceptions::kFormat, args);
Consider using UnsupportedError instead of FormatException.
The method is not supported by the current object (inf or nan) so it fits the
use of UnsupportedError. And it really is an error, not an exception.

https://codereview.chromium.org/11748016/diff/2001/runtime/lib/double.cc#newc...
runtime/lib/double.cc:167: if (isinf(arg.value()) || isnan(arg.value())) {
Repeated code should be abstracted into a helper method, possibly parameterized
by the error message.

https://codereview.chromium.org/11748016/diff/2001/runtime/vm/snapshot_test.dart
File runtime/vm/snapshot_test.dart (right):

https://codereview.chromium.org/11748016/diff/2001/runtime/vm/snapshot_test.d...
runtime/vm/snapshot_test.dart:802: createFromCodePoints([(25 *
Math.random()).truncate() + 97]);
This is idiomatic code where "floor" would be more traditional.

https://codereview.chromium.org/11748016/diff/2001/samples/chat/chat_server_l...
File samples/chat/chat_server_lib.dart (right):

https://codereview.chromium.org/11748016/diff/2001/samples/chat/chat_server_l...
samples/chat/chat_server_lib.dart:646: _bucketTimeRange = (_timeRange /
buckets).truncate();
What happens if _timeRange is negative?
This should probably just be ~/ instead of /.truncate.

https://codereview.chromium.org/11748016/diff/2001/samples/spirodraw/Spirodra...
File samples/spirodraw/Spirodraw.dart (right):

https://codereview.chromium.org/11748016/diff/2001/samples/spirodraw/Spirodra...
samples/spirodraw/Spirodraw.dart:160: int nTurns = (rad / PI2).truncate();
/ -> ~/ ?

https://codereview.chromium.org/11748016/diff/2001/samples/swarm/Views.dart
File samples/swarm/Views.dart (right):

https://codereview.chromium.org/11748016/diff/2001/samples/swarm/Views.dart#n...
samples/swarm/Views.dart:677: return (getOffset(index) /
getPageLength(viewLength)).floor();
~/ ?

https://codereview.chromium.org/11748016/diff/2001/samples/swarm/swarm_ui_lib...
File samples/swarm/swarm_ui_lib/view/MeasureText.dart (right):

https://codereview.chromium.org/11748016/diff/2001/samples/swarm/swarm_ui_lib...
samples/swarm/swarm_ui_lib/view/MeasureText.dart:48: int targetLength =
(lineWidth * maxLines / _typicalCharLength).truncate();
~/

https://codereview.chromium.org/11748016/diff/2001/samples/third_party/dromae...
File samples/third_party/dromaeo/tests/dom-modify-html.dart (right):

https://codereview.chromium.org/11748016/diff/2001/samples/third_party/dromae...
samples/third_party/dromaeo/tests/dom-modify-html.dart:15: str = "$str${new
String.fromCharCodes([((25 * random.nextDouble()) + 97).truncate()])}";
Line length.
You can insert line-breaks inside interpolation-expressions, even in single-line
strings.

https://codereview.chromium.org/11748016/diff/2001/samples/third_party/dromae...
File samples/third_party/dromaeo/tests/dom-modify-htmlidiomatic.dart (right):

https://codereview.chromium.org/11748016/diff/2001/samples/third_party/dromae...
samples/third_party/dromaeo/tests/dom-modify-htmlidiomatic.dart:14: str =
"$str${new String.fromCharCodes([((25 * Math.random()) + 97).truncate()])}";
Line length.

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
File
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart
(right):

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/constant_system_javascript.dart:188:
return (absValue & BITS53) == absValue;
Why not just check (absValue <= BITS53) ?

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/js_number.dart (right):

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_number.dart:53: if (isNaN)
throw new FormatException('NaN');
Consider UnsupportedError.
FormatException is really for when you pass something to a parse-function and
checking the format is just as complicated as actually parsing.
UnsupportedError doesn't fit exactly (it talks about the class not supporting
the operation, not just the value), so perhaps we should change its description.

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_number.dart:54: if (isInfinite)
throw new FormatException('Infinity');
Consider combining the test into one 'Not finite' error. Saves time.

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_number.dart:55: num ceiled =
JS('num', r'Math.ceil(#)', this);
Just do:
  return JS('num', r'Math.ceil(#) + 0', this);

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_number.dart:62: JS('num',
r'Math.floor(#)', this);
This can return -0 if it gets -0 as input. Add the +0 here too.

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_number.dart:66: return this < 0
? ceil() : floor();
Parentheses around (this < 0).

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_number.dart:73: num rounded =
JS('num', r'-Math.round(-#)', this);
Just do 
  return JS('int', r'-Math.round(-#) + 0', this);

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_number.dart:81: || JS('bool',
r'# == -Infinity', this);
Indentation off here.

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/samples/leap/leap_leg.dart (right):

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/samples/leap/leap_leg.dart:79: if
(rect.width.truncate() != 0) {
Thast's silly. Just use:
  rect.width >= 1

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/core/double.dart
File sdk/lib/core/double.dart (right):

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/core/double.dart#n...
sdk/lib/core/double.dart:59: * Returns the integer closest to [this].  Throws a
[FormatException] if
UnsupportedError.

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/core/num.dart
File sdk/lib/core/num.dart (right):

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/core/num.dart#newc...
sdk/lib/core/num.dart:30: * [:(a / b).truncate():].
So add "if a or b is a double. If both are int, this performs truncating integer
division.".

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/core/num.dart#newc...
sdk/lib/core/num.dart:64: * Returns the greatest integer no greater than [this]
Throws a
Missing '.'. Two newlines after the '.'.

https://codereview.chromium.org/11748016/diff/2001/sdk/lib/core/num.dart#newc...
sdk/lib/core/num.dart:70: * Returns the least integer that is no smaller than
[this]. Throws a
Two newlines after '.'.

https://codereview.chromium.org/11748016/diff/2001/tests/corelib/compare_to2_...
File tests/corelib/compare_to2_test.dart (right):

https://codereview.chromium.org/11748016/diff/2001/tests/corelib/compare_to2_...
tests/corelib/compare_to2_test.dart:23: var maxFiniteAsInt =
maxFiniteAsDouble.truncate();
Curious. This is actually a use-case for "toInt" :). You know that the value is
an integer, it's just typed as a double.

https://codereview.chromium.org/11748016/diff/2001/tools/testing/dart/test_pr...
File tools/testing/dart/test_progress.dart (right):

https://codereview.chromium.org/11748016/diff/2001/tools/testing/dart/test_pr...
tools/testing/dart/test_progress.dart:303: var percent = ((_completedTests() /
_foundTests) * 100).truncate().toString();
.truncate().toString() => toStringAsFixed(0) ?

https://codereview.chromium.org/11748016/diff/2001/utils/tests/string_encodin...
File utils/tests/string_encoding/benchmark_runner.dart (right):

https://codereview.chromium.org/11748016/diff/2001/utils/tests/string_encodin...
utils/tests/string_encoding/benchmark_runner.dart:61: ((n * sumXSquared) - (sumX
* sumX))) / source.length).truncate();
Indent by one more here.

https://codereview.chromium.org/11748016/diff/2001/utils/tests/string_encodin...
utils/tests/string_encoding/benchmark_runner.dart:65: ((n * sumXSquared) - (sumX
* sumX))).truncate();
Ditto.

https://codereview.chromium.org/11748016/diff/2001/utils/tests/string_encodin...
utils/tests/string_encoding/benchmark_runner.dart:75: return
((targetDurationNanos - overheadNanos) / perRequestNanos).truncate();
Long line. Use ~/ ?

https://codereview.chromium.org/11748016/diff/2001/utils/tests/string_encodin...
utils/tests/string_encoding/benchmark_runner.dart:82: return ((fullBlocks *
blocksize) + extraReps).truncate();
All operands are int, so drop truncate.

https://codereview.chromium.org/11748016/diff/2001/utils/tests/string_encodin...
utils/tests/string_encoding/benchmark_runner.dart:108:
BlockSample._totalCount(results));
Indent by 1 more, or put on same line if possible.

https://codereview.chromium.org/11748016/diff/2001/utils/tests/string_encodin...
utils/tests/string_encoding/benchmark_runner.dart:188: return
val.truncate().toString();
.toStringAsFixed(0)

[floitsch, 2013-03-11 13:39:15]

https://chromiumcodereview.appspot.com/11748016/diff/2001/samples/spirodraw/S...
File samples/spirodraw/Spirodraw.dart (right):

https://chromiumcodereview.appspot.com/11748016/diff/2001/samples/spirodraw/S...
samples/spirodraw/Spirodraw.dart:160: int nTurns = (rad / PI2).truncate();
On 2013/01/04 10:29:42, Lasse Reichstein Nielsen wrote:
> / -> ~/ ?

done in different CL.

https://chromiumcodereview.appspot.com/11748016/diff/2001/samples/swarm/swarm...
File samples/swarm/swarm_ui_lib/view/MeasureText.dart (right):

https://chromiumcodereview.appspot.com/11748016/diff/2001/samples/swarm/swarm...
samples/swarm/swarm_ui_lib/view/MeasureText.dart:48: int targetLength =
(lineWidth * maxLines / _typicalCharLength).truncate();
On 2013/01/04 10:29:42, Lasse Reichstein Nielsen wrote:
> ~/

Done in a different CL.

https://chromiumcodereview.appspot.com/11748016/diff/2001/samples/swarm/swarm...
File samples/swarm/swarm_ui_lib/view/PagedViews.dart (right):

https://chromiumcodereview.appspot.com/11748016/diff/2001/samples/swarm/swarm...
samples/swarm/swarm_ui_lib/view/PagedViews.dart:249: pageNumber = pageNumber;
Remove line.

https://chromiumcodereview.appspot.com/11748016/diff/2001/utils/tests/string_...
File utils/tests/string_encoding/benchmark_runner.dart (right):

https://chromiumcodereview.appspot.com/11748016/diff/2001/utils/tests/string_...
utils/tests/string_encoding/benchmark_runner.dart:108:
BlockSample._totalCount(results));
On 2013/01/04 10:29:42, Lasse Reichstein Nielsen wrote:
> Indent by 1 more, or put on same line if possible.

Done in a different CL.