From bug 449561, bug 445949, and others: let's just ditch nsDerivedSafe since the class of errors it's trying to protect against are uncommon in today's world. r=dbaron
authorBenjamin Smedberg <benjamin@smedbergs.us>
Mon, 11 Aug 2008 11:05:58 -0400
changeset 16558 2cdac4c2a0238475b2598847df9818d8589d6bb2
parent 16557 0d6458ad345d22361108dffa12a8853fb959d24b
child 16559 e10d489d473f89094b0a2b3505ed9fb0371a8a18
push id1131
push userbsmedberg@mozilla.com
push dateMon, 11 Aug 2008 15:31:14 +0000
treeherdermozilla-central@2cdac4c2a023 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs449561, 445949
milestone1.9.1a2pre
From bug 449561, bug 445949, and others: let's just ditch nsDerivedSafe since the class of errors it's trying to protect against are uncommon in today's world. r=dbaron
editor/libeditor/base/IMETextTxn.h
editor/libeditor/base/InsertTextTxn.h
netwerk/base/src/nsStandardURL.cpp
xpcom/base/nsAutoPtr.h
xpcom/glue/nsCOMPtr.h
xpcom/tests/TestCOMPtrEq.cpp
xpcom/tests/TestProxies.cpp
xpcom/tests/TestTimers.cpp
--- a/editor/libeditor/base/IMETextTxn.h
+++ b/editor/libeditor/base/IMETextTxn.h
@@ -115,14 +115,11 @@ protected:
   nsCOMPtr<nsIPrivateTextRangeList>	mRangeList;
 
   /** the selection controller, which we'll need to get the selection */
   nsWeakPtr mSelConWeak;  // use a weak reference
 
   PRBool	mFixed;
 
   friend class TransactionFactory;
-
-  friend class nsDerivedSafe<IMETextTxn>; // work around for a compiler bug
-
 };
 
 #endif
--- a/editor/libeditor/base/InsertTextTxn.h
+++ b/editor/libeditor/base/InsertTextTxn.h
@@ -99,14 +99,11 @@ protected:
 
   /** the text to insert into mElement at mOffset */
   nsString mStringToInsert;
 
   /** the editor, which we'll need to get the selection */
   nsIEditor *mEditor;   
 
   friend class TransactionFactory;
-
-  friend class nsDerivedSafe<InsertTextTxn>; // work around for a compiler bug
-
 };
 
 #endif
--- a/netwerk/base/src/nsStandardURL.cpp
+++ b/netwerk/base/src/nsStandardURL.cpp
@@ -1502,24 +1502,19 @@ nsStandardURL::SetPath(const nsACString 
 }
 
 NS_IMETHODIMP
 nsStandardURL::Equals(nsIURI *unknownOther, PRBool *result)
 {
     NS_ENSURE_ARG_POINTER(unknownOther);
     NS_PRECONDITION(result, "null pointer");
 
-    nsRefPtr<nsStandardURL> otherPtr;
+    nsRefPtr<nsStandardURL> other;
     nsresult rv = unknownOther->QueryInterface(kThisImplCID,
-                                               getter_AddRefs(otherPtr));
-
-    // Hack around issue with MSVC++ not allowing the nsDerivedSafe to access
-    // the private members and not doing the implicit conversion to a raw
-    // pointer.
-    nsStandardURL* other = otherPtr;
+                                               getter_AddRefs(other));
     if (NS_FAILED(rv)) {
         *result = PR_FALSE;
         return NS_OK;
     }
 
     // First, check whether one URIs is an nsIFileURL while the other
     // is not.  If that's the case, they're different.
     if (mSupportsFileURL != other->mSupportsFileURL) {
@@ -1567,17 +1562,17 @@ nsStandardURL::Equals(nsIURI *unknownOth
             LOG(("nsStandardURL::Equals [this=%p spec=%s] failed to ensure file",
                 this, mSpec.get()));
             return rv;
         }
         NS_ASSERTION(mFile, "EnsureFile() lied!");
         rv = other->EnsureFile();
         if (NS_FAILED(rv)) {
             LOG(("nsStandardURL::Equals [other=%p spec=%s] other failed to ensure file",
-                other, other->mSpec.get()));
+                 other.get(), other->mSpec.get()));
             return rv;
         }
         NS_ASSERTION(other->mFile, "EnsureFile() lied!");
         return mFile->Equals(other->mFile, result);
     }
 
     // The URLs are not identical, and they do not correspond to the
     // same file, so they are different.
