Hack analysis macros for Windows to avoid compiler bug

Hack analysis macros for Windows to avoid compiler bug

In crrev.com/2669213005 the static code analysis macros that give the
compilers hints about what they can assume were updated to simplify
them. Unfortunately this broke VC++'s /analyze - this bug has the
details:

https://connect.microsoft.com/VisualStudio/feedback/details/3124774

It's not clear if it's worth adding back the complexity for /analyze
because its value has been marginal lately, so this change just skips
calling AnalysisAssumeTrue in order to get the /analyze builds running
again:

https://build.chromium.org/p/chromium.fyi/builders/Chromium%20Windows%20Analyze/

BUG=687243
R=kmarshall@chromium.org

[brucedawson, 2017-02-25 00:17:39]

The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-25 00:18:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Wez, 2017-02-25 00:25:47]

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

[Wez, 2017-02-25 00:25:47]

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

https://codereview.chromium.org/2720453003/diff/1/base/logging.h#newcode324
base/logging.h:324: //
https://connect.microsoft.com/VisualStudio/feedback/details/3124774
This is the same as the no-analysis case, below, so fold the two together? Or is
the plan to re-enable this if the compiler gets fixed?

[brucedawson, 2017-02-25 00:26:40]

On 2017/02/25 00:25:47, Wez wrote:
> https://codereview.chromium.org/2720453003/diff/1/base/logging.h
> File base/logging.h (right):
> 
> https://codereview.chromium.org/2720453003/diff/1/base/logging.h#newcode324
> base/logging.h:324: //
> https://connect.microsoft.com/VisualStudio/feedback/details/3124774
> This is the same as the no-analysis case, below, so fold the two together? Or
is
> the plan to re-enable this if the compiler gets fixed?

Yes, hope is to re-enable if the compiler gets fixed or if Microsoft suggests a
workaround.

[commit-bot: I haz the power, 2017-02-25 02:55:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-25 02:55:33]

Dry run: This issue passed the CQ dry run.

[brucedawson, 2017-02-27 18:58:39]

On 2017/02/25 00:26:40, brucedawson wrote:
> On 2017/02/25 00:25:47, Wez wrote:
> > https://codereview.chromium.org/2720453003/diff/1/base/logging.h
> > File base/logging.h (right):
> > 
> > https://codereview.chromium.org/2720453003/diff/1/base/logging.h#newcode324
> > base/logging.h:324: //
> > https://connect.microsoft.com/VisualStudio/feedback/details/3124774
> > This is the same as the no-analysis case, below, so fold the two together?
Or
> is
> > the plan to re-enable this if the compiler gets fixed?
> 
> Yes, hope is to re-enable if the compiler gets fixed or if Microsoft suggests
a
> workaround.

I did a local test build and I can confirm that this change is sufficient to get
Chrome building again with /analyze. I can remove the references to
__analysis_assume completely if that is deemed preferable - I can see the
benefits to either method.

[brucedawson, 2017-02-27 19:59:05]

Also, I found out what types cause the issue - bit fields. A universal reference
to a bit field does seem a bit odd - does clang really handle this? Details
here:

https://bugs.chromium.org/p/chromium/issues/detail?id=427616#c97

[brucedawson, 2017-02-27 20:55:08]

I think that the technique used does not actually work - the same code fails on
clang and VC++. Details in the bug.