document COMPILER(GCC) and COMPILER(MSVC) a little better

document COMPILER(GCC) and COMPILER(MSVC) a little better

Clang pretends to be gcc 4.4 by defining __GNUC__ and some related macros,
and therefore COMPILER(GCC) evaluates to 1 there.  To check for GCC
specifically, you need to check something like COMPILER(GCC) && !COMPILER(CLANG).

A similar situation happens with COMPILER(MSVC) in Clang when building for
windows (since Clang emulates MSVC there).

Committed: https://crrev.com/a5ee6277d60307ce8998670b1b4914d64816498b
Cr-Commit-Position: refs/heads/master@{#357260}

[Mostyn Bramley-Moore, 2015-10-23 09:05:31]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-23 09:05:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420613005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420613005/1

[Mostyn Bramley-Moore, 2015-10-23 09:27:20]

Description was changed from

==========
make COMPILER(GCC) evaluate to 0 when using clang

Clang pretends to be gcc 4.4 by defining __GNUC__ and some related macros.
This fools WTF's COMPILER(GCC) macro, returning 1.  I think it makes sense
for COMPILER(GCC) to be more accurate, let's make it return 0 when using
clang, and update all existing uses of COMPILER(GCC) to
(COMPILER(GCC) || COMPILER(CLANG)), unless there's an earlier
COMPILER(CLANG) clause in an #if or #elif block.

BUG=546937
==========

to

==========
make COMPILER(GCC) evaluate to 0 when using clang

Clang pretends to be gcc 4.4 by defining __GNUC__ and some related macros.
This fools WTF's COMPILER(GCC) macro, returning 1.

I think it makes sense
for COMPILER(GCC) to be more accurate, let's make it return 0 when using
clang, and update all existing uses of COMPILER(GCC) to
(COMPILER(GCC) || COMPILER(CLANG)), unless there's an earlier
COMPILER(CLANG) clause in an #if or #elif block.

BUG=546937
==========

[Mostyn Bramley-Moore, 2015-10-23 09:27:47]

Description was changed from

==========
make COMPILER(GCC) evaluate to 0 when using clang

Clang pretends to be gcc 4.4 by defining __GNUC__ and some related macros.
This fools WTF's COMPILER(GCC) macro, returning 1.

I think it makes sense
for COMPILER(GCC) to be more accurate, let's make it return 0 when using
clang, and update all existing uses of COMPILER(GCC) to
(COMPILER(GCC) || COMPILER(CLANG)), unless there's an earlier
COMPILER(CLANG) clause in an #if or #elif block.

BUG=546937
==========

to

==========
make COMPILER(GCC) evaluate to 0 when using clang

Clang pretends to be gcc 4.4 by defining __GNUC__ and some related macros.
This fools WTF's COMPILER(GCC) macro, returning 1.

I think it makes sense for COMPILER(GCC) to be more accurate, let's make
it return 0 when using clang, and update all existing uses of COMPILER(GCC)
to (COMPILER(GCC) || COMPILER(CLANG)), unless there's an earlier
COMPILER(CLANG) clause in an #if or #elif block.

BUG=546937
==========

[Mostyn Bramley-Moore, 2015-10-23 09:41:18]

mostynb@opera.com changed reviewers:
+ jochen@chromium.org, thakis@chromium.org

[Mostyn Bramley-Moore, 2015-10-23 09:43:57]

mostynb@opera.com changed reviewers:
- jochen@chromium.org

[Mostyn Bramley-Moore, 2015-10-23 09:43:58]

@Nico: Does this look OK to you?  IMO the current setup is surprising, and this
CL makes things more explicit/obvious.

[commit-bot: I haz the power, 2015-10-23 12:11:30]

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

[commit-bot: I haz the power, 2015-10-23 12:11:31]

Dry run: This issue passed the CQ dry run.

[Nico, 2015-10-23 16:40:13]

On 2015/10/23 12:11:31, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

Not lgtm, no. This is good as is.

[Mostyn Bramley-Moore, 2015-10-23 18:20:41]

> Not lgtm, no. This is good as is.

Don't you find it confusing?  

We had some downstream clang build breakage when one of my colleagues landed a
patch of the form:
#if COMPILER(GCC)
<use some GCC-specific attributes that clang doesn't support>
#endif
<function definition...>

It's trivial to use a different #if condition to solve the problem
(COMPILER(GCC) && !COMPILER(CLANG)), but surely it would be more readable with
this patch.  I would not be surprised if some of the existing uses of #if
COMPILER(GCC) are not needed for clang, but are used anyway.  

If you still disagree, would you accept a patch which documents this case in
wtf/Compiler.h ?

[Nico, 2015-10-23 18:29:50]

On 2015/10/23 18:20:41, Mostyn Bramley-Moore wrote:
> > Not lgtm, no. This is good as is.
> 
> Don't you find it confusing?  

Yes, it's slightly confusing, but it's better than not doing it imo. It matches
how the "native" macros work: clang sets __clang__ but it also sets __GNUC__ in
gnu mode and _MSC_VER in clang-cl mode, and thanks to that most compiler
detection macros go down the right path without much work.


> 
> We had some downstream clang build breakage when one of my colleagues landed a
> patch of the form:
> #if COMPILER(GCC)
> <use some GCC-specific attributes that clang doesn't support>
> #endif
> <function definition...>
> 
> It's trivial to use a different #if condition to solve the problem
> (COMPILER(GCC) && !COMPILER(CLANG)), but surely it would be more readable with
> this patch.  I would not be surprised if some of the existing uses of #if
> COMPILER(GCC) are not needed for clang, but are used anyway.  
> 
> If you still disagree, would you accept a patch which documents this case in
> wtf/Compiler.h ?

Sure, some comment saying "true for gcc and clang in gcc mode" here and "true
for msvc and clang in msvc mode" for the compiler(msvc) one seems fine

[Mostyn Bramley-Moore, 2015-10-23 18:51:44]

Description was changed from

==========
make COMPILER(GCC) evaluate to 0 when using clang

Clang pretends to be gcc 4.4 by defining __GNUC__ and some related macros.
This fools WTF's COMPILER(GCC) macro, returning 1.

I think it makes sense for COMPILER(GCC) to be more accurate, let's make
it return 0 when using clang, and update all existing uses of COMPILER(GCC)
to (COMPILER(GCC) || COMPILER(CLANG)), unless there's an earlier
COMPILER(CLANG) clause in an #if or #elif block.

BUG=546937
==========

to

==========
document COMPILER(GCC) and COMPILER(MSVC) a little better

Clang pretends to be gcc 4.4 by defining __GNUC__ and some related macros,
and therefore COMPILER(GCC) evaluates to 1 there.  To check for GCC
specifically, you need to check something like COMPILER(GCC) &&
!COMPILER(CLANG).

A similar situation happens with COMPILER(MSVC) in Clang when building for
windows (since Clang emulates MSVC there).
==========

[Mostyn Bramley-Moore, 2015-10-23 18:54:02]

> > If you still disagree, would you accept a patch which documents this case in
> > wtf/Compiler.h ?
> 
> Sure, some comment saying "true for gcc and clang in gcc mode" here and "true
> for msvc and clang in msvc mode" for the compiler(msvc) one seems fine

Done.

I'm not familiar with Clang on platforms other than linux, so I'm not sure the
addition to the MSVC section is 100% accurate (but better than not mentioning
it,
I think).

[Mostyn Bramley-Moore, 2015-10-30 20:29:08]

*ping* (non-urgent)

[Nico, 2015-10-30 20:56:27]

lgtm

(sorry, forgot about this)

[Mostyn Bramley-Moore, 2015-10-30 21:23:55]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2015-10-30 21:24:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420613005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420613005/20001

[commit-bot: I haz the power, 2015-10-30 23:05:19]

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

[commit-bot: I haz the power, 2015-10-30 23:05:20]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Mostyn Bramley-Moore, 2015-10-31 00:07:23]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2015-10-31 00:08:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420613005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420613005/20001

[commit-bot: I haz the power, 2015-10-31 02:29:24]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-31 02:30:24]

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