--- a/xpcom/base/nsAutoPtr.h
+++ b/xpcom/base/nsAutoPtr.h
@@ -37,17 +37,17 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef nsAutoPtr_h___
 #define nsAutoPtr_h___
 
   // Wrapping includes can speed up compiles (see "Large Scale C++ Software Design")
 #ifndef nsCOMPtr_h___
-  // For |already_AddRefed|, |nsDerivedSafe|, |NSCAP_Zero|,
+  // For |already_AddRefed|, |NSCAP_Zero|,
   // |NSCAP_DONT_PROVIDE_NONCONST_OPEQ|,
   // |NSCAP_FEATURE_INLINE_STARTASSIGNMENT|
 #include "nsCOMPtr.h"
 #endif
 
 /*****************************************************************************/
 
 // template <class T> class nsAutoPtrGetterTransfers;
@@ -1051,42 +1051,41 @@ class nsRefPtr
           NS_ASSERTION(rhs, "Null pointer passed to forget!");
           *rhs = 0;
           swap(*rhs);
         }
 
       T*
       get() const
           /*
-            Prefer the implicit conversion provided automatically by |operator nsDerivedSafe<T>*() const|.
-             Use |get()| to resolve ambiguity or to get a castable pointer.
-
-            Returns a |nsDerivedSafe<T>*| to deny clients the use of |AddRef| and |Release|.
+            Prefer the implicit conversion provided automatically by |operator T*() const|.
+            Use |get()| to resolve ambiguity or to get a castable pointer.
           */
         {
           return const_cast<T*>(mRawPtr);
         }
 
-      operator nsDerivedSafe<T>*() const
+      operator T*() const
           /*
-            ...makes an |nsRefPtr| act like its underlying raw pointer type (except against |AddRef()|, |Release()|,
-              and |delete|) whenever it is used in a context where a raw pointer is expected.  It is this operator
-              that makes an |nsRefPtr| substitutable for a raw pointer.
+            ...makes an |nsRefPtr| act like its underlying raw pointer type whenever it
+            is used in a context where a raw pointer is expected.  It is this operator
+            that makes an |nsRefPtr| substitutable for a raw pointer.
 
-            Prefer the implicit use of this operator to calling |get()|, except where necessary to resolve ambiguity.
+            Prefer the implicit use of this operator to calling |get()|, except where
+            necessary to resolve ambiguity.
           */
         {
-          return get_DerivedSafe();
+          return get();
         }
 
-      nsDerivedSafe<T>*
+      T*
       operator->() const
         {
           NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL nsRefPtr with operator->().");
-          return get_DerivedSafe();
+          return get();
         }
 
 #ifdef CANT_RESOLVE_CPP_CONST_AMBIGUITY
   // broken version for IRIX
 
       nsRefPtr<T>*
       get_address() const
           // This is not intended to be used by clients.  See |address_of|
@@ -1111,42 +1110,33 @@ class nsRefPtr
           // below.
         {
           return this;
         }
 
 #endif // CANT_RESOLVE_CPP_CONST_AMBIGUITY
 
     public:
-      nsDerivedSafe<T>&
+      T&
       operator*() const
         {
           NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL nsRefPtr with operator*().");
-          return *get_DerivedSafe();
+          return *get();
         }
 
       T**
       StartAssignment()
         {
 #ifndef NSCAP_FEATURE_INLINE_STARTASSIGNMENT
           return reinterpret_cast<T**>(begin_assignment());
 #else
           assign_assuming_AddRef(0);
           return reinterpret_cast<T**>(&mRawPtr);
 #endif
         }
-
-    private:
-      nsDerivedSafe<T>*
-      get_DerivedSafe() const
-        {
-          return const_cast<nsDerivedSafe<T>*>
-                           (reinterpret_cast<const nsDerivedSafe<T>*>(mRawPtr));
-        }
-      
   };
 
 #ifdef CANT_RESOLVE_CPP_CONST_AMBIGUITY
 
 // This is the broken version for IRIX, which can't handle the version below.
 
 template <class T>
 inline
