873486 - MOZ_OVERRIDE in method definition breaks compilation for C++11

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Juan Gomez [:_AtilA_] (CET/CEST)]

Attachment: 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.

Comment 1

[Daniel Holbert [:dholbert]]

(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)

Comment 2

[Daniel Holbert [:dholbert]]

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.

Comment 3

[Juan Gomez [:_AtilA_] (CET/CEST)]

Attachment: Removes vir-specifier MOZ_OVERRIDE in method definition

Comment 4

[Juan Gomez [:_AtilA_] (CET/CEST)]

(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 5

[Daniel Holbert [:dholbert]]

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)

Comment 6

[Juan Gomez [:_AtilA_] (CET/CEST)]

Attachment: patch for checkin (same as r+'d patch, now with patch headers)

Well formatted commit message.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc64829a0cba

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dc64829a0cba