Make operators on scoped_ptr match the ones defined for std::unique_ptr

base/memory/scoped_ptr.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.
 
 // Scopers help you manage ownership of a pointer, helping you easily manage a
 // pointer within a scope, and automatically destroying the pointer at the end
 // of a scope.  There are two main classes you will use, which correspond to the
 // operators new/delete and new[]/delete[].
 //
 // Example usage (scoped_ptr<T>):
@@ skipping 307 matching lines @@
   // Constructor.  Defaults to initializing with nullptr.
   scoped_ptr() : impl_(nullptr) {}
 
   // Constructor.  Takes ownership of p.
   explicit scoped_ptr(element_type* p) : impl_(p) {}
 
   // Constructor.  Allows initialization of a stateful deleter.
   scoped_ptr(element_type* p, const D& d) : impl_(p, d) {}
 
   // Constructor.  Allows construction from a nullptr.
-  scoped_ptr(decltype(nullptr)) : impl_(nullptr) {}
+  scoped_ptr(std::nullptr_t n) : impl_(nullptr) {}

[dcheng, 2015/11/12 16:17:03]

Apparently the style rule around naming parameters changed this year (see
b/19617501 for the details).
http://google.github.io/styleguide/cppguide.html#Function_Declarations_and_De...
now says:

  Parameter names may be omitted only if the parameter is unused and its purpose
is obvious.

So I think we should omit the name here and elsewhere. I don't see any specific
rules in the Chromium style about this (nor do I think we should add any).

[danakj, 2015/11/13 22:19:41]

Done.

[dcheng, 2015/11/13 22:25:55]

Another random thing I just thought of: should we prefer to include cstddef
instead of stddef.h for nullptr_t?

[danakj, 2015/11/13 22:35:23]

I don't think stddef.h does the right thing (it doesn't put it in std::?) I
tried to use it in another CL and it was bad but cstddef worked).

I guess this file already includes stddef.h, but it must end up getting
std::nullptr_t from somewhere else?

 
   // Constructor.  Allows construction from a scoped_ptr rvalue for a
   // convertible type and deleter.
   //
   // IMPLEMENTATION NOTE: C++11 unique_ptr<> keeps this constructor distinct
   // from the normal move constructor. By C++11 20.7.1.2.1.21, this constructor
   // has different post-conditions if D is a reference type. Since this
   // implementation does not support deleters with reference type,
   // we do not need a separate move constructor allowing us to avoid one
   // use of SFINAE. You only need to care about this if you modify the
@@ skipping 16 matching lines @@
   // scoped_ptr.
   template <typename U, typename V>
   scoped_ptr& operator=(scoped_ptr<U, V>&& rhs) {
     COMPILE_ASSERT(!base::is_array<U>::value, U_cannot_be_an_array);
     impl_.TakeState(&rhs.impl_);
     return *this;
   }
 
   // operator=.  Allows assignment from a nullptr. Deletes the currently owned
   // object, if any.
