Make bit_cast fail if the source or dest are not trivially copyable

Introduce base::is_trivially_copyable and use for bit_cast

bit_cast had some one-off preprocessor logic to check or not to check
for the types being trivially copyable that ended up not being enabled
at all on windows.

Since most of our platforms are now C++11 make that the norm and cover
the various edge cases via base::is_trivially_copyable. Then bit_cast
can use that and will be able to static_assert for this on all platforms.

R=dcheng
BUG=555754
Committed: https://crrev.com/7cebcfb2d80152bc84d88fc711eda6e61acda8be
Cr-Commit-Position: refs/heads/master@{#440491}

[danakj, 2016-12-19 22:06:34]

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

[commit-bot: I haz the power, 2016-12-19 22:06:57]

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

[danakj, 2016-12-19 22:14:00]

This might compile.

[commit-bot: I haz the power, 2016-12-19 22:22:21]

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

[commit-bot: I haz the power, 2016-12-19 22:22:22]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[dcheng, 2016-12-19 23:57:21]

On 2016/12/19 22:14:00, danakj_OOO_and_slow wrote:
> This might compile.

It didn't (which I think is the expected behavior). Should we make this a
nocompile test, or is there some clever thing we can do with static_assert?

[danakj, 2016-12-20 14:27:08]

On 2016/12/19 23:57:21, dcheng wrote:
> On 2016/12/19 22:14:00, danakj_OOO_and_slow wrote:
> > This might compile.
> 
> It didn't (which I think is the expected behavior). Should we make this a
> nocompile test, or is there some clever thing we can do with static_assert?

Oops ya I didnt mean to include the test. I don't know anything clever.

[danakj, 2016-12-20 14:29:02]

PTAL

[danakj, 2016-12-20 15:25:59]

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

[commit-bot: I haz the power, 2016-12-20 15:26:15]

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, 2016-12-20 15:42:37]

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

[commit-bot: I haz the power, 2016-12-20 15:42:38]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[danakj, 2016-12-20 16:40:49]

https://codereview.chromium.org/2583353002/diff/20001/base/bit_cast.h
File base/bit_cast.h (right):

https://codereview.chromium.org/2583353002/diff/20001/base/bit_cast.h#newcode77
base/bit_cast.h:77: static_assert(std::is_trivially_copyable<Dest>::value,
Wait we already have this. With crazy stuff for various libary versions /o\

[danakj, 2016-12-20 16:40:58]

Description was changed from

==========
Make bit_cast fail if the source or dest are not trivially copyable

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

to

==========
Make bit_cast fail if the source or dest are not trivially copyable

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

[danakj, 2016-12-20 16:59:57]

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

[commit-bot: I haz the power, 2016-12-20 17:00:17]

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

[danakj, 2016-12-20 17:00:56]

I have removed the #ifs from bit_cast.h and moved to something simpler in
template_util.h instead. This should now check on windows.. and on android,
which I think was not being checked based on the compile results in an earlier
patch. But let's see what the bots do now

[commit-bot: I haz the power, 2016-12-20 17:15:21]

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

[commit-bot: I haz the power, 2016-12-20 17:15:22]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[danakj, 2016-12-20 17:48:46]

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

[commit-bot: I haz the power, 2016-12-20 17:49:30]

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

[danakj, 2016-12-20 17:55:21]

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

[commit-bot: I haz the power, 2016-12-20 17:55:47]

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

[danakj, 2016-12-20 18:10:16]

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

[commit-bot: I haz the power, 2016-12-20 18:10:52]

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, 2016-12-20 18:33:57]

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

[commit-bot: I haz the power, 2016-12-20 18:33:58]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[danakj, 2016-12-20 18:35:46]

So this is exciting. On android we get this:

