733708 - gfx/layers fail to compile on mingw

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jacek Caban]

Attachment: fix v1.0

I agree that initialization by assignment looks nicer, but it's not really the same thing in standard C++

Comment 1

[Ali Juma [:ajuma]]

Comment on attachment 603653 [details] [diff] [review]
fix v1.0

Flagging Bas for review since he wrote that code.

Comment 2

[Jacek Caban]

Oh, thanks, I was sure I've set it during filling the bug.

Comment 3

[Jacek Caban]

Comment on attachment 603653 [details] [diff] [review]
fix v1.0

Bas, I saw that you fixed this while working on that code. Thanks for that. Sadly, as part of bug 716439, another change that broke compilation in the same way landed. I will reuse this bug for the new patch.

I've investigated the situation again to see if I can find solution to prevent future breakages. Unfortunately, the answer is no, but let me explain it here for the reference.

The problem is that in statements like this:

nsAutoPtr<ClassName> x = new ClassName();

it's not an usual assignment, it is an initializing assignment. Such assignment, according to the standard, can't have an implicit cast, such as ClassName* -> nsAutoPtr is. That's what GCC complains about in this code. MSVC treats it like two separated statements:

nsAutoPtr<ClassName> x; x = ClassName();

Thus it uses default constructor for initialization and then operator= for the assignment. This difference is not specific to mingw. The same applies to all GCCs. I'm sure it causes occasional problems with all code shared between platforms, but they are caught by Tinderbox. So having more uniform behavior would be desirable. Two solutions I've considered are:

1) Make GCC accept non-standard syntax
I couldn't find a way to make it work.

2) Make MSVC *not* accept this syntax
If we could disable this, that would do the trick. I'm not aware of anything we could put into nsAutoPtr that would prohibit such syntax. It's possible to disable *all* MSVC C++ extensions by a compiler switch:

http://msdn.microsoft.com/en-us/library/0k0w269d%28v=vs.71%29.aspx

but it's not possible to do it per-feature. It solvers this particular problem, but the switch either all or nothing. And "all" isn't an option - even Microsoft's own platform headers fail to compile with this option.


So... there is no other way than fixing this stuff manually once it shows up.

Comment 4

[Jacek Caban]

Attachment: fix

Comment 5

[Jacek Caban]

Thanks for review.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2dbd906a4324

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2dbd906a4324