Closed Bug 733708 Opened 10 years ago Closed 9 years ago

gfx/layers fail to compile on mingw

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: jacek, Assigned: jacek)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch fix v1.0 (obsolete) — Splinter Review
I agree that initialization by assignment looks nicer, but it's not really the same thing in standard C++
Attachment #603653 - Flags: review?
Comment on attachment 603653 [details] [diff] [review]
fix v1.0

Flagging Bas for review since he wrote that code.
Attachment #603653 - Flags: review? → review?(bas.schouten)
Oh, thanks, I was sure I've set it during filling the bug.
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.
Attachment #603653 - Flags: review?(bas.schouten)
Attached patch fixSplinter Review
Attachment #603653 - Attachment is obsolete: true
Attachment #625080 - Flags: review?(ncameron)
Attachment #625080 - Flags: review?(ncameron) → review+
Thanks for review.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2dbd906a4324
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/2dbd906a4324
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.