Opened on 11/21/2017 at 12:55:54 PM

Closed on 04/18/2018 at 09:07:48 PM

#6063 closed change (fixed)

Start to use C++14

Reported by: sergz Assignee: sergz
Priority: P2 Milestone:
Module: Libadblockplus Keywords:
Cc: asmirnov, hfiguiere Blocked By:
Blocking: #6588 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):



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.

Attachments (0)

Change History (20)

comment:1 Changed on 11/21/2017 at 01:07:17 PM by 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 c++14 is required.

Note that v8 inherits the style guide from chromium, and chromium allows to use C++14 feature:

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

comment:2 follow-up: Changed on 11/21/2017 at 01:10:57 PM by asmirnov

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

comment:3 in reply to: ↑ 2 ; follow-up: Changed on 11/21/2017 at 01:16:46 PM by 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.

comment:4 in reply to: ↑ 3 Changed on 11/21/2017 at 01:20:21 PM by 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.

Last edited on 11/21/2017 at 01:20:34 PM by asmirnov

comment:5 Changed on 12/21/2017 at 12:27:24 PM by sergz

I think we should try to update V8 (according to 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).

comment:6 Changed on 02/02/2018 at 01:52:45 PM by 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:

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.

comment:7 Changed on 03/23/2018 at 06:24:53 AM by asmirnov

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

comment:8 Changed on 03/23/2018 at 10:28:35 AM by sergz

  • Priority changed from P4 to P2

comment:9 Changed on 03/26/2018 at 07:46:51 AM by 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.

comment:10 Changed on 03/26/2018 at 08:05:11 AM by sergz

  • Owner set to 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.

comment:11 Changed on 03/27/2018 at 01:47:26 PM by 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.

comment:12 Changed on 03/27/2018 at 04:31:43 PM by sergz

  • Owner sergz deleted

Good news: it's possible to obtain a binary using NDK r16b, C++14, the version of V8 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.

comment:13 Changed on 03/28/2018 at 11:49:26 AM by 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)?

comment:14 Changed on 04/04/2018 at 06:14:21 PM by 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)

comment:15 Changed on 04/04/2018 at 06:23:07 PM by hfiguiere

  • Cc hfiguiere added

comment:16 Changed on 04/04/2018 at 06:45:34 PM by sergz

  • Owner set to 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.

comment:17 Changed on 04/11/2018 at 07:59:57 AM by sergz

  • Review URL(s) modified (diff)

comment:18 Changed on 04/16/2018 at 07:39:45 AM by asmirnov

  • Blocking 6588 added

comment:19 Changed on 04/18/2018 at 09:07:07 PM by abpbot

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

comment:20 Changed on 04/18/2018 at 09:07:48 PM by sergz

  • Resolution set to fixed
  • Status changed from new to closed

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from sergz.
Note: See TracTickets for help on using tickets.