-  scoped_ptr& operator=(decltype(nullptr)) {
+  scoped_ptr& operator=(std::nullptr_t n) {
     reset();
     return *this;
   }
 
   // Reset.  Deletes the currently owned object, if any.
   // Then takes ownership of a new object, if given.
   void reset(element_type* p = nullptr) { impl_.reset(p); }
 
   // Accessors to get the owned object.
   // operator* and operator-> will assert() if there is no current object.
@@ skipping 20 matching lines @@
   // to Testable and then do the comparison).
  private:
   typedef base::internal::scoped_ptr_impl<element_type, deleter_type>
       scoped_ptr::*Testable;
 
  public:
   operator Testable() const {
     return impl_.get() ? &scoped_ptr::impl_ : nullptr;
   }
 
-  // Comparison operators.
-  // These return whether two scoped_ptr refer to the same object, not just to
-  // two different but equal objects.
-  bool operator==(const element_type* p) const { return impl_.get() == p; }
-  bool operator!=(const element_type* p) const { return impl_.get() != p; }
-
   // Swap two scoped pointers.
   void swap(scoped_ptr& p2) {
     impl_.swap(p2.impl_);
   }
 
   // Release a pointer.
   // The return value is the current pointer held by this object. If this object
   // holds a nullptr, the return value is nullptr. After this operation, this
   // object will hold a nullptr, and will not own the object any more.
   element_type* release() WARN_UNUSED_RESULT {
     return impl_.release();
   }
 
  private:
   // Needed to reach into |impl_| in the constructor.
   template <typename U, typename V> friend class scoped_ptr;
   base::internal::scoped_ptr_impl<element_type, deleter_type> impl_;
 
   // Forbidden for API compatibility with std::unique_ptr.
   explicit scoped_ptr(int disallow_construction_from_null);
-
-  // Forbid comparison of scoped_ptr types.  If U != T, it totally
-  // doesn't make sense, and if U == T, it still doesn't make sense
-  // because you should never have the same object owned by two different
-  // scoped_ptrs.
-  template <class U> bool operator==(scoped_ptr<U> const& p2) const;
-  template <class U> bool operator!=(scoped_ptr<U> const& p2) const;
 };
 
 template <class T, class D>
 class scoped_ptr<T[], D> {
   MOVE_ONLY_TYPE_WITH_MOVE_CONSTRUCTOR_FOR_CPP_03(scoped_ptr)
 
  public:
   // The element and deleter types.
   typedef T element_type;
   typedef D deleter_type;
 
   // Constructor.  Defaults to initializing with nullptr.
   scoped_ptr() : impl_(nullptr) {}
 
   // Constructor. Stores the given array. Note that the argument's type
   // must exactly match T*. In particular:
   // - it cannot be a pointer to a type derived from T, because it is
   //   inherently unsafe in the general case to access an array through a
   //   pointer whose dynamic type does not match its static type (eg., if
   //   T and the derived types had different sizes access would be
   //   incorrectly calculated). Deletion is also always undefined
   //   (C++98 [expr.delete]p3). If you're doing this, fix your code.
   // - it cannot be const-qualified differently from T per unique_ptr spec
   //   (http://cplusplus.github.com/LWG/lwg-active.html#2118). Users wanting
   //   to work around this may use const_cast<const T*>().
   explicit scoped_ptr(element_type* array) : impl_(array) {}
 
   // Constructor.  Allows construction from a nullptr.
-  scoped_ptr(decltype(nullptr)) : impl_(nullptr) {}
+  scoped_ptr(std::nullptr_t n) : impl_(nullptr) {}
 
   // Constructor.  Allows construction from a scoped_ptr rvalue.
   scoped_ptr(scoped_ptr&& other) : impl_(&other.impl_) {}
 
   // operator=.  Allows assignment from a scoped_ptr rvalue.
   scoped_ptr& operator=(scoped_ptr&& rhs) {
     impl_.TakeState(&rhs.impl_);
     return *this;
   }
 
   // operator=.  Allows assignment from a nullptr. Deletes the currently owned
   // array, if any.
-  scoped_ptr& operator=(decltype(nullptr)) {
+  scoped_ptr& operator=(std::nullptr_t n) {
     reset();
     return *this;
   }
 
   // Reset.  Deletes the currently owned array, if any.
   // Then takes ownership of a new object, if given.
   void reset(element_type* array = nullptr) { impl_.reset(array); }
 
   // Accessors to get the owned array.
   element_type& operator[](size_t i) const {
@@ skipping 10 matching lines @@
   // implicitly convertible to a real bool (which is dangerous).
  private:
   typedef base::internal::scoped_ptr_impl<element_type, deleter_type>
       scoped_ptr::*Testable;
 
  public:
   operator Testable() const {
     return impl_.get() ? &scoped_ptr::impl_ : nullptr;
   }
 
-  // Comparison operators.
-  // These return whether two scoped_ptr refer to the same object, not just to
-  // two different but equal objects.
-  bool operator==(element_type* array) const { return impl_.get() == array; }
-  bool operator!=(element_type* array) const { return impl_.get() != array; }
-
   // Swap two scoped pointers.
   void swap(scoped_ptr& p2) {
     impl_.swap(p2.impl_);
   }
 
   // Release a pointer.
   // The return value is the current pointer held by this object. If this object
   // holds a nullptr, the return value is nullptr. After this operation, this
   // object will hold a nullptr, and will not own the object any more.
   element_type* release() WARN_UNUSED_RESULT {
@@ skipping 12 matching lines @@
   // private and has no definition. This is disabled because it is not safe to
   // call delete[] on an array whose static type does not match its dynamic
   // type.
   template <typename U> explicit scoped_ptr(U* array);
   explicit scoped_ptr(int disallow_construction_from_null);
 
   // Disable reset() from any type other than element_type*, for the same
   // reasons as the constructor above.
   template <typename U> void reset(U* array);
   void reset(int disallow_reset_from_null);
-
-  // Forbid comparison of scoped_ptr types.  If U != T, it totally
-  // doesn't make sense, and if U == T, it still doesn't make sense
-  // because you should never have the same object owned by two different
-  // scoped_ptrs.
-  template <class U> bool operator==(scoped_ptr<U> const& p2) const;
-  template <class U> bool operator!=(scoped_ptr<U> const& p2) const;
 };
 
 // Free functions
 template <class T, class D>
 void swap(scoped_ptr<T, D>& p1, scoped_ptr<T, D>& p2) {
   p1.swap(p2);
 }
 
+template <class T1, class D1, class T2, class D2>
+bool operator==(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) {
+  return p1.get() == p2.get();
+}
 template <class T, class D>
-bool operator==(T* p1, const scoped_ptr<T, D>& p2) {
-  return p1 == p2.get();
+bool operator==(const scoped_ptr<T, D>& p, std::nullptr_t n) {
+  return !p;

[Nico, 2015/11/12 17:20:47]

nit: i'd say `return p.get() == nullptr` (and for != `get() != nullptr` instead
of !!) – like you do with op< etc

[danakj, 2015/11/13 22:19:40]

Done. Used .get() == nullptr here, and below I did

!(p == nullptr)

to just reuse the == operator, if we're not going to try match how libc++
happened to write it super closely.

+}
+template <class T, class D>
+bool operator==(std::nullptr_t n, const scoped_ptr<T, D>& p) {
+  return !p;
 }
 
+template <class T1, class D1, class T2, class D2>
+bool operator!=(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) {
+  return !(p1 == p2);
+}
 template <class T, class D>
-bool operator!=(T* p1, const scoped_ptr<T, D>& p2) {
-  return p1 != p2.get();
+bool operator!=(const scoped_ptr<T, D>& p, std::nullptr_t n) {
+  return !!p;
+}
+template <class T, class D>
+bool operator!=(std::nullptr_t n, const scoped_ptr<T, D>& p) {
+  return !!p;
+}
+
+template <class T1, class D1, class T2, class D2>
+bool operator<(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) {
+  return p1.get() < p2.get();
+}
+template <class T, class D>
+bool operator<(const scoped_ptr<T, D>& p, std::nullptr_t n) {
+  return p.get() < nullptr;
+}
+template <class T, class D>
+bool operator<(std::nullptr_t n, const scoped_ptr<T, D>& p) {
+  return nullptr < p.get();
+}
+
+template <class T1, class D1, class T2, class D2>
+bool operator>(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) {
+  return p2 < p1;
+}
+template <class T, class D>
+bool operator>(const scoped_ptr<T, D>& p, std::nullptr_t n) {
+  return nullptr < p;
+}
+template <class T, class D>
+bool operator>(std::nullptr_t n, const scoped_ptr<T, D>& p) {
+  return p < nullptr;
+}
+
+template <class T1, class D1, class T2, class D2>
+bool operator<=(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) {
+  return !(p1 > p2);
+}
+template <class T, class D>
+bool operator<=(const scoped_ptr<T, D>& p, std::nullptr_t n) {
+  return !(p > nullptr);
+}
+template <class T, class D>
+bool operator<=(std::nullptr_t n, const scoped_ptr<T, D>& p) {
+  return !(nullptr > p);
+}
+
+template <class T1, class D1, class T2, class D2>
+bool operator>=(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) {
+  return !(p1 < p2);
+}
+template <class T, class D>
+bool operator>=(const scoped_ptr<T, D>& p, std::nullptr_t n) {
+  return !(p < nullptr);
+}
+template <class T, class D>
+bool operator>=(std::nullptr_t n, const scoped_ptr<T, D>& p) {
+  return !(nullptr < p);
 }
 
 // A function to convert T* into scoped_ptr<T>
 // Doing e.g. make_scoped_ptr(new FooBarBaz<type>(arg)) is a shorter notation
 // for scoped_ptr<FooBarBaz<type> >(new FooBarBaz<type>(arg))
 template <typename T>
 scoped_ptr<T> make_scoped_ptr(T* ptr) {
   return scoped_ptr<T>(ptr);
 }
 
 template <typename T>
 std::ostream& operator<<(std::ostream& out, const scoped_ptr<T>& p) {
   return out << p.get();
 }
 
 #endif  // BASE_MEMORY_SCOPED_PTR_H_