Start to use C++14

[sergz]

Background

We should at least ensure that our headers are compatible with C++14 in order to give a possibility to use libadblockplus in modern applications. Additionally it would be good have an easy way of compiling the library with different C++ standards.

[asmirnov]

For using libadblockplus-android in Chromium we should compile libadblockplus with exactly the same version of v8 that chromium app uses. At the moment we compile for c++11. Starting recent times (actually starting that change ​https://chromium-review.googlesource.com/c/v8/v8/+/728026) c++14 is required.

Note that v8 inherits the style guide from chromium, and chromium allows to use C++14 feature: ​https://chromium-cpp.appspot.com/.

Probably we could revert some files to still use c++11 but i believe that it would be better to support C++14. BTW I was able to compile chromium with v8 revision e57e7b81b8584ddbd5268b8e7af2988e4f87fc84 that uses C++14.

I've tried to edit common.gypi and replace -std=c++11 with -std=c++14 and 'CLANG_CXX_LANGUAGE_STANDARD': 'c++11' with 'CLANG_CXX_LANGUAGE_STANDARD': 'c++14' but it introduced new build errors:

     /Users/asmirnov/Library/Android/ndk/android-ndk-r12b/sources/cxx-stl/llvm-libc++/libcxx/include/tuple:356:72: error: with 'constexpr _Hp& std::__ndk1::__tuple_leaf<_Ip, _Hp, true>::get() const'
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11       _Hp& get()       _NOEXCEPT {return static_cast<_Hp&>(*this);}

similar issue ​https://github.com/android-ndk/ndk/issues/174

[asmirnov]

BTW it would be great to check compilation with the newest android NDK, which is 16. At the moment we still use 12b.

[sergz]

Replying to asmirnov:

BTW it would be great to check compilation with the newest android NDK, which is 16. At the moment we still use 12b.

This dependency comes from V8.

[asmirnov]

Replying to sergz:

Replying to asmirnov:

BTW it would be great to check compilation with the newest android NDK, which is 16. At the moment we still use 12b.

This dependency comes from V8.

Yeah, i can see android_tools in Chromium still having NDK 12b so probably that's the limit.

[sergz]

I think we should try to update V8 (according to ​https://omahaproxy.appspot.com/ we can do it again, the prev update was #5698) to a version containing required changes and either at the same time or right after the update switch to C++14 if it's possible (the latter means supported by the compiler from the ndk).

[asmirnov]

64.0.3249.2 is the last version of Chromium where we can build libadblockplus while building of Chromium, it uses V8 of 6.4.102 .version.

In Chromium 64.0.3250.0 v8 of version 6.4.126 is used and it requires C++14 to be supported.
Actually C++ is required starting V8 of version 6.4.107: ​https://chromium.googlesource.com/v8/v8.git/+/refs/heads/6.4.107.

So if we we'd like to update to Chromium 64.0.3250.0 this ticket will be the blocker.

However we could probably solve it by avoid building of V8 for libadblockplus as we can just reuse Chromium V8 build files. For this we need #5503 to be solved.

[asmirnov]

Now it's blocking some of our partners to use Chromium 65.

[sergz]

The code of libadblockplus, abpshell and tests can be compiled with -std=c++14 (modified common.gypi) on linux.

I suppose that it also works with Visual C++ since we are using relatively recent platform toolsets, e.g. v141. If we decide to limit ourselves to, e.g. C++14, then please pay attention to the compiler flag ​/std:c++14 what can be specified in common.gypi as (not tested)

'msvs_settings': {
  'VCCLCompilerTool': { 'AdditionalOptions': ['/std:c++14'], },
},

The support of C++14 by V8 and by android ndk is not tested yet.

[sergz]

I'm observing the same issue as in comment:1 trying to compile by ndk, no matter only libadblockplus or v8.

So, one need to see whether it will be resolved by updating of V8.

[sergz]

When one selects -std=c++14 then some features are not supported by the compiler from the ndk r12b. So it seems that the only option is to switch to a new ndk, presumable r16b.

Basically the everything can be compiled by r16b but there are some linking errors. I'm looking into it.

Additionally there is a non-standard linker chromium project, so one has to check that that linker is able to link the code compiled by r16b and that it is still properly working.

[sergz]

Good news: it's possible to obtain a binary using NDK r16b, C++14, the version of V8 6.5.254.41 and gyp, though with several manipulations (The most tricky one was to drop the linker param -Wl,--fatal-warnings). And this binary is actually running on a device.

So, since we know that the code is compatible with the compiler, IMO now we should rather focus on the building system, namely prioritize looking into using GN to build V8 than to create hacks in the current building system, if there is time for that.

---
I'm unassigned myself so far, we will firstly discuss the priorities, because my involvement in this issue was rather an ad-hoc.

[asmirnov]

so what should be changed in gyp files to compile it with make android_arm with NDK 16b (except passing correct path to NDK)?

[hfiguiere]

the path to the stdlib have changed in the NDK 16. standalone.gypi need to be changed.

For example:

'android_libcpp_include': '<(android_stl)/llvm-libc++/libcxx/include',
'android_libcpp_abi_include': '<(android_stl)/llvm-libc++abi/libcxxabi/include',

needs to be changed to

'android_libcpp_include': '<(android_stl)/llvm-libc++/include',
'android_libcpp_abi_include': '<(android_stl)/llvm-libc++abi/include',

(probably not the only place)

[sergz]

Yep, I will firstly send the required changes to some repository and then we will see.
BTW, the same changes should be done even in .gn files.

[abpbot]

A commit referencing this issue has landed:
​Issue 6063, 6531 - Update V8 to 6.5.254.41, use C++14 and NDK-r16b