Add MOZ_FINAL to classes which were NS_FINAL_CLASS but couldn't be MOZ_FINAL when nsDerivedSafe existed

RESOLVED FIXED in mozilla12

Status

()

Core
General
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

unspecified
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.  :-)
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
Created attachment 582948 [details] [diff] [review]
Patch now that nsDerivedSafe is history
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #582948 - Flags: review?(dbaron)
Summary: Add a static analysis for classes which are final but for nsDerivedSafe, or remove nsDerivedSafe entirely → Add MOZ_FINAL to classes which were NS_FINAL_CLASS but couldn't be MOZ_FINAL when nsDerivedSafe existed
Comment on attachment 582948 [details] [diff] [review]
Patch now that nsDerivedSafe is history

r=dbaron
Attachment #582948 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c0b89927f59
Target Milestone: --- → mozilla12

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/7c0b89927f59
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.