Enable type_traits fallback for all < gcc 5.0 releases.

Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < gcc-5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable' in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=danakj
BUG=555754
Review-Url: https://codereview.chromium.org/2612933003
Cr-Commit-Position: refs/heads/master@{#441985}
Committed: https://chromium.googlesource.com/chromium/src/+/7c03b2072b7497184e8a1127c8ab443aeae42a96

[sof, 2017-01-05 09:50:05]

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

[commit-bot: I haz the power, 2017-01-05 09:50:22]

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

[sof, 2017-01-05 10:46:22]

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

[commit-bot: I haz the power, 2017-01-05 10:46:40]

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

[Mostyn Bramley-Moore, 2017-01-05 10:53:54]

mostynb@opera.com changed reviewers:
+ mostynb@opera.com

[Mostyn Bramley-Moore, 2017-01-05 10:53:54]

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h#ne...
base/template_util.h:31: #if _GNUC_VER < 500 || (defined(__GLIBCXX__) &&
__GLIBCXX__ == CR_GLIBCXX_5_0_0)
Where is _GNUC_VER defined?  It doesn't appear to be predefined in my tests with
g++ 4.8 and 5.4.0.

Also, I think this feature is libstdc++ thing, which is not strictly coupled to
the compiler version (ie you can use a brand new libstdc++ with an old compiler
just fine).

[sof, 2017-01-05 10:57:38]

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h#ne...
base/template_util.h:31: #if _GNUC_VER < 500 || (defined(__GLIBCXX__) &&
__GLIBCXX__ == CR_GLIBCXX_5_0_0)
On 2017/01/05 10:53:54, Mostyn Bramley-Moore wrote:
> Where is _GNUC_VER defined?  It doesn't appear to be predefined in my tests
with
> g++ 4.8 and 5.4.0.
> 
> Also, I think this feature is libstdc++ thing, which is not strictly coupled
to
> the compiler version (ie you can use a brand new libstdc++ with an old
compiler
> just fine).

It's used below in this file, so I assume it is well-defined and safe to use in
this context?

As we're testing against older libstdc++ here, your concern doesn't seem to
hold. You could use a newer g++, but why would you want to use an old libstdc++
with it?

[sof, 2017-01-05 11:22:09]

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

[commit-bot: I haz the power, 2017-01-05 11:22:26]

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

[Mostyn Bramley-Moore, 2017-01-05 12:26:32]

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h#ne...
base/template_util.h:31: #if _GNUC_VER < 500 || (defined(__GLIBCXX__) &&
__GLIBCXX__ == CR_GLIBCXX_5_0_0)
> It's used below in this file, so I assume it is well-defined and safe to use
in
> this context?

It looks like _GNUC_VER is a libcxx macro, and since it's undefined here then
the value is 0 in libstdc++ builds.  Which would mean that the __GLIBCXX__
checks are possibly never used.

> As we're testing against older libstdc++ here, your concern doesn't seem to
> hold. You could use a newer g++, but why would you want to use an old
libstdc++
> with it?

I don't personally use newer g++ with older libstdc++, but I know that Google
does this (or did at some point), in order to produce chrome builds for eg old
debian releases.

[sof, 2017-01-05 12:35:29]

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/20001/base/template_util.h#ne...
base/template_util.h:31: #if _GNUC_VER < 500 || (defined(__GLIBCXX__) &&
__GLIBCXX__ == CR_GLIBCXX_5_0_0)
On 2017/01/05 12:26:32, Mostyn Bramley-Moore wrote:
> > It's used below in this file, so I assume it is well-defined and safe to use
> in
> > this context?
> 
> It looks like _GNUC_VER is a libcxx macro, and since it's undefined here then
> the value is 0 in libstdc++ builds.  Which would mean that the __GLIBCXX__
> checks are possibly never used.
> 

It's been subsequently updated to only rely on __GNUC__; we're only after the
major version here after all.

[sof, 2017-01-05 12:37:48]

sigbjornf@opera.com changed reviewers:
+ danakj@chromium.org, reveman@chromium.org
- mostynb@opera.com

[sof, 2017-01-05 12:37:49]

please take a look.

Followup to r440791 - i'm unable to build with 4.9.4. It seems safe to widen the
fallback to hold for all < 5 releases instead.

[commit-bot: I haz the power, 2017-01-05 13:18:57]

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

[commit-bot: I haz the power, 2017-01-05 13:18:58]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-01-05 16:30:15]

https://codereview.chromium.org/2612933003/diff/40001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/40001/base/template_util.h#ne...
base/template_util.h:31: #if (defined(__GNUC__) && __GNUC_ < 5) || \
I agree that it's odd to check the compiler version not the library version
here. If some toolchain is using an older gcc but a newer library, then they
will fall back to our own implementation of is_trivially_copyable. But I think
a) using a new lib with older compiler is probably not supported and wouldn't
work anyways b) falling back in this case still does something reasonable..ish.

Can you improve the comment to clarify about the mismatch between checking
compiler vs library versions.