--- a/xpcom/glue/nsCOMPtr.h
+++ b/xpcom/glue/nsCOMPtr.h
@@ -169,91 +169,23 @@
 
 
 
   /*
     WARNING:
       VC++4.2 is very picky.  To compile under VC++4.2, the classes must be defined
       in an order that satisfies:
     
-        nsDerivedSafe < nsCOMPtr
         already_AddRefed < nsCOMPtr
         nsCOMPtr < nsGetterAddRefs
 
       The other compilers probably won't complain, so please don't reorder these
       classes, on pain of breaking 4.2 compatibility.
   */
 
-
-template <class T>
-class
-  NS_FINAL_CLASS
-  NS_STACK_CLASS
-nsDerivedSafe : public T
-    /*
-      No client should ever see or have to type the name of this class.  It is the
-      artifact that makes it a compile-time error to call |AddRef| and |Release|
-      on a |nsCOMPtr|.  DO NOT USE THIS TYPE DIRECTLY IN YOUR CODE.
-
-      See |nsCOMPtr::operator->|, |nsCOMPtr::operator*|, et al.
-
-      This type should be a nested class inside |nsCOMPtr<T>|.
-    */
-  {
-    private:
-#ifdef HAVE_CPP_ACCESS_CHANGING_USING
-      using T::AddRef;
-      using T::Release;
-#else
-      nsrefcnt AddRef(void);
-      nsrefcnt Release(void);
-#endif
-
-#if !defined(AIX) && !defined(IRIX)
-      void operator delete( void*, size_t );                  // NOT TO BE IMPLEMENTED
-        // declaring |operator delete| private makes calling delete on an interface pointer a compile error
-#endif
-
-      nsDerivedSafe<T>& operator=( const T& );                // NOT TO BE IMPLEMENTED
-        // you may not call |operator=()| through a dereferenced |nsCOMPtr|, because you'd get the wrong one
-
-        /*
-          Compiler warnings and errors: nsDerivedSafe operator=() hides inherited operator=().
-          If you see that, that means somebody checked in a [XP]COM interface class that declares an
-          |operator=()|, and that's _bad_.  So bad, in fact, that this declaration exists explicitly
-          to stop people from doing it.
-        */
-
-    protected:
-      nsDerivedSafe();                                        // NOT TO BE IMPLEMENTED
-        /*
-          This ctor exists to avoid compile errors and warnings about nsDeriviedSafe using the
-          default ctor but inheriting classes without an empty ctor. See bug 209667.
-        */
-  };
-
-#if !defined(HAVE_CPP_ACCESS_CHANGING_USING) && defined(NEED_CPP_UNUSED_IMPLEMENTATIONS)
-template <class T>
-nsrefcnt
-nsDerivedSafe<T>::AddRef()
-  {
-    return 0;
-  }
-
-template <class T>
-nsrefcnt
-nsDerivedSafe<T>::Release()
-  {
-    return 0;
-  }
-
-#endif
-
-
-
 template <class T>
 struct already_AddRefed
     /*
       ...cooperates with |nsCOMPtr| to allow you to assign in a pointer _without_
       |AddRef|ing it.  You might want to use this as a return type from a function
       that produces an already |AddRef|ed pointer as a result.
 
       See also |getter_AddRefs()|, |dont_AddRef()|, and |class nsGetterAddRefs|.
@@ -848,40 +780,41 @@ nsCOMPtr
           NS_ASSERTION(rhs, "Null pointer passed to forget!");
           *rhs = 0;
           swap(*rhs);
         }
 
       T*
       get() const
           /*
-            Prefer the implicit conversion provided automatically by |operator nsDerivedSafe<T>*() const|.
-             Use |get()| to resolve ambiguity or to get a castable pointer.
+            Prefer the implicit conversion provided automatically by |operator T*() const|.
+            Use |get()| to resolve ambiguity or to get a castable pointer.
           */
         {
           return reinterpret_cast<T*>(mRawPtr);
         }
 
-      operator nsDerivedSafe<T>*() const
+      operator T*() const
           /*
-            ...makes an |nsCOMPtr| act like its underlying raw pointer type (except against |AddRef()|, |Release()|,
-              and |delete|) whenever it is used in a context where a raw pointer is expected.  It is this operator
-              that makes an |nsCOMPtr| substitutable for a raw pointer.
+            ...makes an |nsCOMPtr| act like its underlying raw pointer type whenever it
+            is used in a context where a raw pointer is expected.  It is this operator
+            that makes an |nsCOMPtr| substitutable for a raw pointer.
 
-            Prefer the implicit use of this operator to calling |get()|, except where necessary to resolve ambiguity.
+            Prefer the implicit use of this operator to calling |get()|, except where
+            necessary to resolve ambiguity.
           */
         {
-          return get_DerivedSafe();
+          return get();
         }
 