In file included from ../../base/bit_cast_unittest.cc:6:0:
../../base/bit_cast.h: In instantiation of 'Dest bit_cast(const Source&) [with
Dest = int; Source = base::{anonymous}::A]':
../../base/bit_cast_unittest.cc:26:26:   required from here
../../base/bit_cast.h:69:3: error: static assertion failed: bit_cast requires
the source type to be copyable
   static_assert(base::is_trivially_copyable<Source>::value,
   ^

The line it's referring to is

struct A {
  int x;
};

TEST(BitCastTest, StructureInt) {
  A a = { 1 };
  int b = bit_cast<int>(a);  <--- HERE
  EXPECT_EQ(1, b);
}

Of course A is POD so it should be trivially copyable... so...?

[danakj, 2016-12-20 18:37:07]

On chromeos we get:

/b/c/cipd/goma/gomacc armv7a-cros-linux-gnueabi-g++
-B/b/c/b/chromeos_daisy_chromium_compile_only_ng/.cros_cache/chrome-sdk/tarballs/daisy+9095.0.0+target_toolchain/usr/x86_64-pc-linux-gnu/armv7a-cros-linux-gnueabi/binutils-bin/2.25.51-gold
-MMD -MF
obj/ash/public/interfaces/interfaces_shared_cpp_sources/accelerator_controller.mojom-shared.o.d
-DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_ASH=1
-DUSE_AURA=1 -DUSE_NSS_CERTS=1 -DUSE_OZONE=1 -DENABLE_WAYLAND_SERVER=1
-DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL
-DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2
-DOS_CHROMEOS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -I../.. -Igen
-fno-strict-aliasing -funwind-tables -fPIC -pipe
-B../../third_party/binutils/Linux_x64/Release/bin -march=armv7-a
-mfloat-abi=hard -mtune=generic-armv7-a -pthread -mfpu=neon -mthumb -Wall
-Werror -Wno-psabi -Wno-unused-local-typedefs -Wno-maybe-uninitialized
-Wno-missing-field-initializers -Wno-unused-parameter -O2 -fno-ident
-fdata-sections -ffunction-sections -g1 -gsplit-dwarf
--sysroot=../../../.cros_cache/chrome-sdk/tarballs/daisy+9095.0.0+sysroot_chromeos-base_chromeos-chrome.tar.xz
-fvisibility=hidden -fno-threadsafe-statics -fvisibility-inlines-hidden
-std=gnu++11 -Wno-narrowing -Wno-literal-suffix -fno-rtti -fno-exceptions -pipe
-march=armv7-a -mtune=cortex-a15 -mfpu=neon -mfloat-abi=hard
-D__google_stl_debug_vector=1 -c
gen/ash/public/interfaces/accelerator_controller.mojom-shared.cc -o
obj/ash/public/interfaces/interfaces_shared_cpp_sources/accelerator_controller.mojom-shared.o
In file included from ../../base/logging.h:20:0,
                 from ../../mojo/public/cpp/bindings/lib/array_internal.h:14,
                 from ../../mojo/public/cpp/bindings/array_data_view.h:10,
                 from
gen/ash/public/interfaces/accelerator_controller.mojom-shared.h:16,
                 from
gen/ash/public/interfaces/accelerator_controller.mojom-shared.cc:10:
../../base/template_util.h:138:36: error: 'is_trivially_copyable' in namespace
'std' does not name a template type
 using is_trivially_copyable = std::is_trivially_copyable<T>;
                                    ^

So.. it's not linux chroot but it also doesn't have is_trivially_copyable. Why
is C++ so hard augh.

[danakj, 2016-12-20 19:26:57]

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

[commit-bot: I haz the power, 2016-12-20 19:27:36]

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

[danakj, 2016-12-20 19:41:06]

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

[commit-bot: I haz the power, 2016-12-20 19:41:38]

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

[danakj, 2016-12-20 19:53:24]

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

[commit-bot: I haz the power, 2016-12-20 19:53:43]

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

[danakj, 2016-12-20 20:02:41]

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

[commit-bot: I haz the power, 2016-12-20 20:03:04]

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, 2016-12-20 20:14:53]

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

