831279 - b2g18 branch fails to build with clang: "ipc_channel_posix.cc:136:55: error: invalid use of non-static data member 'sun_path'" and other errors

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Justin Lebar (not reading bugmail)]

This is the b2g version of bug 806609.

When building debug desktop Firefox on Linux with clang from a clean version of the b2g18 tree, I get

../../../src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc:136:55: error: invalid use of non-static data member 'sun_path'

Comment 1

[Justin Lebar (not reading bugmail)]

From bug 806609, I understand that the key configure tests are

> checking for gcc c++0x headers bug without rtti...
> checking for usable char16_t (2 bytes, unsigned)...

I'm getting yes/no here, while I need to get no/yes.  Here are the relevant bits from my config.log:

>configure:10346: checking for gcc c++0x headers bug without rtti
>configure:10358: clang++ -Qunused-arguments -c  -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -Qunused-arguments  conftest.C 1>&5
>In file included from configure:10352:
>In file included from /usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/memory:63:
>In file included from /usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_algobase.h:65:
>In file included from /usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_pair.h:61:
>In file included from /usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/move.h:57:
>/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/type_traits:256:39: error: use of undeclared identifier '__float128'
>    struct __is_floating_point_helper<__float128>
>                                      ^
>1 error generated.
>configure: failed program was:
>#line 10351 "configure"
>#include "confdefs.h"
>#include <memory>
>int main() {
>
>; return 0; }

>configure:10378: checking for usable char16_t (2 bytes, unsigned)
>configure:10394: clang++ -Qunused-arguments -c  -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -Qunused-arguments  conftest.C 1>&5
>configure:10390:32: error: use of undeclared identifier 'char16_t'
>CONFIGURE_STATIC_ASSERT(sizeof(char16_t) == 2);

Comment 2

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

I think this is llvm.org/pr13530.

Comment 3

[Justin Lebar (not reading bugmail)]

Pulling the fix to ipc_channel_posix [1] doesn't fix this entirely.  When I do that, I then get compile errors in the HTML5 parser on nullptr:

In file included from ../../../src/parser/html/nsHtml5NamedCharactersAccel.cpp:10:
In file included from ../../../src/parser/html/nsHtml5NamedCharactersAccel.h:13:
../../../src/parser/html/jArray.h:105:22: error: expected parameter declarator
    typedef decltype(nullptr) jArray_nullptr_t;
                     ^
../../dist/include/mozilla/NullPtr.h:38:21: note: expanded from macro 'nullptr'
#    define nullptr __null

[1] http://src.chromium.org/viewvc/chrome/trunk/src/ipc/ipc_channel_posix.cc?view=diff&r1=60130&r2=60131

Comment 4

[Mike Hommey [:glandium]]

(In reply to Justin Lebar [:jlebar] from comment #1)
> From bug 806609, I understand that the key configure tests are
> 
> > checking for gcc c++0x headers bug without rtti...
> > checking for usable char16_t (2 bytes, unsigned)...
> 
> I'm getting yes/no here, while I need to get no/yes.

You don't /need/ to. clang in c++98 mode is still a valid mode.

And Rafael is right, it's a clang/libstdc++ 4.7 combination bug.

Comment 5

[Justin Lebar (not reading bugmail)]

> You don't /need/ to. clang in c++98 mode is still a valid mode.

Understood.  :)

> And Rafael is right, it's a clang/libstdc++ 4.7 combination bug.

Okay.  So how should we fix this?

Comment 6

[Mike Hommey [:glandium]]