-      nsDerivedSafe<T>*
+      T*
       operator->() const
         {
           NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL nsCOMPtr with operator->().");
-          return get_DerivedSafe();
+          return get();
         }
 
 #ifdef CANT_RESOLVE_CPP_CONST_AMBIGUITY
   // broken version for IRIX
 
       nsCOMPtr<T>*
       get_address() const
           // This is not intended to be used by clients.  See |address_of|
@@ -906,46 +839,33 @@ nsCOMPtr
           // below.
         {
           return this;
         }
 
 #endif // CANT_RESOLVE_CPP_CONST_AMBIGUITY
 
     public:
-      nsDerivedSafe<T>&
+      T&
       operator*() const
         {
           NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL nsCOMPtr with operator*().");
-          return *get_DerivedSafe();
+          return *get();
         }
 
-#if 0
-    private:
-      friend class nsGetterAddRefs<T>;
-#endif
-
       T**
       StartAssignment()
         {
 #ifndef NSCAP_FEATURE_INLINE_STARTASSIGNMENT
           return reinterpret_cast<T**>(begin_assignment());
 #else
           assign_assuming_AddRef(0);
           return reinterpret_cast<T**>(&mRawPtr);
 #endif
         }
-
-    private:
-      nsDerivedSafe<T>*
-      get_DerivedSafe() const
-        {
-          return reinterpret_cast<nsDerivedSafe<T>*>(mRawPtr);
-        }
-
   };
 
 
 
   /*
     Specializing |nsCOMPtr| for |nsISupports| allows us to use |nsCOMPtr<nsISupports>| the
     same way people use |nsISupports*| and |void*|, i.e., as a `catch-all' pointer pointing
     to any valid [XP]COM interface.  Otherwise, an |nsCOMPtr<nsISupports>| would only be able
@@ -1171,40 +1091,42 @@ class nsCOMPtr<nsISupports>
           swap(*rhs);
         }
 
         // Other pointer operators
 
       nsISupports*
       get() const
           /*
-            Prefer the implicit conversion provided automatically by |operator nsDerivedSafe<nsISupports>*() const|.
-             Use |get()| to resolve ambiguity or to get a castable pointer.
+            Prefer the implicit conversion provided automatically by
+            |operator nsISupports*() const|.
+            Use |get()| to resolve ambiguity or to get a castable pointer.
           */
         {
           return reinterpret_cast<nsISupports*>(mRawPtr);
         }
 
-      operator nsDerivedSafe<nsISupports>*() const
+      operator nsISupports*() const
           /*
-            ...makes an |nsCOMPtr| act like its underlying raw pointer type (except against |AddRef()|, |Release()|,
-              and |delete|) whenever it is used in a context where a raw pointer is expected.  It is this operator
-              that makes an |nsCOMPtr| substitutable for a raw pointer.
+            ...makes an |nsCOMPtr| act like its underlying raw pointer type whenever it
+            is used in a context where a raw pointer is expected.  It is this operator
+            that makes an |nsCOMPtr| substitutable for a raw pointer.
 
-            Prefer the implicit use of this operator to calling |get()|, except where necessary to resolve ambiguity.
+            Prefer the implicit use of this operator to calling |get()|, except where
+            necessary to resolve ambiguity.
           */
         {
-          return get_DerivedSafe();
+          return get();
         }
 
-      nsDerivedSafe<nsISupports>*
+      nsISupports*
       operator->() const
         {
           NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL nsCOMPtr with operator->().");
-          return get_DerivedSafe();
+          return get();
         }
 
 #ifdef CANT_RESOLVE_CPP_CONST_AMBIGUITY
   // broken version for IRIX
 
       nsCOMPtr<nsISupports>*
       get_address() const
           // This is not intended to be used by clients.  See |address_of|
@@ -1230,46 +1152,33 @@ class nsCOMPtr<nsISupports>
         {
           return this;
         }
 
 #endif // CANT_RESOLVE_CPP_CONST_AMBIGUITY
 
     public:
 
