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

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

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

+++ This bug was initially created as a clone of Bug #1482002 +++

Not fixed. See Bug 1482002 comment 11.

Comment 1

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

Attachment: newbug - add nsCaseInsensitiveStringComparator default constructor

Comment 2

[Nathan Froyd [:froydnj]]

The problem is that clang 3.8 (and apparently whatever version of Apple's clang is based off of the 3.8-era codebase) was correct about this code when 3.8 was released.  But the standards committee decided that the 3.8-imposed behavior is actually incorrect, revised the standard, and clang 3.9 and above implement the correct (and non-head-scratching) behavior.  (The SO answer cited in bug 1482002 comment 11 is therefore out of date.)

You should upgrade your Xcode.

Comment 3

[Robert Longson [:longsonr]]

§12.1/6 [class.ctor] A default constructor that is defaulted and not defined as deleted is implicitly defined when it is odr-used to create an object of its class type or when it is explicitly defaulted after its first declaration. The implicitly-defined default constructor performs the set of initializations of the class that would be performed by a user-written default constructor for that class with no ctor-initializer (12.6.2) and an empty compound-statement. [...]

So this is a compiler bug still presumably.

Comment 4

[Robert Longson [:longsonr]]

Should we simply insist on clang >= 3.9 rather than failing here?

Comment 5

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

Odd, I'm on Xcode 8.2.1. which https://en.wikipedia.org/wiki/Xcode#Latest_versions claims is 3.9.0...

Comment 6

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

Attachment: add clang 3.8 compatible nsCaseInsensitiveStringComparator default constructor

Rebased patch to apply, in case we decide to take it for clang 3.8 support.

Comment 7

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

Attachment: Fix 'unused variable' clang 3.8 error in PaymentRequestManager.cpp

Fyi I needed this second patch as well to build central with clang-800.0.42.1 today.

I'm biting the bullet and upgrading my Xcode, so I'm just dropping this here in case we still care about clang 3.8 support. 

With my just back from 2 weeks of PTO and bug 1480306, dropping 3.8 seems reasonable to me.

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 9005675 [details] [diff] [review]
add clang 3.8 compatible nsCaseInsensitiveStringComparator default constructor

Given bug 1482196, I think we can just drop this.

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 9005683 [details] [diff] [review]
Fix 'unused variable' clang 3.8 error in PaymentRequestManager.cpp

Given bug 1482196, I think we can just drop this.

Comment 10

[David Durst [:ddurst]]

We'd still like a fix for 63.

Comment 11

[Nathan Froyd [:froydnj]]

(In reply to David Durst [:ddurst] (REO for 63) from comment #10)
> We'd still like a fix for 63.

For avoidance of doubt, you want a fix to land directly on beta, bypassing central (since central will have bug 1482196).  Is this just because the "affected" flag for 63 is set, or something else?

Comment 12

[David Durst [:ddurst]]

(In reply to Nathan Froyd [:froydnj] from comment #11)
> (In reply to David Durst [:ddurst] (REO for 63) from comment #10)
> > We'd still like a fix for 63.
> 
> For avoidance of doubt, you want a fix to land directly on beta, bypassing
> central (since central will have bug 1482196).  Is this just because the
> "affected" flag for 63 is set, or something else?

Based on that, yes (or at least that was the determination in the regression triage). I assume that's still valid?