Make implicit float -> int/long conversions explicit....

Remove code doing a no-op due to float -> int rounding.
Make implicit float -> int/long conversions explicit.

(Implicit float -> int conversions can be found by compiling with 
-Wconversion in gcc [versions 4.1.1 and 4.2.4, and surely many others].)

BUG=none
TEST=app_unittests & base_unittests

[Jacob Mandelson, 2009-09-11 05:24:08]

(Split off from CL 171028)

Ran tests on Linux. Passed except for ConditionVariableTest.LargeFastTaskTest,
which also fails without patch.

[Paweł Hajdan Jr., 2009-09-11 15:50:42]

Do we really want to patch DTOA code? More patches there = potentially more
maintenance problems. I'd rather *not* patch this code.

Jacob, could you give more rationale in the commit message? Is this patch fixing
some compile warnings for you? For which compiler?

[Jacob Mandelson, 2009-09-11 16:12:37]

The dtoa changes are because of compiler warnings, yes.
There's the use of a character literal (which has type char in C++, but
int in C) as an array subscript, which triggers a warning in gcc controlled
by -Wchar-subscripts.
There are implicit conversions from floating point to integeral types,
which triggers a warning in gcc controlled by -Wconversion.

[Paweł Hajdan Jr., 2009-09-11 16:19:03]

I think we should then *disable* the warnings for DTOA code (see a relevant .gyp
file). Fixes in other places look OK.

[Jacob Mandelson, 2009-09-11 16:25:34]

All right, dtoa changes removed from the patchset.

[Paweł Hajdan Jr., 2009-09-11 16:29:20]

Code changes LGTM, let's wait for Mark's comments too.

Please put the rationale from your previous comments into the commit message, as
well as the GCC version which complains about these.

[Mark Mentovai, 2009-09-11 16:35:15]

LG

http://codereview.chromium.org/201091/diff/4001/5009
File chrome/browser/gtk/bookmark_bar_gtk.cc (right):

http://codereview.chromium.org/201091/diff/4001/5009#newcode483
Line 483: gint height = static_cast<gint>(animation->GetCurrentValue() *
More readable?

  gint height =
      static_cast<gint>(animaton->GetCurrentValue() *
                        (kBookmarkBarHeight - kBookmarkBarMinimumHeight)) +
      kBookmarkBarMinimumHeight;

http://codereview.chromium.org/201091/diff/4001/5006
File chrome/browser/gtk/bookmark_manager_gtk.cc (right):

http://codereview.chromium.org/201091/diff/4001/5006#newcode1188
Line 1188: static_cast<gint>(event->y), &path,
Bump &path to the next line.  It can go with the NULLs.

http://codereview.chromium.org/201091/diff/4001/5012
File chrome/browser/gtk/tabs/tab_gtk.cc (right):

http://codereview.chromium.org/201091/diff/4001/5012#newcode234
Line 234: if (dragging)
This conditional needs {curlies} now.

[Jacob Mandelson, 2009-09-11 19:49:44]

> http://codereview.chromium.org/201091/diff/4001/5009#newcode483
> Line 483: gint height = static_cast<gint>(animation->GetCurrentValue() *
> More readable?

Done.

> Line 1188: static_cast<gint>(event->y), &path,
> Bump &path to the next line.  It can go with the NULLs.

Done.

> Line 234: if (dragging)
> This conditional needs {curlies} now.

Done.

[Jacob Mandelson, 2009-09-11 20:07:52]

> Please put the rationale from your previous comments into the commit message,
as
> well as the GCC version which complains about these.

Done.

[Mark Mentovai, 2009-09-11 20:13:23]

FYI.

On older x86-family processors, a static cast from a floating-point number to an
integer is an expensive operation.  The truncation behavior specified by the
C[++] standard requires the x87 floating-point control word to be saved,
reloaded, and then restored around the cast.  Pre-P4, this would cause a
pipeline stall.  On P4, it is possible to switch between two values of the
floating-point control word without a stall.  On more recent processors,
reloading the control word never causes a stall.  On systems where SSE[2] is
guaranteed to be available (really, for us, only the Mac), the truncation can be
done in an SSE instruction that does not cause a stall.

In cases where truncation isn't required, a function from the lrint family may
be preferable to casting a floating-point number to an integer.

[Jacob Mandelson, 2009-09-11 20:25:16]

> On older x86-family processors, a static cast from a floating-point number to
an
> integer is an expensive operation.

You mean conversion when you say static cast, right?  There are implicit
conversions (usual arithmetic conversions, cross-type assignment) and explicit
(casts, type constructors).
-Wconversion catches the implicit conversions, and this patch makes them
explicit.
My understanding is there is no semantic change to making the implicit
conversions explicit, and they'll have the same performance even on older
processors, but by being explicit they are easier to clean.

[Paweł Hajdan Jr., 2009-09-14 18:54:38]

Landed in http://codereview.chromium.org/200122, thanks for the patch.

[Jacob Mandelson, 2009-09-14 19:17:24]

On 2009/09/14 18:54:38, Paweł Hajdan Jr. wrote:
> Landed in http://codereview.chromium.org/200122, thanks for the patch.

Thanks for your assistance!