Skip to content

Commit

Permalink
Make operators on scoped_ptr match the ones defined for std::unique_ptr
Browse files Browse the repository at this point in the history
Currently scoped_ptr is missing comparison operators other than == and
!=, and it defines those operators incorrectly (it compares to a raw
pointer but unique_ptr compares to a unique_ptr).

This fixes the operator== and !=, and adds .get() at a bunch of
callsites. And adds the < > <= >= operators so that we don't have any
differences in where you can use it. This will help the transition
from scoped_ptr to unique_ptr as no code will have to change along with
the rename wrt these operators.

R=Nico, dcheng
TBR=ellyjones, sky, jochen, wez, rockot, droger
BUG=554390
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1440593004

Cr-Commit-Position: refs/heads/master@{#359957}
  • Loading branch information
danakj authored and Commit bot committed Nov 17, 2015
1 parent 5634546 commit 8199479
Show file tree
Hide file tree
Showing 47 changed files with 363 additions and 173 deletions.
2 changes: 1 addition & 1 deletion ash/shelf/shelf_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1900,7 +1900,7 @@ void ShelfView::OnBoundsAnimatorProgressed(views::BoundsAnimator* animator) {
}

void ShelfView::OnBoundsAnimatorDone(views::BoundsAnimator* animator) {
if (snap_back_from_rip_off_view_ && animator == bounds_animator_) {
if (snap_back_from_rip_off_view_ && animator == bounds_animator_.get()) {
if (!animator->IsAnimating(snap_back_from_rip_off_view_)) {
// Coming here the animation of the ShelfButton is finished and the
// previously hidden status can be shown again. Since the button itself
Expand Down
6 changes: 2 additions & 4 deletions ash/wm/overview/window_selector_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,7 @@ TEST_F(WindowSelectorTest, WindowDoesNotReceiveEvents) {

// The event should target the window because we are still not in overview
// mode.
EXPECT_EQ(window, static_cast<aura::Window*>(
targeter->FindTargetForEvent(root_target, &event1)));
EXPECT_EQ(window.get(), targeter->FindTargetForEvent(root_target, &event1));

ToggleOverview();

Expand All @@ -751,8 +750,7 @@ TEST_F(WindowSelectorTest, WindowDoesNotReceiveEvents) {
ui::EventTimeForNow(), ui::EF_NONE, ui::EF_NONE);

// Now the transparent window should be intercepting this event.
EXPECT_NE(window, static_cast<aura::Window*>(
targeter->FindTargetForEvent(root_target, &event2)));
EXPECT_NE(window.get(), targeter->FindTargetForEvent(root_target, &event2));
}

// Tests that clicking on the close button effectively closes the window.
Expand Down
2 changes: 1 addition & 1 deletion ash/wm/system_gesture_event_filter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ TEST_F(SystemGestureEventFilterTest, LongPressAffordanceStateOnCaptureLoss) {

base::OneShotTimer* timer = GetLongPressAffordanceTimer();
EXPECT_TRUE(timer->IsRunning());
EXPECT_EQ(window1, GetLongPressAffordanceTarget());
EXPECT_EQ(window1.get(), GetLongPressAffordanceTarget());

// Force timeout so that the affordance animation can start.
timer->user_task().Run();
Expand Down
2 changes: 1 addition & 1 deletion ash/wm/system_modal_container_layout_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ TEST_F(SystemModalContainerLayoutManagerTest, ModalNonTransient) {

EXPECT_TRUE(wm::IsActiveWindow(t2));

EXPECT_EQ(t1, ::wm::GetTransientParent(t2));
EXPECT_EQ(t1.get(), ::wm::GetTransientParent(t2));
EXPECT_EQ(GetModalContainer(), t2->parent());

// t2 should still be active, even after clicking on t1.
Expand Down
2 changes: 1 addition & 1 deletion ash/wm/workspace/workspace_window_resizer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class WorkspaceWindowResizerTest : public test::AshTestBase {
const aura::Window::Windows& windows = parent->children();
for (aura::Window::Windows::const_reverse_iterator i = windows.rbegin();
i != windows.rend(); ++i) {
if (*i == window_ || *i == window2_ || *i == window3_) {
if (*i == window_.get() || *i == window2_.get() || *i == window3_.get()) {
if (!result.empty())
result += " ";
result += base::IntToString((*i)->id());
Expand Down
110 changes: 76 additions & 34 deletions base/memory/scoped_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ class scoped_ptr {
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) : impl_(nullptr) {}

// Constructor. Allows construction from a scoped_ptr rvalue for a
// convertible type and deleter.
Expand Down Expand Up @@ -362,7 +362,7 @@ class scoped_ptr {

// operator=. Allows assignment from a nullptr. Deletes the currently owned
// object, if any.
scoped_ptr& operator=(decltype(nullptr)) {
scoped_ptr& operator=(std::nullptr_t) {
reset();
return *this;
}
Expand Down Expand Up @@ -403,12 +403,6 @@ class scoped_ptr {
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_);
Expand All @@ -429,13 +423,6 @@ class scoped_ptr {

// 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>
Expand Down Expand Up @@ -464,7 +451,7 @@ class scoped_ptr<T[], D> {
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) : impl_(nullptr) {}

// Constructor. Allows construction from a scoped_ptr rvalue.
scoped_ptr(scoped_ptr&& other) : impl_(&other.impl_) {}
Expand All @@ -477,7 +464,7 @@ class scoped_ptr<T[], D> {

// operator=. Allows assignment from a nullptr. Deletes the currently owned
// array, if any.
scoped_ptr& operator=(decltype(nullptr)) {
scoped_ptr& operator=(std::nullptr_t) {
reset();
return *this;
}
Expand Down Expand Up @@ -508,12 +495,6 @@ class scoped_ptr<T[], D> {
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_);
Expand Down Expand Up @@ -546,13 +527,6 @@ class scoped_ptr<T[], D> {
// 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
Expand All @@ -561,14 +535,82 @@ 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==(const scoped_ptr<T, D>& p, std::nullptr_t) {
return p.get() == nullptr;
}
template <class T, class D>
bool operator==(std::nullptr_t, const scoped_ptr<T, D>& p) {
return p.get() == 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) {
return !(p == nullptr);
}
template <class T, class D>
bool operator!=(std::nullptr_t, 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.get() < p2.get();
}
template <class T, class D>
bool operator<(const scoped_ptr<T, D>& p, std::nullptr_t) {
return p.get() < nullptr;
}
template <class T, class D>
bool operator<(std::nullptr_t, 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) {
return nullptr < p;
}
template <class T, class D>
bool operator==(T* p1, const scoped_ptr<T, D>& p2) {
return p1 == p2.get();
bool operator>(std::nullptr_t, 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) {
return !(p > nullptr);
}
template <class T, class D>
bool operator<=(std::nullptr_t, 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) {
return !(p < nullptr);
}
template <class T, class D>
bool operator!=(T* p1, const scoped_ptr<T, D>& p2) {
return p1 != p2.get();
bool operator>=(std::nullptr_t, const scoped_ptr<T, D>& p) {
return !(nullptr < p);
}

// A function to convert T* into scoped_ptr<T>
Expand Down
Loading

0 comments on commit 8199479

Please sign in to comment.