Disable scoped_refptr operator T* on Linux.

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 "base/atomic_ref_count.h"
 #include "base/base_export.h"
 #include "base/compiler_specific.h"
 #ifndef NDEBUG
 #include "base/logging.h"
 #endif
 #include "base/threading/thread_collision_warner.h"
 
+#if defined(OS_LINUX)
+#define DISABLE_SCOPED_REFPTR_CONVERSION_OPERATOR
+#endif
+
 namespace base {
 
 namespace subtle {
 
 class BASE_EXPORT RefCountedBase {
  public:
   bool HasOneRef() const { return ref_count_ == 1; }
 
  protected:
   RefCountedBase()
@@ skipping 256 matching lines @@
       ptr_->AddRef();
   }
 
   ~scoped_refptr() {
     if (ptr_)
       ptr_->Release();
   }
 
   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_;
   }
@@ skipping 21 matching lines @@
   void swap(T** pp) {
     T* p = ptr_;
     ptr_ = *pp;
     *pp = p;
   }
 
   void swap(scoped_refptr<T>& r) {
     swap(&r.ptr_);
   }
 
+#if defined(DISABLE_SCOPED_REFPTR_CONVERSION_OPERATOR)

[jamesr, 2014/08/28 18:19:42]

c++11 have these operators defined for shared_ptr<> so we should probably just
define them without guards. it seems weird but that's life

[dcheng, 2014/08/28 18:28:39]

I need to wrap them in this guard, because otherwise, resolving the right
overload for == would be ambiguous.

+  template <typename U>
+  bool operator==(const scoped_refptr<U>& rhs) const {
+    return ptr_ == rhs.get();
+  }
+
+  template <typename U>
+  bool operator!=(const scoped_refptr<U>& rhs) const {
+    return !operator==(rhs);
+  }
+
+  // STL is fun.

[jamesr, 2014/08/28 18:19:43]

comment isn't useful

[dcheng, 2014/08/28 18:28:39]

Done.

+  template <typename U>
+  bool operator<(const scoped_refptr<U>& rhs) const {
+    return ptr_ < rhs.get();
+  }
+#endif
+
  protected:
   T* ptr_;
 };
 
 // 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>
+bool operator==(const scoped_refptr<T>& lhs, const U* rhs) {

[jamesr, 2014/08/28 18:19:42]

i'd really prefer not to add this and instead insert .get() where needed. do you
have an exact count of how many places would need these? how many can be handled
by a machine translation?  this is a very tricky API to add and it'll be hard to
remove once we add it

[dcheng, 2014/08/28 18:28:40]

I'll try to get a more accurate count once there aren't as many other compile
failures. It's difficult to get an exact measure when I still need to apply
several hundred lines of manual fixups.

+  return lhs.get() == rhs;
+}
+
+template <typename T, typename U>
+bool operator==(const T* lhs, const scoped_refptr<U>& rhs) {
+  return lhs == rhs.get();
+}
+
+template <typename T, typename U>
+bool operator!=(const scoped_refptr<T>& lhs, const U* rhs) {
+  return !operator==(lhs, rhs);
+}
+
+template <typename T, typename U>
+bool operator!=(const T* lhs, const scoped_refptr<U>& rhs) {
+  return !operator==(lhs, rhs);
+}

[Ryan Sleevi, 2014/08/28 17:32:20]

For comparison, shared_ptr (C++11) defines
==
!=
<
<=
(For comparing between shared<T> and shared<U>)

1) Why no < for *?
2) Why no >, <=, >= for scoped<U> (line 345)

[dcheng, 2014/08/28 17:37:15]

It wasn't needed for things to compile. I think only things like
std::map/std::set end up using this, which only needs <. I can certainly add it
if it's desired (I know shared_ptr defines all these), but I opted to only add
what was necessary.

+
+// It turns out the following compiles:
+// scoped_ptr<Foo> a;
+// Foo b;
+// CHECK_EQ(a, b);
+// which is somewhat surprising, since there's no operator<<(std::ostream&, ...)
+// defined. It turns out that this magically works because scoped_ptr is
+// testable--so MakeCheckOpString actually prints the boolean truth value of the
+// scoped_ptr. As a temporary hack, overload operator<< for scoped_refptr to
+// match the fact that scoped_refptr will eventually be testable and this will
+// "Just Work" for some very twisted definiton of "Just Working".

[jamesr, 2014/08/28 18:19:42]

this comment isn't terribly useful

[dcheng, 2014/08/28 18:28:39]

I've removed the comment (it was to try to document why I used bool coercion
below)

+template <typename T>
+BASE_EXPORT std::ostream& operator<<(std::ostream& out,

[jamesr, 2014/08/28 18:19:42]

EXPORT on an inline function makes no sense. there's no symbol in base.dll to
export

[dcheng, 2014/08/28 18:28:39]

Done.

+                                     const scoped_refptr<T>& p) {

[Ryan Sleevi, 2014/08/28 17:32:20]

shared_ptr defines in 20.7.2.2.11, so seems reasonable.

[dcheng, 2014/08/28 17:37:15]

In that case, I won't bother matching scoped_ptr's behavior then? Printing 0 vs
1 is pretty useless =)

[jamesr, 2014/08/28 18:19:43]

Can't you just rely on the fallback byte-by-byte printer in gtest?  That works
for every type and doesn't require us to compile this template-bloaty stream
code into every translation unit that #includes ref_counted (which will be
almost all of them)

[dcheng, 2014/08/28 18:28:40]

This is for the logging macros (CHECK/DCHECK) and friends.

+  return out << !!p.get();
+}
+#endif  // defined(DISABLE_SCOPED_REFPTR_CONVERSION_OPERATOR)
+
 #endif  // BASE_MEMORY_REF_COUNTED_H_