Fix compilation on Android with GNU STL #596
Conversation
|
✅ Build benchmark 1229 completed (commit 23da59f4fb by @Maratyszcza) |
|
we use c++11 throughout benchmark. perhaps it's worth adding a platform check for this, if it's unique to Android? |
|
I think it is Android-specific problem: its GNU STL library mostly, but not fully C++11-compliant, and I haven't seen similar issues on other platforms. That said, Android is a non-negligible platform. |
|
I think it may be interesting to define which versions Android suffer from this issue. |
|
Android STL lives in Android NDK, apps include pre-compiled STL along with their own code. The problem fixed by this patch is present since ancient times (since C++ got support in Android NDK) and up to and including Android NDK r17. |
|
there are some semantic differences between the old and new methods. The newer std:: versions will throw exceptions on error, while the old versions will return 0 (or 0.0) and set I think it may be worth wrapping them in a method that uses |
da3bb26 to
3bc3c22
Compare
|
@dominichamon Please take another look |
|
❌ Build benchmark 1230 failed (commit c60a676d19 by @Maratyszcza) |
|
✅ Build benchmark 1231 completed (commit 22d87ec06b by @Maratyszcza) |
src/string_util.h
Outdated
| return ss.str(); | ||
| } | ||
|
|
||
| #if defined(__ANDROID__) && defined(__GLIBCXX__) |
There was a problem hiding this comment.
can you define a BENCHMARK_OS_ANDROID where we define the other OS constants (src/internal_macros.h)?
i'm not sure we currently differentiate on libcxx but maybe we should add something there too.
src/string_util.h
Outdated
| * strtol, strtod. Note that reimplemented functions are in benchmark:: | ||
| * namespace, not std:: namespace. | ||
| */ | ||
| inline unsigned long stoul(const std::string& str, size_t* pos = nullptr, |
There was a problem hiding this comment.
why inline and why in the header?
3bc3c22 to
92acab0
Compare
|
@dominichamon Added |
|
✅ Build benchmark 1232 completed (commit 2bce6f714b by @Maratyszcza) |
| } | ||
| } | ||
|
|
||
| #ifdef BENCHMARK_STL_ANDROID_GNUSTL |
There was a problem hiding this comment.
can you add tests for these please?
you'll have to create a test/string_util_gtest.cc file, and add it to test/CMakeLists.txt. bazel will pick it up automatically.
There was a problem hiding this comment.
Added tests and replaced log2(n) with 1.44269504088896340736 * log(n) as Android GNUSTL lacks that too.
92acab0 to
cac8645
Compare
GNU STL in Android NDK lacks string conversion functions from C++11, including std::stoul, std::stoi, and std::stod. This patch reimplements these functions in benchmark:: namespace using C-style equivalents from C++03.
cac8645 to
526500c
Compare
|
✅ Build benchmark 1269 completed (commit 9cdd297ed3 by @Maratyszcza) |
src/complexity.cc
Outdated
| case oLogN: | ||
| return [](int64_t n) { return log2(n); }; | ||
| /* Note: can't use log2 because Android's GNU STL lacks it */ | ||
| return [](int64_t n) { return 1.44269504088896340736 * log(n); }; |
There was a problem hiding this comment.
can you at least add a kLog2E constant?
There was a problem hiding this comment.
@dominichamon done
9d64806 to
23c7137
Compare
|
❌ Build benchmark 1270 failed (commit f7d2f8bcfa by @Maratyszcza) |
GNU STL in Android NDK lacks log2 function from C99/C++11. This patch replaces their use in the code with double log(double) function.
23c7137 to
b9b7b87
Compare
|
✅ Build benchmark 1271 completed (commit 7ff6cca59d by @Maratyszcza) |
|
✅ Build benchmark 1272 completed (commit 9099b20361 by @Maratyszcza) |
|
(note that gnustl is being removed from NDK r18.) |
* Fix compilation on Android with GNU STL GNU STL in Android NDK lacks string conversion functions from C++11, including std::stoul, std::stoi, and std::stod. This patch reimplements these functions in benchmark:: namespace using C-style equivalents from C++03. * Avoid use of log2 which doesn't exist in Android GNU STL GNU STL in Android NDK lacks log2 function from C99/C++11. This patch replaces their use in the code with double log(double) function.
GNU STL in Android NDK lacks string conversion functions from C++11, including
std::stoul,std::stoi, andstd::stod. This patch replaces calls to these functions with C-style equivalents from C++03.