Adding /analyze support to common.gypi through GYP_DEFINES=win_analyze=1

Adding /analyze support to common.gypi through GYP_DEFINES=win_analyze=1, and suppress one very noisy warning.

When win_analyze=1 is specified then /analyze is added to the command
line to enable static code analysis with VC++.
WarnAsErrors is set to false to allow reporting of all errors.
Additional, WX- is added to disable WarnAsErrors more robustly, necessary because some projects override WarnAsErrors.
Additionally, many noisy but low-value warnings are suppressed.
Finally, _USING_V110_SDK71_ is un-set when win_analyze is true because
this is incompatible with /analyze.

The change to callback_internal.h suppresses an extremely noisy warning about a potentially incorrect use of sizeof():
    base\callback_internal.h(80) : warning C6334: sizeof operator applied to an expression with an operator might yield unexpected results:  Parentheses can be used to disambiguate certain usages.

With this change almost all of Chrome can be built with high but useful levels of warnings.

bug=427616
Committed: https://crrev.com/4b6961df21fb1475efd3f6713ef5c34b1763d748
Cr-Commit-Position: refs/heads/master@{#301723}

[scottmg, 2014-10-27 17:47:02]

scottmg@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2014-10-27 17:47:03]

Is there a crbug filed for this? If not please file one to tie future changes
together, and then link it here with "BUG=12345" in the CL description.

https://codereview.chromium.org/676743003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/676743003/diff/1/build/common.gypi#newcode2799
build/common.gypi:2799: ['win_analyze==0', {
i think it would be better to do

'win_analyze':
  'defines!':
    '_USING...'

which removes the define, rather than not adding it, and then it can live near
the other block.

https://codereview.chromium.org/676743003/diff/1/build/common.gypi#newcode3223
build/common.gypi:3223: ['win_analyze!=0', {
just 'win_analyze' is fine.

https://codereview.chromium.org/676743003/diff/1/build/common.gypi#newcode3224
build/common.gypi:3224: # When win_analye is specified add the /analyze switch.
There will be many
rewrap to 80col

https://codereview.chromium.org/676743003/diff/1/build/common.gypi#newcode3228
build/common.gypi:3228: 'AdditionalOptions': ['/analyze /WX- /wd6312 /wd6326
/wd28159 /wd28204 /wd28251 /wd28301 /wd28252 /wd28196 /wd6340 /wd6334 /wd6011
/wd6246 /wd6244 /wd28182'],
make these separate items in a list, i.e.

'AdditionalOptions': [
  '/analyze',
  '/WX-',
  '/wd6312',  # Disables xyz.
  ...
]

[cpu_(ooo_6.6-7.5), 2014-10-27 19:46:59]

bug is 427616

[brucedawson, 2014-10-27 23:18:58]

All comments addressed, including my prior-to-checkin to-do list.

TANL

[scottmg, 2014-10-28 00:36:14]

We normally put "BUG=427616" at the end of the CL, not for any great reason.

lgtm with below

You'll need some in base/OWNERS to stamp for the trivial base change too.

https://codereview.chromium.org/676743003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/676743003/diff/20001/build/common.gypi#newcod...
build/common.gypi:3224: ['win_analyze', {
do you mean for this to be inside Debug_Base? (i would have assumed it'd be
Common_Base?)

if it's temporary until other configs work, please add that comment

if there's some reason why it'll only work in Debug, please add that comment.

[scottmg, 2014-10-28 00:38:38]

https://codereview.chromium.org/676743003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/676743003/diff/20001/build/common.gypi#newcod...
build/common.gypi:3228: # When win_analye is specified add the /analyze switch.
"win_analye" -> "win_analyze"

[brucedawson, 2014-10-28 17:34:52]

Fixed the spelling error and moved the analyze blocks together, out of the debug
block. win_analyze=1 works for debug and release now.

My assumption is that I need owners from both base and . (for the build change),
so maybe rvargas and cpu?

[scottmg, 2014-10-28 17:51:43]

You don't need to walk the whole tree, only at the level files you're touching
are at.

build/OWNERS is * so my lgtm is sufficient. Anyone in base/OWNERS can
rubberstamp for the trivial brackets change. I think Ricardo is on vacation.
thakis, thestig, or danakj would be good candidates.

(cpu is at the root, so he can approve any cl. Generally, prefer reviewers that
are "closer" to the code, rather than top level unless it's a cross-cutting
refactor with a lot of little changes.)

[brucedawson, 2014-10-28 17:54:36]

brucedawson@chromium.org changed reviewers:
+ danakj@chromium.org

[brucedawson, 2014-10-28 17:55:16]

Added danakj for change to base. Ready for final review.

[danakj, 2014-10-28 18:29:26]

LGTM

[cpu_(ooo_6.6-7.5), 2014-10-28 19:44:42]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2014-10-28 19:46:13]

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

[commit-bot: I haz the power, 2014-10-28 22:18:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-28 22:18:55]

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