-      nsDerivedSafe<nsISupports>&
+      nsISupports&
       operator*() const
         {
           NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL nsCOMPtr with operator*().");
-          return *get_DerivedSafe();
+          return *get();
         }
 
-#if 0
-    private:
-      friend class nsGetterAddRefs<nsISupports>;
-#endif
-
       nsISupports**
       StartAssignment()
         {
 #ifndef NSCAP_FEATURE_INLINE_STARTASSIGNMENT
           return reinterpret_cast<nsISupports**>(begin_assignment());
 #else
           assign_assuming_AddRef(0);
           return reinterpret_cast<nsISupports**>(&mRawPtr);
 #endif
         }
-
-    private:
-      nsDerivedSafe<nsISupports>*
-      get_DerivedSafe() const
-        {
-          return reinterpret_cast<nsDerivedSafe<nsISupports>*>(mRawPtr);
-        }
-
   };
 
 #ifndef NSCAP_FEATURE_USE_BASE
 template <class T>
 void
 nsCOMPtr<T>::assign_with_AddRef( nsISupports* rawPtr )
   {
     if ( rawPtr )
--- a/xpcom/tests/TestCOMPtrEq.cpp
+++ b/xpcom/tests/TestCOMPtrEq.cpp
@@ -82,26 +82,26 @@ int
 main()
   {
     nsCOMPtr<nsICOMPtrEqTestFoo> s;
     nsICOMPtrEqTestFoo* r = 0;
     const nsCOMPtr<nsICOMPtrEqTestFoo> sc;
     const nsICOMPtrEqTestFoo* rc = 0;
     nsICOMPtrEqTestFoo* const rk = 0;
     const nsICOMPtrEqTestFoo* const rkc = 0;
-    nsDerivedSafe<nsICOMPtrEqTestFoo>* d = s;
+    nsICOMPtrEqTestFoo* d = s;
     
 #ifdef NSCAP_EQTEST_TEST_ACROSS_TYPES
     nsCOMPtr<nsICOMPtrEqTestFoo2> s2;
     nsICOMPtrEqTestFoo2* r2 = 0;
     const nsCOMPtr<nsICOMPtrEqTestFoo2> sc2;
     const nsICOMPtrEqTestFoo2* rc2 = 0;
     nsICOMPtrEqTestFoo2* const rk2 = 0;
     const nsICOMPtrEqTestFoo2* const rkc2 = 0;
-    nsDerivedSafe<nsICOMPtrEqTestFoo2>* d2 = s2;
+    nsICOMPtrEqTestFoo2* d2 = s2;
 #endif
 
     return (!(PR_TRUE &&
               (s == s) &&
               (s == r) &&
               (s == sc) &&
               (s == rc) &&
               (s == rk) &&
--- a/xpcom/tests/TestProxies.cpp
+++ b/xpcom/tests/TestProxies.cpp
@@ -110,22 +110,22 @@ public:
     void* nativeThread = static_cast<void*>(mNativeThread);
 #endif
 
     LOG(("Shutting down test thread [0x%p]", nativeThread));
     mThread->Shutdown();
     LOG(("Test thread successfully shut down [0x%p]", nativeThread));
   }
 
-  operator nsDerivedSafe<nsIThread>*() const
+  operator nsIThread*() const
   {
     return mThread;
   }
 
-  nsDerivedSafe<nsIThread>* operator->() const
+  nsIThread* operator->() const
   {
     return mThread;
   }
 
 private:
   nsIThread** mGlobal;
   nsCOMPtr<nsIThread> mThread;
   PRThread* mNativeThread;
--- a/xpcom/tests/TestTimers.cpp
+++ b/xpcom/tests/TestTimers.cpp
@@ -60,21 +60,21 @@ public:
 
     newThread.swap(mThread);
   }
 
   ~AutoTestThread() {
     mThread->Shutdown();
   }
 
-  operator nsDerivedSafe<nsIThread>*() const {
+  operator nsIThread*() const {
     return mThread;
   }
 
-  nsDerivedSafe<nsIThread>* operator->() const {
+  nsIThread* operator->() const {
     return mThread;
   }
 
 private:
   nsCOMPtr<nsIThread> mThread;
 };
 
 class AutoCreateAndDestroyMonitor