The default bug view has changed. See this FAQ.

gfx/layers fail to compile on mingw

RESOLVED FIXED in mozilla15

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jacek Caban, Assigned: Jacek Caban)

Tracking

Trunk
mozilla15
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

fix
3.53 KB, patch
bas
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Created attachment 603653 [details] [diff] [review]
fix v1.0

I agree that initialization by assignment looks nicer, but it's not really the same thing in standard C++
Attachment #603653 - Flags: review?

Comment 1

5 years ago
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)
(Assignee)

Comment 2

5 years ago
Oh, thanks, I was sure I've set it during filling the bug.
(Assignee)

Comment 3

5 years ago
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)
(Assignee)

Comment 4

5 years ago
Created attachment 625080 [details] [diff] [review]
fix
Attachment #603653 - Attachment is obsolete: true
Attachment #625080 - Flags: review?(ncameron)
Attachment #625080 - Flags: review?(ncameron) → review+
(Assignee)

Comment 5

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.