MOZ_OVERRIDE in method definition breaks compilation for C++11

RESOLVED FIXED in mozilla24

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: _AtilA_, Assigned: _AtilA_)

Tracking

Trunk
mozilla24
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Created attachment 751042 [details] [diff] [review]
Removes MOZ_OVERRIDE from method definition.

C++11 specs says that the virt-specifiers ("override" and "final") shall only appear in the declaration of a virtual member function. But TextureHostOGL.h has a MOZ_OVERRIDE (so override) after SetBuffer(..) declaracion, causing gcc-4.7 with C++11 to break compliation.
(Assignee)

Updated

4 years ago
Assignee: nobody → atilag
(Assignee)

Updated

4 years ago
Attachment #751042 - Flags: review?(matt.woodrow)
Attachment #751042 - Flags: review?(matt.woodrow) → review+
(In reply to Juan Gomez [:_AtilA_] from comment #0)
> C++11 specs says that the virt-specifiers ("override" and "final") shall
> only appear in the declaration of a virtual member function. But
> TextureHostOGL.h has a MOZ_OVERRIDE (so override) after SetBuffer(..)
> declaracion, causing gcc-4.7 with C++11 to break compliation.

To be clear, this is TextureHostOGL.cpp (not .h), and the faulty MOZ_OVERRIDE in question appears after the method *definition* (not the declaration), and that's the problem.

(correct me if I'm wrong)
Blocks: 862324
Version: unspecified → Trunk
Juan: could you attach an updated version of the patch with a commit message (including r=mattwoodrow) and with the "user" patch-header included so that the patch is correctly attributed to you?

See this page for how to do that, if you're not sure how:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

After that, you should be able to mark this bug as "checkin-needed", and then someone can come along and trivially import & land the patch on your behalf.
Status: NEW → ASSIGNED
Flags: needinfo?(atilag)
(Assignee)

Comment 3

4 years ago
Created attachment 751681 [details] [diff] [review]
Removes vir-specifier MOZ_OVERRIDE in method definition
Attachment #751042 - Attachment is obsolete: true
Flags: needinfo?(atilag)
(Assignee)

Comment 4

4 years ago
(In reply to Daniel Holbert [:dholbert] from comment #1)
> (In reply to Juan Gomez [:_AtilA_] from comment #0)
> > C++11 specs says that the virt-specifiers ("override" and "final") shall
> > only appear in the declaration of a virtual member function. But
> > TextureHostOGL.h has a MOZ_OVERRIDE (so override) after SetBuffer(..)
> > declaracion, causing gcc-4.7 with C++11 to break compliation.
> 
> To be clear, this is TextureHostOGL.cpp (not .h), and the faulty
> MOZ_OVERRIDE in question appears after the method *definition* (not the
> declaration), and that's the problem.
> 
> (correct me if I'm wrong)

Oh! I confused things a little, but you are right!.
I submitted a new patch using git forma-patch, it has author information and a verbose commit message. I read this format is importable by Mercurial, if not, I'll try hg.
Comment on attachment 751042 [details] [diff] [review]
Removes MOZ_OVERRIDE from method definition.

[un-obsoleting the original r+'d patch]

In cases like this one where you've got a r+'d patch and you're attaching an updated version for someone else to checkin, it's best *not* to obsolete the r+'d patch, to make it more immediately-clear that the patch has indeed been reviewed.

(And then perhaps add "(for checkin)" to the description field for the actual ready-to-land patch, to make it clear which version is intended for checkin)
Attachment #751042 - Attachment is obsolete: false
(Assignee)

Comment 6

4 years ago
Created attachment 751689 [details] [diff] [review]
patch for checkin (same as r+'d patch, now with patch headers)

Well formatted commit message.
Attachment #751042 - Attachment is obsolete: true
Attachment #751681 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #751689 - Attachment description: Removes vir-specifier MOZ_OVERRIDE in method definition → patch for checkin (same as r+'d patch, now with patch headers)
(Assignee)

Updated

4 years ago
Attachment #751042 - Attachment is obsolete: false
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc64829a0cba
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dc64829a0cba
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.