[commit-bot: I haz the power, 2016-12-20 20:14:54]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[danakj, 2016-12-20 20:21:38]

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

[commit-bot: I haz the power, 2016-12-20 20:22:33]

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

[danakj, 2016-12-20 20:33:03]

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

[commit-bot: I haz the power, 2016-12-20 20:33:45]

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, 2016-12-20 21:05:40]

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

[commit-bot: I haz the power, 2016-12-20 21:05:41]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[danakj, 2016-12-21 15:01:48]

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

[commit-bot: I haz the power, 2016-12-21 15:02:03]

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

[danakj, 2016-12-21 15:15:29]

Ok so whats happening here is I believe:

1. Android is using clang with libstdc++, but that breaks in the libc++
implementation of is_trivially_copyable until either gcc 5.1 (which adds an
intrinsic) or some commit in libc++ that branches on gcc 5.1 so kinda half-ass
supports earlier gcc.

I've added the same branchiness into base::is_trivially_copyable so that we work
on what android has which is gcc < 5.1 and libc++ before the fix.

2. 2 chromeos bots appear to be using an experimental gcc 5.0 instead of 5.1 or
something. why, one will never know. So I'm trying adding it to the blacklist of
"dont try to use libstdc++ for type_traits here" to see if that passes

[commit-bot: I haz the power, 2016-12-21 15:41:19]

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

[commit-bot: I haz the power, 2016-12-21 15:41:19]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[danakj, 2016-12-21 16:01:31]

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

[commit-bot: I haz the power, 2016-12-21 16:02:25]

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

[danakj, 2016-12-21 16:48:34]

Description was changed from

==========
Make bit_cast fail if the source or dest are not trivially copyable

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

to

==========
Introduce base::is_trivially_copyable and use for bit_cast

bit_cast had some one-off preprocessor logic to check or not to check
for the types being trivially copyable that ended up not being enabled
at all on windows.

Since most of our platforms are now C++11 make that the norm and cover
the various edge cases via base::is_trivially_copyable. Then bit_cast
can use that and will be able to static_assert for this on all platforms.

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

[danakj, 2016-12-21 17:24:25]

Yep got it working.. pTAL

[commit-bot: I haz the power, 2016-12-21 18:04:37]

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

[commit-bot: I haz the power, 2016-12-21 18:04:38]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[danakj, 2016-12-21 18:19:39]

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

[commit-bot: I haz the power, 2016-12-21 18:20:13]

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, 2016-12-21 20:21:33]

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

