Last Comment Bug 702877 - Remove NS_OVERRIDE and replace all uses of it with MOZ_OVERRIDE
: Remove NS_OVERRIDE and replace all uses of it with MOZ_OVERRIDE
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Joshua Cranmer [:jcranmer]
:
Mentors:
: 727279 (view as bug list)
Depends on: 702437 817387
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-15 21:29 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2012-12-02 06:40 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Replace all uses in mozilla-central (165.87 KB, patch)
2012-07-13 07:10 PDT, Joshua Cranmer [:jcranmer]
no flags Details | Diff | Review
Version that passes tinderbox (166.55 KB, patch)
2012-07-13 13:14 PDT, Joshua Cranmer [:jcranmer]
ehsan: review+
Details | Diff | Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-15 21:29:15 PST
NS_OVERRIDE and MOZ_OVERRIDE both indicate the same thing: that the virtual function member being annotated must override a virtual function member on a base class.  MOZ_OVERRIDE abstracts away new C++11 goodness and actually has teeth in some recent compilers.  NS_OVERRIDE is a user attribute and has no teeth except in static analysis builds, and I don't even know what the status of those is any more.

Probably static analysis can look for MOZ_OVERRIDE in new enough gcc/clang compilers (4.7+ and 3.0+, respectively, but note gcc 4.7 isn't released yet), so NS_OVERRIDE and all uses of it should be removed and replaced with MOZ_OVERRIDE.
Comment 1 Joshua Cranmer [:jcranmer] 2011-11-15 22:25:51 PST
Some notes here:
1. NS_OVERRIDE is generally done before the method declaration, whereas the override keyword would have to be after the method declaration. So all uses of NS_OVERRIDE need to be moved around.

2. It should be possible to alias MOZ_OVERRIDE to the __attribute__((user("NS_override"))) version in sufficiently old (i.e., gcc 4.6-or-less) builds.
Comment 2 Joshua Cranmer [:jcranmer] 2012-07-13 07:10:53 PDT
Created attachment 641861 [details] [diff] [review]
Replace all uses in mozilla-central

This kills all uses of NS_OVERRIDE and deletes the macro to make sure no one else tries to use it again.
Comment 3 Joshua Cranmer [:jcranmer] 2012-07-13 13:14:23 PDT
Created attachment 642018 [details] [diff] [review]
Version that passes tinderbox

Silly me, why did I assume it worked on all platforms just because it worked on my computer?
Comment 4 Justin Lebar (not reading bugmail) 2012-07-23 07:36:09 PDT
*** Bug 727279 has been marked as a duplicate of this bug. ***
Comment 5 Joshua Cranmer [:jcranmer] 2012-07-23 13:55:12 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/950048a58302
Comment 6 Ed Morley [:emorley] 2012-07-24 02:59:26 PDT
https://hg.mozilla.org/mozilla-central/rev/950048a58302
Comment 7 Marco Castelluccio [:marco] 2012-09-03 14:35:30 PDT
There's still xpcom/analysis/override.js that refers to NS_OVERRIDE.
Comment 8 :Ehsan Akhgari (out sick) 2012-09-06 19:04:11 PDT
(In reply to comment #7)
> There's still xpcom/analysis/override.js that refers to NS_OVERRIDE.

Good catch.  I think that entire script should be removed.  Can you please file a bug on that?  Thanks!

Note You need to log in before you can comment on or make changes to this bug.