Disable scoped_refptr operator T* on Linux.

base/memory/ref_counted.h

Index: base/memory/ref_counted.h
diff --git a/base/memory/ref_counted.h b/base/memory/ref_counted.h
index 96329cb1f49954adb375d82617019ee067f76c30..5a6a76e9a4d85c79423b6b9b87c281e41d10c6b9 100644
--- a/base/memory/ref_counted.h
+++ b/base/memory/ref_counted.h
@@ -15,6 +15,10 @@
 #endif
 #include "base/threading/thread_collision_warner.h"
 
+#if defined(OS_LINUX)
+#define DISABLE_SCOPED_REFPTR_CONVERSION_OPERATOR
+#endif
+
 namespace base {
 
 namespace subtle {
@@ -291,9 +295,11 @@ class scoped_refptr {
 
   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);
@@ -335,6 +341,24 @@ class scoped_refptr {
     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_;
 };
@@ -346,4 +370,43 @@ 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_