(In reply to Justin Lebar [:jlebar] from comment #3)
> Pulling the fix to ipc_channel_posix [1] doesn't fix this entirely.  When I
> do that, I then get compile errors in the HTML5 parser on nullptr:
> 
> In file included from
> ../../../src/parser/html/nsHtml5NamedCharactersAccel.cpp:10:
> In file included from
> ../../../src/parser/html/nsHtml5NamedCharactersAccel.h:13:
> ../../../src/parser/html/jArray.h:105:22: error: expected parameter
> declarator
>     typedef decltype(nullptr) jArray_nullptr_t;
>                      ^

That would be a separate bug in jArray.h.

Comment 7

[Justin Lebar (not reading bugmail)]

I'm a bit confused about what is and isn't a bug here.

* Is it a bug if our source doesn't compile with clang in c++98 mode?  If so, it sounds like jArray.h and ipc_channel_posix.cc both have bugs.  Perhaps other modules as well.

* I guess it's /also/ a bug that we're not turning on c++11 mode for clang?

Comment 8

[Mike Hommey [:glandium]]

(In reply to Justin Lebar [:jlebar] from comment #7)
> I'm a bit confused about what is and isn't a bug here.
> 
> * Is it a bug if our source doesn't compile with clang in c++98 mode?  If
> so, it sounds like jArray.h and ipc_channel_posix.cc both have bugs. 
> Perhaps other modules as well.

Yes

> * I guess it's /also/ a bug that we're not turning on c++11 mode for clang?

Yes, and short of awful hacks, there's nothing we can do about it. You can try using libc++ instead of libstdc++.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #8)
> (In reply to Justin Lebar [:jlebar] from comment #7)
> > I'm a bit confused about what is and isn't a bug here.
> > 
> > * Is it a bug if our source doesn't compile with clang in c++98 mode?  If
> > so, it sounds like jArray.h and ipc_channel_posix.cc both have bugs. 
> > Perhaps other modules as well.
> 
> Yes

Although both compile for me on m-c with clang in c++98 mode.

Comment 10

[Justin Lebar (not reading bugmail)]

The ipc_channel_posix.cc issue was introduced in bug 798354 and backed out soon thereafter.  But the landing and backout spanned the Oct 8 merge.

It sounds like we should port the backout to b2g18.

Comment 11

[Justin Lebar (not reading bugmail)]

>> * Is it a bug if our source doesn't compile with clang in c++98 mode?
>
> Yes.

Okay; let's make this bug about getting our source to compile in that mode, and we can worry about clang c++11 (or not) separately.

> Although both compile for me on m-c with clang in c++98 mode.

On the b2g18 branch?

Comment 12

[Justin Lebar (not reading bugmail)]

And the jArray fix is probably bug 796925.

Comment 13

[Mike Hommey [:glandium]]

The jArray.h error is probably bug 796925.

Comment 14

[Justin Lebar (not reading bugmail)]

Attachment: Back out changeset 9adf5ca922a4 (bug 798354)

Already backed out on m-c, so I don't need approval; only a review.

Comment 15

[Justin Lebar (not reading bugmail)]

> Already backed out on m-c, so I don't need approval; only a review.

Erm, I mean, I don't need a review; only approval.  :)

Comment 16

[Justin Lebar (not reading bugmail)]

Comment on attachment 702827 [details] [diff] [review]
Back out changeset 9adf5ca922a4 (bug 798354)

[Approval Request Comment]
* Bug caused by (feature/regressing bug #): Bug 798354
* User impact if declined: Developers can't build the b2g18 branch with clang without this patch.
* Testing completed: Already landed on m-c.
Risk to taking this patch (and alternatives if risky): Trivial change.
String or UUID changes made by this patch: None.

Comment 17

[Justin Lebar (not reading bugmail)]

Another error.  Boy, this is fun.

> In file included from ../../../src/layout/base/nsCSSFrameConstructor.cpp:239:
> ../../../src/layout/base/../xul/base/src/nsListItemFrame.h:31:34: error: implicit conversion of NULL > constant to 'bool' [-Werror,-Wnull-conversion]

Comment 18

[Justin Lebar (not reading bugmail)]

For the one in comment 17, we need

http://hg.mozilla.org/mozilla-central/rev/978e9865042f4be110e719742d17a7d5480ec2b3

Comment 19

[Justin Lebar (not reading bugmail)]

Attachment: Backport hg cset 978e9865042

This is enough to get it to compile for me.

Comment 20

[Justin Lebar (not reading bugmail)]

Comment on attachment 702906 [details] [diff] [review]
Backport hg cset 978e9865042

[Approval Request Comment]
Backport a simple fix from upstream.  Low risk compared to the current breakage.  No string changes, etc.

Comment 21

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/068ec3fe8da9
https://hg.mozilla.org/releases/mozilla-b2g18/rev/115b09d80438