- Address warnings about 64-bit to 32-bit conversions.

First round of http://dartbug.com/15922:
- Address warnings about 64-bit to 32-bit conversions.
- Remove heap profiler.

R=asiva@google.com

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

[Søren Gjesse, 2014-01-15 10:38:43]

dbc

https://codereview.chromium.org/139043003/diff/1/runtime/bin/eventhandler.cc
File runtime/bin/eventhandler.cc (right):

https://codereview.chromium.org/139043003/diff/1/runtime/bin/eventhandler.cc#...
runtime/bin/eventhandler.cc:97: handle = Dart_GetField(handle,
DartUtils::NewString(DartUtils::kIdFieldName));
Move this to DartUtils::GetInt64Field.

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.h
File runtime/bin/filter.h (right):

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.h#newcode57
runtime/bin/filter.h:57: ZLibDeflateFilter(bool gzip = false, int32_t level = 6)
Just use int64_t to avoid the static cast in filter.cc.

[siva, 2014-01-16 00:15:59]

lgtm

https://codereview.chromium.org/139043003/diff/1/runtime/bin/eventhandler.cc
File runtime/bin/eventhandler.cc (right):

https://codereview.chromium.org/139043003/diff/1/runtime/bin/eventhandler.cc#...
runtime/bin/eventhandler.cc:105: Dart_PropagateError(handle);
UNREACHABLE here like you have above.

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.cc
File runtime/bin/filter.cc (right):

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.cc#newcode65
runtime/bin/filter.cc:65: (level < kMinInt32) || (level > kMaxInt32)) {
Would be more readable if this code is written as:
Dart_Handle result = Dart_IntegerToInt64(level_obj, &level);
if (Dart_IsError(result) || (level < kMinInt32) || (level > kMaxInt32)) {
...
}

Feels weird when the condition has side effects (setting level in this case)

https://codereview.chromium.org/139043003/diff/1/runtime/lib/typed_data.cc
File runtime/lib/typed_data.cc (right):

https://codereview.chromium.org/139043003/diff/1/runtime/lib/typed_data.cc#ne...
runtime/lib/typed_data.cc:332: ASSERT(host_value.AsInt64Value() <= kMaxInt32);
ASSERT((host_value.AsInt64Value() >= kMinInt32) || (host_value.AsInt64Value() <=
kMaxInt32));

https://codereview.chromium.org/139043003/diff/1/runtime/platform/globals.h
File runtime/platform/globals.h (right):

https://codereview.chromium.org/139043003/diff/1/runtime/platform/globals.h#n...
runtime/platform/globals.h:472: __result = static_cast<intptr_t>(expression);   
                     \
We probably should not have a static cast here to avoid inadvertant usage of
expressions that return non intptr_t values. Without the case we will get a
compilation error.

[Ivan Posva, 2014-01-16 05:05:40]

Thanks!

-Ivan

https://codereview.chromium.org/139043003/diff/1/runtime/bin/eventhandler.cc
File runtime/bin/eventhandler.cc (right):

https://codereview.chromium.org/139043003/diff/1/runtime/bin/eventhandler.cc#...
runtime/bin/eventhandler.cc:97: handle = Dart_GetField(handle,
DartUtils::NewString(DartUtils::kIdFieldName));
On 2014/01/15 10:38:43, Søren Gjesse wrote:
> Move this to DartUtils::GetInt64Field.

I strongly believe that we need to significantly redo DartUtils, so I did not
want to add more to it.

https://codereview.chromium.org/139043003/diff/1/runtime/bin/eventhandler.cc#...
runtime/bin/eventhandler.cc:105: Dart_PropagateError(handle);
On 2014/01/16 00:15:59, siva wrote:
> UNREACHABLE here like you have above.

Done.

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.cc
File runtime/bin/filter.cc (right):

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.cc#newcode65
runtime/bin/filter.cc:65: (level < kMinInt32) || (level > kMaxInt32)) {
On 2014/01/16 00:15:59, siva wrote:
> Would be more readable if this code is written as:
> Dart_Handle result = Dart_IntegerToInt64(level_obj, &level);
> if (Dart_IsError(result) || (level < kMinInt32) || (level > kMaxInt32)) {
> ...
> }
> 
> Feels weird when the condition has side effects (setting level in this case)

Done.

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.h
File runtime/bin/filter.h (right):

https://codereview.chromium.org/139043003/diff/1/runtime/bin/filter.h#newcode57
runtime/bin/filter.h:57: ZLibDeflateFilter(bool gzip = false, int32_t level = 6)
On 2014/01/15 10:38:43, Søren Gjesse wrote:
> Just use int64_t to avoid the static cast in filter.cc.

That would be pushing the issue just downstream. The static cast would still
have to happen in filter.cc just at a different place. The way it is setup now
it allows us to report an error when creating the ZLibDeflateFilter, which is
where we would be passing the bad parameter instead of later when the deflation
is invoked.

https://codereview.chromium.org/139043003/diff/1/runtime/lib/typed_data.cc
File runtime/lib/typed_data.cc (right):

https://codereview.chromium.org/139043003/diff/1/runtime/lib/typed_data.cc#ne...
runtime/lib/typed_data.cc:332: ASSERT(host_value.AsInt64Value() <= kMaxInt32);
On 2014/01/16 00:15:59, siva wrote:
> ASSERT((host_value.AsInt64Value() >= kMinInt32) || (host_value.AsInt64Value()
<=
> kMaxInt32));

Done.

https://codereview.chromium.org/139043003/diff/1/runtime/platform/globals.h
File runtime/platform/globals.h (right):

https://codereview.chromium.org/139043003/diff/1/runtime/platform/globals.h#n...
runtime/platform/globals.h:472: __result = static_cast<intptr_t>(expression);   
                     \
On 2014/01/16 00:15:59, siva wrote:
> We probably should not have a static cast here to avoid inadvertant usage of
> expressions that return non intptr_t values. Without the case we will get a
> compilation error.

You are absolutely right. I did review the current calls to be correct, but by
not casting it we will trigger if we ever go outside of the bounds.

[Ivan Posva, 2014-01-16 05:05:51]

Committed patchset #2 manually as r31867 (presubmit successful).