1482002 - Compilation error in SVGTest.cpp with clang-3.8

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Paul Bone [:pbone]]

Attachment: newbug - add nsCaseInsensitiveStringComparator default constructor

I get the following compilation error with clang-3.8 on Linux.

 0:15.63 In file included from /home/paul/dev/moz/dont-poison/obj-x86_64-pc-linux-gnu/dom/svg/Unified_cpp_dom_svg6.cpp:128:
 0:15.64 /home/paul/dev/moz/dont-poison/dom/svg/SVGTests.cpp:72:43: error: default initialization of an object of const type 'const nsCaseInsensitiveStringComparator' without a user-provided default constructor
 0:15.64   const nsCaseInsensitiveStringComparator caseInsensitiveComparator;
 0:15.64                                           ^
 0:15.64                                                                    {}
 0:15.65 /home/paul/dev/moz/dont-poison/dom/svg/SVGTests.cpp:159:45: error: default initialization of an object of const type 'const nsCaseInsensitiveStringComparator' without a user-provided default constructor
 0:15.65     const nsCaseInsensitiveStringComparator caseInsensitiveComparator;
 0:15.65                                             ^
 0:15.66                                                                      {}
 0:15.67 2 errors generated.

I guess Bug 1480946 created this regression.

Comment 1

[Mike Hommey [:glandium]]

Considering you need at least libclang 3.9 for bindgen, I think we should just drop support for clang < 3.9 completely.

Comment 2

[Paul Bone [:pbone]]

Comment on attachment 8998715 [details] [diff] [review]
newbug - add nsCaseInsensitiveStringComparator default constructor


I'll ask bz to review the change, since it's in a different component. but I want to ni longsonr in case they want some input, it looks like their change in Bug 1480946 created the issue.

Thanks.

Comment 3

[Paul Bone [:pbone]]

(In reply to Mike Hommey [:glandium] from comment #1)
> Considering you need at least libclang 3.9 for bindgen, I think we should
> just drop support for clang < 3.9 completely.

How does that decision get made?  Right now docs say 3.8 and that's what my system has.

https://developer.mozilla.org/en-US/docs/Mozilla/Using_CXX_in_Mozilla_code

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Paul Bone [:pbone] from comment #3)
> (In reply to Mike Hommey [:glandium] from comment #1)
> > Considering you need at least libclang 3.9 for bindgen, I think we should
> > just drop support for clang < 3.9 completely.
> 
> How does that decision get made?  Right now docs say 3.8 and that's what my
> system has.
> 
> https://developer.mozilla.org/en-US/docs/Mozilla/Using_CXX_in_Mozilla_code

You talk to a build peer sort of person, which glandium and myself qualify as. :)

I think we've wanted to drop clang < 3.9 (or even < 5 or so) for a while; I always thought that it was difficult to determine whether the XCode clang was actually supported under that scheme or not.

Comment 5

[Eric Rahm [:erahm]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> (In reply to Paul Bone [:pbone] from comment #3)
> > (In reply to Mike Hommey [:glandium] from comment #1)
> > > Considering you need at least libclang 3.9 for bindgen, I think we should
> > > just drop support for clang < 3.9 completely.
> > 
> > How does that decision get made?  Right now docs say 3.8 and that's what my
> > system has.
> > 
> > https://developer.mozilla.org/en-US/docs/Mozilla/Using_CXX_in_Mozilla_code
> 
> You talk to a build peer sort of person, which glandium and myself qualify
> as. :)
> 
> I think we've wanted to drop clang < 3.9 (or even < 5 or so) for a while; I
> always thought that it was difficult to determine whether the XCode clang
> was actually supported under that scheme or not.

Filed bug 1482196 for this, we've been seeing a fair amount of user reports of bustage on 3.8.

Comment 6

[Eric Rahm [:erahm]]

Comment on attachment 8998715 [details] [diff] [review]
newbug - add nsCaseInsensitiveStringComparator default constructor

Review of attachment 8998715 [details] [diff] [review]:
-----------------------------------------------------------------

::: intl/unicharutil/util/nsUnicharUtils.h
@@ +53,4 @@
>  class nsCaseInsensitiveStringComparator : public nsStringComparator
>  {
>  public:
> +  nsCaseInsensitiveStringComparator() {};

I think we want `= default`.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8998715 [details] [diff] [review]
newbug - add nsCaseInsensitiveStringComparator default constructor

What comment 6 says, please.  r=me with that.

Comment 8

[Paul Bone [:pbone]]

(In reply to Eric Rahm [:erahm] from comment #5)
> 
> Filed bug 1482196 for this, we've been seeing a fair amount of user reports
> of bustage on 3.8.

Thanks, I'll follow that and update also.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d58330c8fbc
add nsCaseInsensitiveStringComparator default constructor r=bz

Comment 10

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/0d58330c8fbc

Comment 11

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

While Paul's original patch in comment 0 works, the fix in comment 10 does not work for me.

With Apple LLVM version 8.0.0 (clang-800.0.42.1) (3.9.0 according to https://en.wikipedia.org/wiki/Xcode), I still get:

13:46.40 In file included from /Users/Jan/moz/mozilla-central/obj-x86_64-apple-darwin12.2.1-opt/dom/svg/Unified_cpp_dom_svg6.cpp:128:
13:46.40 /Users/Jan/moz/mozilla-central/dom/svg/SVGTests.cpp:72:43: error: default initialization of an object of const type 'const nsCaseInsensitiveStringComparator' without a user-provided default constructor
13:46.40   const nsCaseInsensitiveStringComparator caseInsensitiveComparator;
13:46.40                                           ^
13:46.40                                                                    {}
...etc.

According to https://stackoverflow.com/a/28338123/918910 

    A() = default;

does not actually provide a constructor, it does the opposite by telling the compiler that you don't want to provide one, thus the error. 

If I do

    A() {};

It works.

I believe the "default" in "user-provided default constructor" refers to the user-provided (aka non-default) constructor without any arguments.

Too many things called "default" IMHO.

Comment 12

[Robert Longson [:longsonr]]

reopening is reserved for if/when we back out this patch. Please open another bug.