scoped_refptr: support opaque pointed-to classes via extern template

base/memory/ref_counted.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_MEMORY_REF_COUNTED_H_
 #define BASE_MEMORY_REF_COUNTED_H_
 
 #include <cassert>
 #include <iosfwd>
 
@@ skipping 258 matching lines @@
 template <class T>
 class scoped_refptr {
  public:
   typedef T element_type;
 
   scoped_refptr() : ptr_(NULL) {
   }
 
   scoped_refptr(T* p) : ptr_(p) {
     if (ptr_)
-      ptr_->AddRef();
+      AddRef(ptr_);
   }
 
   scoped_refptr(const scoped_refptr<T>& r) : ptr_(r.ptr_) {
     if (ptr_)
-      ptr_->AddRef();
+      AddRef(ptr_);
   }
 
   template <typename U>
   scoped_refptr(const scoped_refptr<U>& r) : ptr_(r.get()) {
     if (ptr_)
-      ptr_->AddRef();
+      AddRef(ptr_);
   }
 
   ~scoped_refptr() {
     if (ptr_)
-      ptr_->Release();
+      Release(ptr_);
   }
 
   T* get() const { return ptr_; }
 
 #if !defined(DISABLE_SCOPED_REFPTR_CONVERSION_OPERATOR)
   // Allow scoped_refptr<C> to be used in boolean expression
   // and comparison operations.
   operator T*() const { return ptr_; }
 #endif
 
   T& operator*() const {
     assert(ptr_ != NULL);
     return *ptr_;
   }
 
   T* operator->() const {
     assert(ptr_ != NULL);
     return ptr_;
   }
 
   scoped_refptr<T>& operator=(T* p) {
     // AddRef first so that self assignment should work
     if (p)
-      p->AddRef();
+      AddRef(p);
     T* old_ptr = ptr_;
     ptr_ = p;
     if (old_ptr)
-      old_ptr->Release();
+      Release(old_ptr);
     return *this;
   }
 
   scoped_refptr<T>& operator=(const scoped_refptr<T>& r) {
     return *this = r.ptr_;
   }
 
   template <typename U>
   scoped_refptr<T>& operator=(const scoped_refptr<U>& r) {
     return *this = r.get();
@@ skipping 21 matching lines @@
   }
 
   template <typename U>
   bool operator<(const scoped_refptr<U>& rhs) const {
     return ptr_ < rhs.get();
   }
 #endif
 
  protected:
   T* ptr_;
+
+ private:
+  // Non-inline helpers to allow:
+  //     class Opaque;
+  //     extern template class scoped_refptr<Opaque>;
+  // Otherwise the compiler will complain that Opaque is an incomplete type.

[danakj, 2014/10/09 00:06:57]

Can you add a test that demonstrates this working?

[mdempsky, 2014/10/09 00:21:33]

Sure.  Do you know if there are any existing idioms for testing
cross-compilation-unit behavior I can follow?

My inclination is to write something like:

  ref_counted_unittest_helper.h:
    class OpaqueHelper;
    scoped_refptr<OpaqueHelper> MakeOpaque();
    void UseOpaque(scoped_refptr<OpaqueHelper>);
    extern template class scoped_refptr<OpaqueHelper>;

  ref_counted_unittest_helper.cc:
    class OpaqueHelper : public base::RefCounted<OpaqueHelper> {
      ...
    };
    scoped_refptr<OpaqueHelper> MakeOpaque() {
      return new OpaqueHelper();
    }
    void UseOpaque(scoped_refptr<OpaqueHelper> p) {
      ...
    }
    template class scoped_refptr<Opaque>;

  ref_counted_unittest.cc:
    TEST(RefCountedUnitTest, ScopedRefPtrToOpaque) {
      scoped_refptr<OpaqueHelper> p = MakeOpaque();
      UseOpaque(p);

      scoped_refptr<OpaqueHelper> q;
      q = p;
      UseOpaque(p);
      UseOpaque(q);
    }

and add the new _helper files to only the test target.  Does that sound
reasonable?

[danakj, 2014/10/09 00:31:47]

That sounds like how I'd do it, putting the helper files in base/test/. But is
extern template class still needed to make this compile after this change?

[mdempsky, 2014/10/09 00:52:48]

SGTM
Yes. The "extern template class" declaration informs the compiler it can assume
another compilation unit will instantiate that template class; otherwise, it has
to conservatively instantiate all needed templates itself, and would complain
that ptr->AddRef() is dereferencing a pointer to an incomplete type.

+  static void AddRef(T* ptr);
+  static void Release(T* ptr);
 };
 
+template <typename T>
+void scoped_refptr<T>::AddRef(T* ptr) {
+  ptr->AddRef();

[danakj, 2014/10/09 00:06:57]

What does the code look like with/out this change in an optimized build? Is
there any difference? (I expect not but please check.)

+}
+
+template <typename T>
+void scoped_refptr<T>::Release(T* ptr) {
+  ptr->Release();
+}
+
 // Handy utility for creating a scoped_refptr<T> out of a T* explicitly without
 // having to retype all the template arguments
 template <typename T>
 scoped_refptr<T> make_scoped_refptr(T* t) {
   return scoped_refptr<T>(t);
 }
 
 #if defined(DISABLE_SCOPED_REFPTR_CONVERSION_OPERATOR)
 // Temporary operator overloads to facilitate the transition...
 template <typename T, typename U>
@@ skipping 16 matching lines @@
   return !operator==(lhs, rhs);
 }
 
 template <typename T>
 std::ostream& operator<<(std::ostream& out, const scoped_refptr<T>& p) {
   return out << p.get();
 }
 #endif  // defined(DISABLE_SCOPED_REFPTR_CONVERSION_OPERATOR)
 
 #endif  // BASE_MEMORY_REF_COUNTED_H_