[commit-bot: I haz the power, 2016-12-21 20:21:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[danakj, 2016-12-21 20:36:20]

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

[commit-bot: I haz the power, 2016-12-21 20:36:44]

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

[dcheng, 2016-12-21 20:50:18]

Use a sysroot, they said. Just one standard library to program against, they
said.

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h#n...
base/template_util.h:29: #define CR_GLIBCXX_5_0_0 20150123
Doesn't line 22 reference this? Should we delete addition at line 22, since we
want to use a different fallback path?

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h#n...
base/template_util.h:148: // type traits. In this case we should provide it
based on compiler intrinsics.
Nit: I find bulleted lists in comments to be easier to read when the list item
is aligned to the right of the bullets.

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h#n...
base/template_util.h:153: // - When compiling libc++ from before git commit
eeeada1c7, with gcc compiler,
I'm guessing this is not a Chromium commit hash, so perhaps mention which
project this is (I guess libc++? But I think libc++ still uses svn?)

[danakj, 2016-12-21 20:58:33]

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

[danakj, 2016-12-21 20:58:38]

PTAL

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h
File base/template_util.h (right):

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h#n...
base/template_util.h:29: #define CR_GLIBCXX_5_0_0 20150123
On 2016/12/21 20:50:18, dcheng wrote:
> Doesn't line 22 reference this? Should we delete addition at line 22, since we
> want to use a different fallback path?

Oops, ya removing the 22 one.

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h#n...
base/template_util.h:148: // type traits. In this case we should provide it
based on compiler intrinsics.
On 2016/12/21 20:50:18, dcheng wrote:
> Nit: I find bulleted lists in comments to be easier to read when the list item
> is aligned to the right of the bullets.

k

https://codereview.chromium.org/2583353002/diff/280001/base/template_util.h#n...
base/template_util.h:153: // - When compiling libc++ from before git commit
eeeada1c7, with gcc compiler,
On 2016/12/21 20:50:18, dcheng wrote:
> I'm guessing this is not a Chromium commit hash, so perhaps mention which
> project this is (I guess libc++? But I think libc++ still uses svn?)

Youre right, I got it from the bug, but it is r239653 for libc++ thats better
then.

[commit-bot: I haz the power, 2016-12-21 20:59:11]

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

[dcheng, 2016-12-21 22:34:43]

lgtm, gl on the waterfall

[commit-bot: I haz the power, 2016-12-21 23:01:00]

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

[commit-bot: I haz the power, 2016-12-21 23:01:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[danakj, 2016-12-22 18:35:20]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-12-22 18:35:45]

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, 2016-12-22 20:44:11]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1482431720705800,
"parent_rev": "ac6a5266d7d77c41756ca69b2962acfa69191e45", "commit_rev":
"f1fe282305b0d2a91b95086a37c0d838c2592015"}

[commit-bot: I haz the power, 2016-12-22 20:44:41]

Description was changed from

==========
Introduce base::is_trivially_copyable and use for bit_cast

bit_cast had some one-off preprocessor logic to check or not to check
for the types being trivially copyable that ended up not being enabled
at all on windows.

Since most of our platforms are now C++11 make that the norm and cover
the various edge cases via base::is_trivially_copyable. Then bit_cast
can use that and will be able to static_assert for this on all platforms.

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

to

==========
Introduce base::is_trivially_copyable and use for bit_cast

bit_cast had some one-off preprocessor logic to check or not to check
for the types being trivially copyable that ended up not being enabled
at all on windows.

Since most of our platforms are now C++11 make that the norm and cover
the various edge cases via base::is_trivially_copyable. Then bit_cast
can use that and will be able to static_assert for this on all platforms.

R=dcheng
BUG=555754

Review-Url: https://codereview.chromium.org/2583353002
==========

[commit-bot: I haz the power, 2016-12-22 20:44:43]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-12-22 20:48:19]

Description was changed from

==========
Introduce base::is_trivially_copyable and use for bit_cast

bit_cast had some one-off preprocessor logic to check or not to check
for the types being trivially copyable that ended up not being enabled
at all on windows.

Since most of our platforms are now C++11 make that the norm and cover
the various edge cases via base::is_trivially_copyable. Then bit_cast
can use that and will be able to static_assert for this on all platforms.

R=dcheng
BUG=555754

Review-Url: https://codereview.chromium.org/2583353002
==========

to

==========
Introduce base::is_trivially_copyable and use for bit_cast

bit_cast had some one-off preprocessor logic to check or not to check
for the types being trivially copyable that ended up not being enabled
at all on windows.

Since most of our platforms are now C++11 make that the norm and cover
the various edge cases via base::is_trivially_copyable. Then bit_cast
can use that and will be able to static_assert for this on all platforms.

R=dcheng
BUG=555754

Committed: https://crrev.com/7cebcfb2d80152bc84d88fc711eda6e61acda8be
Cr-Commit-Position: refs/heads/master@{#440491}
==========

[commit-bot: I haz the power, 2016-12-22 20:48:20]

Patchset 16 (id:??) landed as
https://crrev.com/7cebcfb2d80152bc84d88fc711eda6e61acda8be
Cr-Commit-Position: refs/heads/master@{#440491}