Add __analysis_assume support to CHECK macros to reduce warnings with /analyze.

Add __analysis_assume support to CHECK macros to reduce warnings with /analyze.

Due to bugs and quirks in the analysis engine this is a very delicate dance.

The __analysis_assume invocation has to evaluate !!(condition) instead
of just condition because otherwise it fails on overloaded operators
such as with smart pointers. See this post for details:
http://randomascii.wordpress.com/2011/09/13/analyze-for-visual-studio-the-ugly-part-5/

The evaluation of condition in the 'regular' part of the macro has to
be removed during the /analyze phase in order to avoid confusing the
analysis engine which thinks that branching on the condition implies
uncertainty about the condition.

None of this is documented -- thinking like a compiler is required.

Wrapping this with #ifdef _PREFAST_ makes it easy to guarantee that this
will not affect regular builds.

To build add  win_debug_extra_cflags="/analyze"  to GYP_DEFINES, rebuild
the project files and then build.

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

[brucedawson, 2014-10-22 23:41:47]

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

[brucedawson, 2014-10-22 23:42:19]

Please take a look -- part one of making /analyze work better with Chrome.

[brucedawson, 2014-10-22 23:54:38]

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

[brucedawson, 2014-10-22 23:55:27]

Adding thestig as reviewer since he is an owner in base. Review away.

[Lei Zhang, 2014-10-23 00:03:17]

Looks ok to me, but I defer to cpu for the review.

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

https://codereview.chromium.org/674643002/diff/1/base/logging.h#newcode471
base/logging.h:471: #else // _PREFAST_
nit: two spaces before // per style guide.

[brucedawson, 2014-10-23 00:20:07]

On 2014/10/23 00:03:17, Lei Zhang wrote:
> Looks ok to me, but I defer to cpu for the review.
> 
> https://codereview.chromium.org/674643002/diff/1/base/logging.h
> File base/logging.h (right):
> 
> https://codereview.chromium.org/674643002/diff/1/base/logging.h#newcode471
> base/logging.h:471: #else // _PREFAST_
> nit: two spaces before // per style guide.

Fixed.

[cpu_(ooo_6.6-7.5), 2014-10-23 00:20:53]

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

https://codereview.chromium.org/674643002/diff/1/base/logging.h#newcode453
base/logging.h:453: #ifdef _PREFAST_
use #if defined(_PREFAST_)

In case of doubt about those stylistic issues check the rest of the file, use
whatever style is most common on the file.

The other comment which I don't feel strongly about is:

#if defined(_PREFAST_) && defined(OS_WIN)

I feel that it would be silly for another os to define that variable but maybe
it is more friendly to the casual reader of this file.

https://codereview.chromium.org/674643002/diff/1/base/logging.h#newcode642
base/logging.h:642: // See comments on the previous use of __analysis_assume
period at the end of this comment.

The comments like "#else // _PREFAST_" do not need a period, since they don't
form a sentence.

[brucedawson, 2014-10-23 00:33:06]

All comments addressed.

[cpu_(ooo_6.6-7.5), 2014-10-23 18:18:10]

lgtm

[cpu_(ooo_6.6-7.5), 2014-10-23 18:18:14]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2014-10-23 18:19:46]

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

[Lei Zhang, 2014-10-23 18:51:47]

lgtm++

[commit-bot: I haz the power, 2014-10-23 20:14:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-23 20:15:04]

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