Some type_traits don't work with gcc and libc++, as on Android
Reported by
tsniatow...@opera.com,
Apr 27 2016
|
||
Issue description
In the current default android config some std:: type traits don't work as expected. Notably:
* std::is_convertible<char[], std::string>::value is false
* with struct A { int a; }; std::is_trivially_copyable<A>::value is false
The above are true in other configs I checked (linux gcc, linux clang, android clang).
The problem is that there are now more ways for code to trigger the incorrect traits after changes like https://chromium-review.googlesource.com/331150 use more std:: traits. Some non-default targets already fail to build, for example android with enable_plugins=true triggers a static_assert in bit_cast about is_trivially_copyable because of https://codereview.chromium.org/1837563002
We should probably check that the traits work as expected before we use them everywhere.
,
Apr 27 2016
Will do, however this is not specific to NDK, it fails for me locally on an Ubuntu x86_64 g++/libc++ toolchain as well (from libc++-helpers):
$ g++-libc++ trivially_copyable.cpp -std=c++11
trivially_copyable.cpp:3:1: error: static assertion failed: A
static_assert(std::is_trivially_copyable<A>::value, "A");
^
$ cat trivially_copyable.cpp
#include <type_traits>
struct A { int a; };
static_assert(std::is_trivially_copyable<A>::value, "A");
$ cat $(which g++-libc++)
#!/bin/sh
g++ -std=c++0x -nodefaultlibs -lc++ -lc -lgcc_s -isystem/usr/include/c++/v1 "$@"
,
Apr 27 2016
Yes, but I think we only use gcc + libc++ on Android atm, and that's a configuration that's supported by the NDK. (But eventually we'll use clang for Android and this will go away -- and gcc is already deprecated in the NDK so who knows if they'll fix.) -- I guess you can file a libc++ bug on llvm.org/bugs
,
Apr 27 2016
Filed https://code.google.com/p/android/issues/detail?id=208352 and https://llvm.org/bugs/show_bug.cgi?id=27538 Suggestions on how to proceed for now? I can fiddle with the ifdefs to fix base/bit_case by avoiding std::is_convertible on gcc+libc++, but making sure nobody uses the buggy is_convertible to choose between template specializations is harder.
,
Apr 27 2016
I updated the LLVM bug with details: https://llvm.org/bugs/show_bug.cgi?id=27538 Based on that, I think the code should detect: defined(_GNUC_VER) && _GNUC_VER < 501 && defined(_LIBCPP_VERSION) && _LIBCPP_VERSION <= 1101 And simply fall back to the basic case without static_assert. The bot will catch bad code.
,
Apr 28 2016
https://codereview.chromium.org/1925683002/ has some wip
,
Apr 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/474f4a511b7bdbfcf09c1e914ee99fd9e35db0f8 commit 474f4a511b7bdbfcf09c1e914ee99fd9e35db0f8 Author: tsniatowski <tsniatowski@opera.com> Date: Thu Apr 28 20:53:20 2016 Add simple bit_cast unittests, avoid static_assert on gcc+libc++ These should work provided that the is_trivially_copyable logic is not broken, and will fail to compile of the type traits lie. They do on Android where we have gcc+libc++, so avoid being too strict there. BUG=607158 Review-Url: https://codereview.chromium.org/1925683002 Cr-Commit-Position: refs/heads/master@{#390472} [modify] https://crrev.com/474f4a511b7bdbfcf09c1e914ee99fd9e35db0f8/base/BUILD.gn [modify] https://crrev.com/474f4a511b7bdbfcf09c1e914ee99fd9e35db0f8/base/base.gyp [modify] https://crrev.com/474f4a511b7bdbfcf09c1e914ee99fd9e35db0f8/base/bit_cast.h [add] https://crrev.com/474f4a511b7bdbfcf09c1e914ee99fd9e35db0f8/base/bit_cast_unittest.cc
,
Aug 3 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by thakis@chromium.org
, Apr 27 2016