756640 - Consider making nsCOMPtr<T> statically assert that T is an abstract class

Status: NEW

(Core :: XPCOM, defect)

Description

[Jeff Walden [:Waldo]]

nsCOMPtr is supposed to be used as a smart pointer for XPCOM interface types.  nsRefPtr is supposed to be used as a smart pointer for concrete classes implementing XPCOM interfaces.  This distinction isn't enforced, so it's possible in some cases to use the wrong class for the task.

MSVC, gcc, and clang all support (I don't know how long for any, but it's easy to #ifdef for it) a built-in __is_abstract macro which can distinguish interface types from concrete types.  Perhaps we should add static assertions to nsCOMPtr and nsRefPtr that verify the proper smart pointer is being used for the particular class that's being instantiated.  The deliberate goal would be to break code misusing the smart pointer classes, so there would be some cleanup needed to do this.  And embedders and such would have to adapt somewhat, albeit only in proportion to how often they misused the classes, and also recognizing that adapting would be a matter of s/COM/Ref/ or vice versa in the affected places.  But it seems like it'd be a nice simplification if we could provide a clear error message explaining the difference, rather than triggering inscrutable compiler errors about ambiguous casts and such.

Comment 1

[Boris Zbarsky [:bzbarsky]]

As I pointed out on IRC, making nsRefPtr require not-abstract would make it pretty impossible to write templates that need to hold strong refs to objects that might or might not be abstract.  Right now those can just use nsRefPtr.

Making nsCOMPtr require abstract is a good idea, though.

Comment 2

[Jonathan Watt [:jwatt]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #1)
> As I pointed out on IRC, making nsRefPtr require not-abstract would make it
> pretty impossible to write templates that need to hold strong refs to
> objects that might or might not be abstract.

Such as OwningNonNull.