Last Comment Bug 704687 - Add MOZ_FINAL to classes which were NS_FINAL_CLASS but couldn't be MOZ_FINAL when nsDerivedSafe existed
: Add MOZ_FINAL to classes which were NS_FINAL_CLASS but couldn't be MOZ_FINAL ...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-22 17:05 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-12-22 03:46 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch now that nsDerivedSafe is history (4.85 KB, patch)
2011-12-19 14:03 PST, Jeff Walden [:Waldo] (remove +bmo to email)
dbaron: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-22 17:05:11 PST
A few classes were marked as NS_FINAL_CLASS, but it turned out that after static analysis stopped happening, nsDerivedSafe got readded to nsCOMPtr and nsRefPtr, and then some of those "final" classes suddenly became base classes of particular specializations of nsCOMPtr_base<T>::nsDerivedSafe.  MOZ_FINAL replaces NS_FINAL_CLASS, and unlike that macro it actually has teeth in the compilers developers will run with.  Classes that wrongly used NS_FINAL_CLASS have had it removed for now.  In the longer run, however, we should either have a final-but-for-nsDerivedSafe annotation/analysis, or we should remove nsDerivedSafe.

bsmedberg in bug 704127 comment 5 also suggests maybe removing nsDerivedSafe from nsRefPtr alone might be a reasonable way to address this, and to maybe permit those classes to be MOZ_FINAL.

dbaron thinks the remove-nsDerivedSafe/not-mark-those-classes-as-MOZ_FINAL question is a close one.  I'll let people here fight it out, or something.  :-)
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-22 17:20:34 PST
Those few classes were the ones listed in bug 704127 comment 0, and which were changed in the obvious way in this commit:

https://hg.mozilla.org/integration/mozilla-inbound/rev/3edc79afc842
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-19 14:03:27 PST
Created attachment 582948 [details] [diff] [review]
Patch now that nsDerivedSafe is history
Comment 3 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2011-12-19 15:29:39 PST
Comment on attachment 582948 [details] [diff] [review]
Patch now that nsDerivedSafe is history

r=dbaron
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-21 19:05:25 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c0b89927f59
Comment 5 Ed Morley [:emorley] 2011-12-22 03:46:37 PST
https://hg.mozilla.org/mozilla-central/rev/7c0b89927f59

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