Fix more disabled MSVC warnings, base/ edition.

Fix more disabled MSVC warnings, base/ edition.

Mostly this fixes cases of "possible value truncation", usually by inserting
explicit typecasts.

BUG=81439
TEST=none

Committed: https://crrev.com/9cf9b94a63dec7653bb48bac948deb4008b532c8
Cr-Commit-Position: refs/heads/master@{#297723}

[Peter Kasting, 2014-09-23 23:07:33]

pkasting@chromium.org changed reviewers:
+ thakis@chromium.org

[Peter Kasting, 2014-09-23 23:07:34]

https://codereview.chromium.org/596103002/diff/1/base/files/file_path_unittes...
File base/files/file_path_unittest.cc (right):

https://codereview.chromium.org/596103002/diff/1/base/files/file_path_unittes...
base/files/file_path_unittest.cc:779: FilePath
path(double_extension_cases[i].input);
This was caught by "warn on unused locals"; turns out we were not actually
running tests for these cases...

https://codereview.chromium.org/596103002/diff/1/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/596103002/diff/1/base/logging.h#newcode622
base/logging.h:622: LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON ? !(condition)
: false)  \
This form of the macro eliminates "unused variable" warnings compared to the
other form.

[Peter Kasting, 2014-09-30 23:16:37]

pkasting@chromium.org changed reviewers:
+ brettw@chromium.org
- thakis@chromium.org

[Peter Kasting, 2014-09-30 23:16:37]

Since Nico is apparently away, changing to brettw.

[brettw, 2014-10-01 17:54:07]

https://codereview.chromium.org/596103002/diff/40001/base/debug/crash_logging.cc
File base/debug/crash_logging.cc (left):

https://codereview.chromium.org/596103002/diff/40001/base/debug/crash_logging...
base/debug/crash_logging.cc:39: return std::ceil(length /
static_cast<float>(g_chunk_max_length_));
I felt like the intent of the old code was clearer here, even if it involved an
"ugly" round-trip through floating-point-land. Can we just cast the result (I
assume there's a warning here or you wouldn't be changing it).

https://codereview.chromium.org/596103002/diff/40001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/596103002/diff/40001/base/logging.h#newcode622
base/logging.h:622: LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON ? !(condition)
: false)  \
I don't follow the reason for this change.

[Peter Kasting, 2014-10-01 19:30:21]

https://codereview.chromium.org/596103002/diff/40001/base/debug/crash_logging.cc
File base/debug/crash_logging.cc (left):

https://codereview.chromium.org/596103002/diff/40001/base/debug/crash_logging...
base/debug/crash_logging.cc:39: return std::ceil(length /
static_cast<float>(g_chunk_max_length_));
On 2014/10/01 17:54:06, brettw wrote:
> I felt like the intent of the old code was clearer here, even if it involved
an
> "ugly" round-trip through floating-point-land. Can we just cast the result (I
> assume there's a warning here or you wouldn't be changing it).

We can cast the result, but if something is unclear, I think it would be better
to add a comment than use floating-point math for a case when we don't need it. 
(I'm actually surprised you're not familiar with the (n+k-1)/k idiom, I thought
it was pretty well-known.)

I added a comment, see what you think.

https://codereview.chromium.org/596103002/diff/40001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/596103002/diff/40001/base/logging.h#newcode622
base/logging.h:622: LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON ? !(condition)
: false)  \
On 2014/10/01 17:54:07, brettw wrote:
> I don't follow the reason for this change.

My explanatory comment from an earlier patchset (
https://codereview.chromium.org/596103002/diff/1/base/logging.h#newcode622 ):
"This form of the macro eliminates "unused variable" warnings compared to the
other form."

More specifically, since && short-circuits, MSVC doesn't consider the old form
to have "referenced" |condition| as desired when DCHECK_IS_ON is known at
compile time to be false.  Using ?: instead ensures the compiler considers
|condition| to have been referenced in all cases, while preserving the property
that it isn't actually evaluated when DCHECKs are off.

(I ran into these warnings and found a number of helpful pages online noting
that the ?: form was a solution that worked across compilers.)

[brettw, 2014-10-01 19:32:41]

I am familiar with the ceil math, I just think it's less clear than the old
code. At the end of the day I don't care that much either way. LGTM

[Peter Kasting, 2014-10-01 19:37:43]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2014-10-01 19:39:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/596103002/60001

[commit-bot: I haz the power, 2014-10-01 22:19:01]

Committed patchset #4 (id:60001) as 7ef65448af945f9799a1a60bcee4185e7d23c9b8

[commit-bot: I haz the power, 2014-10-01 22:19:54]

Patchset 4 (id:??) landed as
https://crrev.com/9cf9b94a63dec7653bb48bac948deb4008b532c8
Cr-Commit-Position: refs/heads/master@{#297723}