Re: c++/libc++ help needed for chromium update

2016-03-19 Thread Dimitry Andric 
On 17 Mar 2016, at 12:38, Christoph Moench-Tegeder  wrote:
...
>> Last but not least, please ask about this on the Chromium mailing lists.
>> There must be lots of C++ libraries out there with non-trivial std::pair
>> copy constructors, and they must have some sort of workaround for those.
> 
> It's in the nature of chromium that it's Open Source, but the list of
> target platforms is rather short - officially supported are Windows
> (and I don't know how many people build their own chromium there),
> OS X and Linux. Chromium tends to bundle a lot of dependencies, I
> believe the default build process on Linux even brings it's own
> private clang installation if you don't stop it. The only other
> "special" build I'm aware of is OpenBSD, and they use gcc 4.9
> with it's bundled libstdc++ (their base system uses the old gcc).
> So I guess we're the odd ones with our libc++...

Not really, OSX also uses libc++ these days.  However, they use the
trivial pair copy constructor by default, as upstream libc++ does.  See
/Applications/Xcode.app/Contents//Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__config
on a Mac with Xcode installed, which has:

   650  #ifndef _LIBCPP_TRIVIAL_PAIR_COPY_CTOR
   651  #  define _LIBCPP_TRIVIAL_PAIR_COPY_CTOR 1
   652  #endif

So not having the trivial pair copy constructor makes us the odd ones
out, but until now I didn't see any port failing because of it.  We
could enable this feature, but then we'd have to bump our libc++
version, together with all the followup hassle.

It would still be interesting to find some workaround in Chrome's code,
though.  It used to build in the previous version?  (I remember building
it successfully a few times in the past, at least.)

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: c++/libc++ help needed for chromium update

2016-03-19 Thread Dimitry Andric 
On 16 Mar 2016, at 22:27, Christoph Moench-Tegeder  wrote:
> 
> I'm currently working on updating www/chromium to the latest
> version (the 49 versions). Recently (that is, during development
> of version 49) the chromium developers allowed some c++11 features
> to be used.
> Included in those is std::move(), replacing their homegrown
> rvalue-reference-generating code (the .Pass() hack). That
> required me to use lang/clang36 as a compiler (with our
> clang 3.4.1 in FreeBSD 10 I got a bunch of obviously "wrong
> errors").
> 
> The following is my interpretation, grain-of-salt applies.
> 
> There's one issue remaining: this line
> https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/sync/internal_api/public/data_batch_impl.cc#15
> causes an compiler error - complaining that a copy-assignment
> operator had to be used but the operator had been deleted (error
> message attached, it's a little unwieldy).
> 
> The scoped_ptr implementation is this one:
> https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/base/memory/scoped_ptr.h
> and the magic DISALLOW_COPY_AND_ASSIGN_WITH_MOVE_FOR_BIND()
> has been defined here:
> https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/base/move.h#35
> 
> The key_data_pairs_ thing is a std::vector
> https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/sync/internal_api/public/data_batch_impl.h#41
> while KeyAndData is a std::pair<>
> https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/sync/api/data_batch.h#18
> 
> (I think you get the hang of it...)
> 
> As clang is the "official" compiler used upstream, and after some
> research into C++ land (I'm from the plain C side), I'm suspecting
> that our libc++ is at fault (and it's behind upstream, of course,
> but there's no "simple" way of importing a new one - devel/libc++
> leaves the templates in /usr/include alone). Build results on
> FreeBSD 11 are still... building
> 
> Could anyone point me in a direction to resolve this?

While you are waiting for the FreeBSD 11 builds to complete, there are
maybe a few things you could try.

In r261801 [1], we turned off the _LIBCPP_TRIVIAL_PAIR_COPY_CTOR option
in libc++, because it made the ABI incompatible with the version of
libc++ we had shipped before that time.  However, this option makes the
std::pair copy constructor non-trivial, and this is a possible cause for
your compilation error.

The relevant part in  is:

template 
struct _LIBCPP_TYPE_VIS_ONLY pair
{
[...]
#if !defined(_LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS) && 
_LIBCPP_TRIVIAL_PAIR_COPY_CTOR
_LIBCPP_INLINE_VISIBILITY
pair(const pair& __p) = default;
#elif !defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES) || 
!_LIBCPP_TRIVIAL_PAIR_COPY_CTOR
_LIBCPP_INLINE_VISIBILITY
pair(const pair& __p)
_NOEXCEPT_(is_nothrow_copy_constructible::value &&
   is_nothrow_copy_constructible::value)
: first(__p.first),
  second(__p.second)
{
}
#endif

E.g. in the case of a trivial copy constructor, it is created with "=
default", but otherwise it uses the copy constructors of the 'first' and
'second' members.  If these copy constructors are inaccessible or
deleted, as it looks like in this case, you will get an error about it.

The first thing you could try is to compile the Chromium source with
-D_LIBCPP_TRIVIAL_PAIR_COPY_CTOR=1 added to the flags.  This will
re-enable the trivial copy constructor for std::pair, but it is possibly
incompatible with precompiled libc++.so on your system.  Therefore,
things might break at runtime, in interesting ways.

Another thing you could try is to change the definition of scoped_ptr,
and comment out the deletion of its copy constructor.  However, I'm
unsure what side effects this may cause, as it is probably unwise to
copy scoped_ptrs.

Last but not least, please ask about this on the Chromium mailing lists.
There must be lots of C++ libraries out there with non-trivial std::pair
copy constructors, and they must have some sort of workaround for those.

-Dimitry

[1] http://svnweb.freebsd.org/changeset/base/261801



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: c++/libc++ help needed for chromium update

2016-03-19 Thread Dimitry Andric 
On 16 Mar 2016, at 23:36, Dimitry Andric  wrote:
> 
> On 16 Mar 2016, at 22:27, Christoph Moench-Tegeder  
> wrote:
...
>> Could anyone point me in a direction to resolve this?
...
> Last but not least, please ask about this on the Chromium mailing lists.
> There must be lots of C++ libraries out there with non-trivial std::pair
> copy constructors, and they must have some sort of workaround for those.

Yet another thing you could try is changing DataBatchImpl::Put() as follows:

--- a/data_batch_impl.cc2016-03-16 23:43:50.0 +0100
+++ b/data_batch_impl.cc2016-03-16 23:44:01.0 +0100
@@ -12,7 +12,7 @@ DataBatchImpl::~DataBatchImpl() {}

 void DataBatchImpl::Put(const std::string& client_key,
 scoped_ptr specifics) {
-  key_data_pairs_.push_back(KeyAndData(client_key, std::move(specifics)));
+  key_data_pairs_.push_back(std::move(KeyAndData(client_key, 
std::move(specifics;
 }

 bool DataBatchImpl::HasNext() const {

I'm not 100% sure this will work, but it might... :)

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: c++/libc++ help needed for chromium update

2016-03-19 Thread David Chisnall 
On 18 Mar 2016, at 17:22, Dimitry Andric  wrote:
> 
> We
> could enable this feature, but then we'd have to bump our libc++
> version, together with all the followup hassle.

There are quite a few ABI-breaking changes in libc++.  Some improve standards 
compliance and a few improve performance.  I’d like for us to enable them all 
(with associated so version bump) in 11.  At this point, it might make sense to 
do the same for libc++ in ports and move ports over to depending on the ports 
version on <11.

David

___
freebsd-toolchain@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-toolchain
To unsubscribe, send any mail to "freebsd-toolchain-unsubscr...@freebsd.org"

Re: c++/libc++ help needed for chromium update

2016-03-19 Thread Christoph Moench-Tegeder 
## Dimitry Andric (d...@freebsd.org):

> In r261801 [1], we turned off the _LIBCPP_TRIVIAL_PAIR_COPY_CTOR option
> in libc++, because it made the ABI incompatible with the version of
> libc++ we had shipped before that time.  However, this option makes the
> std::pair copy constructor non-trivial, and this is a possible cause for
> your compilation error.

Ah, that sounds plausible. Let's see what the compiler says :)

> Another thing you could try is to change the definition of scoped_ptr,
> and comment out the deletion of its copy constructor.  However, I'm
> unsure what side effects this may cause, as it is probably unwise to
> copy scoped_ptrs.

Additionally, the scoped_ptrs are used widely throughout chromium,
I'm not sure I'd be able to "fix" all the places.

> Last but not least, please ask about this on the Chromium mailing lists.
> There must be lots of C++ libraries out there with non-trivial std::pair
> copy constructors, and they must have some sort of workaround for those.

It's in the nature of chromium that it's Open Source, but the list of
target platforms is rather short - officially supported are Windows
(and I don't know how many people build their own chromium there),
OS X and Linux. Chromium tends to bundle a lot of dependencies, I
believe the default build process on Linux even brings it's own
private clang installation if you don't stop it. The only other
"special" build I'm aware of is OpenBSD, and they use gcc 4.9
with it's bundled libstdc++ (their base system uses the old gcc).
So I guess we're the odd ones with our libc++...

Regards,
Christoph

-- 
Spare Space
___
freebsd-toolchain@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-toolchain
To unsubscribe, send any mail to "freebsd-toolchain-unsubscr...@freebsd.org"

c++/libc++ help needed for chromium update

2016-03-19 Thread Christoph Moench-Tegeder 
Hi,
I'm currently working on updating www/chromium to the latest
version (the 49 versions). Recently (that is, during development
of version 49) the chromium developers allowed some c++11 features
to be used.
Included in those is std::move(), replacing their homegrown
rvalue-reference-generating code (the .Pass() hack). That
required me to use lang/clang36 as a compiler (with our
clang 3.4.1 in FreeBSD 10 I got a bunch of obviously "wrong
errors").

The following is my interpretation, grain-of-salt applies.

There's one issue remaining: this line
https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/sync/internal_api/public/data_batch_impl.cc#15
causes an compiler error - complaining that a copy-assignment
operator had to be used but the operator had been deleted (error
message attached, it's a little unwieldy).

The scoped_ptr implementation is this one:
https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/base/memory/scoped_ptr.h
and the magic DISALLOW_COPY_AND_ASSIGN_WITH_MOVE_FOR_BIND()
has been defined here:
https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/base/move.h#35

The key_data_pairs_ thing is a std::vector
https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/sync/internal_api/public/data_batch_impl.h#41
while KeyAndData is a std::pair<>
https://chromium.googlesource.com/chromium/src.git/+/49.0.2623.87/sync/api/data_batch.h#18

(I think you get the hang of it...)

As clang is the "official" compiler used upstream, and after some
research into C++ land (I'm from the plain C side), I'm suspecting
that our libc++ is at fault (and it's behind upstream, of course,
but there's no "simple" way of importing a new one - devel/libc++
leaves the templates in /usr/include alone). Build results on
FreeBSD 11 are still... building

Could anyone point me in a direction to resolve this?

Regards,
Christoph
-- 
Spare Space
FAILED: /usr/local/bin/clang++36 -MMD -MF 
obj/sync/internal_api/public/sync_core.data_batch_impl.o.d 
-DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -D_FILE_OFFSET_BITS=64 -DNO_TCMALLOC 
-DDISABLE_NACL -DCHROMIUM_BUILD -DCR_CLANG_REVISION=255169-1 -DUSE_AURA=1 
-DUSE_ASH=1 -DUSE_PANGO=1 -DUSE_CAIRO=1 -DUSE_DEFAULT_RENDER_THEME=1 
-DUSE_LIBJPEG_TURBO=1 -DUSE_X11=1 -DUSE_CLIPBOARD_AURAX11=1 
-DENABLE_ONE_CLICK_SIGNIN -DENABLE_WEBRTC=1 -DENABLE_MEDIA_ROUTER=1 
-DUSE_PROPRIETARY_CODECS -DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS 
-DDONT_EMBED_BUILD_METADATA -DFIELDTRIAL_TESTING_ENABLED 
-DENABLE_TASK_MANAGER=1 -DENABLE_EXTENSIONS=1 -DENABLE_PDF=1 -DENABLE_PLUGINS=1 
-DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_AUTOFILL_DIALOG=1 
-DENABLE_BACKGROUND=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 
-DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 
-DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 -DENABLE_SETTINGS_APP=1 
-DENABLE_SUPERVISED_USERS=1 -DENABLE_MDNS=1 -DENABLE_SERVICE_DISCOVERY=1 
-DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL 
-DSYNC_IMPLEMENTATION -DU_USING_ICU_NAMESPACE=0 -DPROTOBUF_USE_DLLS 
-DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER 
-DUSE_LIBPCI=1 -DUSE_OPENSSL=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DNDEBUG -DNVALGRIND 
-DDYNAMIC_ANNOTATIONS_ENABLED=0 -D_FORTIFY_SOURCE=2 
-Igen/shim_headers/snappy/target -Igen/shim_headers/re2/target 
-Igen/shim_headers/icuuc/target -Igen/shim_headers/icui18n/target 
-Igen/shim_headers/libevent/target -Igen -I../.. 
-I../../third_party/leveldatabase/src/include 
-I../../third_party/leveldatabase/src -I../../third_party/leveldatabase 
-I../../third_party/protobuf -I../../third_party/protobuf/src 
-I../../third_party/zlib -Igen/protoc_out -fstack-protector 
--param=ssp-buffer-size=4  -pthread -fno-strict-aliasing -Wall -Wextra 
-Wno-unused-parameter -Wno-missing-field-initializers -fvisibility=hidden -pipe 
-fPIC -fcolor-diagnostics -Wheader-hygiene -Wfor-loop-analysis 
-Wno-char-subscripts -Wno-unneeded-internal-declaration 
-Wno-covered-switch-default -Wstring-conversion -Wno-c++11-narrowing 
-Wno-deprecated-register -Wno-inconsistent-missing-override 
-Wno-shift-negative-value -Wexit-time-destructors -I/usr/local/include/glib-2.0 
-I/usr/local/lib/glib-2.0/include -I/usr/local/include -pthread 
-I/usr/local/include -I/usr/local/include -I/usr/local/include/nss 
-I/usr/local/include/nss/nss -I/usr/local/include/nspr -Wno-header-guard -m64 
-march=x86-64 -O2 -fno-ident -fdata-sections -ffunction-sections 
-funwind-tables -O2 -pipe -march=core2 -isystem/usr/local/include 
-I/usr/local/include/atk-1.0 -Wno-unknown-warning-option -fstack-protector 
-fno-strict-aliasing -fno-exceptions -fno-rtti -fno-threadsafe-statics 
-fvisibility-inlines-hidden -std=gnu++11  -c 
../../sync/internal_api/public/data_batch_impl.cc -o 
obj/sync/internal_api/public/sync_core.data_batch_impl.o
In file included from ../../sync/internal_api/public/data_batch_impl.cc:5:
In file included from