[sof, 2017-01-05 18:59:44]

https://codereview.chromium.org/2612933003/diff/40001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/40001/base/template_util.h#ne...
base/template_util.h:31: #if (defined(__GNUC__) && __GNUC_ < 5) || \
On 2017/01/05 16:30:15, danakj_OOO_and_slow wrote:
> I agree that it's odd to check the compiler version not the library version
> here. If some toolchain is using an older gcc but a newer library, then they
> will fall back to our own implementation of is_trivially_copyable. But I think
> a) using a new lib with older compiler is probably not supported and wouldn't
> work anyways b) falling back in this case still does something
reasonable..ish.
> 
> Can you improve the comment to clarify about the mismatch between checking
> compiler vs library versions.

ok, tried to do so.

ABI skew is one reason why it makes some sense to compile with a consistent
(compiler,library) pairing, esp so with gcc5's changes in that area (
https://gcc.gnu.org/gcc-5/changes.html#libstdcxx )

The version testing here mirrors how we do it on the clang side, btw --

 http://llvm.org/docs/doxygen/html/type__traits_8h_source.html#l00030

[danakj, 2017-01-05 19:18:05]

Description was changed from

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation with various < 5.0  toolchains lacking
std::is_trivially_copyable<T>.

R=
BUG=555754
==========

to

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation with various < 5.0  toolchains lacking
std::is_trivially_copyable<T>.

BUG=555754
==========

[sof, 2017-01-05 19:18:33]

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

[commit-bot: I haz the power, 2017-01-05 19:18:57]

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

[danakj, 2017-01-05 19:19:55]

https://codereview.chromium.org/2612933003/diff/60001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/60001/base/template_util.h#ne...
base/template_util.h:34: #if (defined(__GNUC__) && __GNUC_ < 5) || \
This should have two _s right? How is this working for you?

[sof, 2017-01-05 19:25:18]

https://codereview.chromium.org/2612933003/diff/60001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2612933003/diff/60001/base/template_util.h#ne...
base/template_util.h:34: #if (defined(__GNUC__) && __GNUC_ < 5) || \
On 2017/01/05 19:19:55, danakj_OOO_and_slow wrote:
> This should have two _s right? How is this working for you?

ouch, good catch - it happens to work out as it expands to 0. Fixed, thanks.

[sof, 2017-01-05 19:27:28]

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

[commit-bot: I haz the power, 2017-01-05 19:27:55]

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

[commit-bot: I haz the power, 2017-01-05 21:35:52]

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

[commit-bot: I haz the power, 2017-01-05 21:35:53]

Dry run: This issue passed the CQ dry run.

[sof, 2017-01-05 21:57:47]

Description was changed from

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation with various < 5.0  toolchains lacking
std::is_trivially_copyable<T>.

BUG=555754
==========

to

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < 5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable'
in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=
BUG=555754
==========

[sof, 2017-01-05 22:00:03]

extended the description to include error message details.

[sof, 2017-01-05 22:11:51]

Description was changed from

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < 5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable'
in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=
BUG=555754
==========

to

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < gcc-5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable'
in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=
BUG=555754
==========

[sof, 2017-01-06 16:30:20]

ping, it'd be good to cleanly be able to build ToT again.

[danakj, 2017-01-06 16:31:33]

Oops meant to LGTM, thanks

[sof, 2017-01-06 18:24:24]

Description was changed from

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < gcc-5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable'
in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=
BUG=555754
==========

to

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < gcc-5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable'
in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=danakj
BUG=555754
==========

[sof, 2017-01-06 18:27:14]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2017-01-06 18:27:42]

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

[commit-bot: I haz the power, 2017-01-06 18:35:00]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1483727234325180,
"parent_rev": "b89df0eff63dbd798fbc33765c6f70a9b3d7f3bf", "commit_rev":
"7c03b2072b7497184e8a1127c8ab443aeae42a96"}

[commit-bot: I haz the power, 2017-01-06 18:35:30]

Description was changed from

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < gcc-5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable'
in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=danakj
BUG=555754
==========

to

==========
Enable type_traits fallback for all < gcc 5.0 releases.

Fixes compilation failures with various < gcc-5.0  toolchains lacking
std::is_trivially_copyable<T>. Without it, compile errors like

../../base/template_util.h:189:36: error: no type named 'is_trivially_copyable'
in namespace 'std'
using is_trivially_copyable = std::is_trivially_copyable<T>;
                              ~~~~~^
../../base/template_util.h:189:57: error: expected ';' after alias declaration
using is_trivially_copyable = std::is_trivially_copyable<T>;
                                                        ^
                                                        ;
will be encountered.

R=danakj
BUG=555754

Review-Url: https://codereview.chromium.org/2612933003
Cr-Commit-Position: refs/heads/master@{#441985}
Committed:
https://chromium.googlesource.com/chromium/src/+/7c03b2072b7497184e8a1127c8ab...
==========

[commit-bot: I haz the power, 2017-01-06 18:35:31]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/7c03b2072b7497184e8a1127c8ab...