Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(937)

Issue 611973002: Revert of Revert of Add nullptr support to scoped_ptr. (Closed)

Created:
6 years, 2 months ago by eustas
Modified:
6 years, 2 months ago
Reviewers:
danakj, jamesr, brettw, Nico, dcheng
CC:
chromium-reviews, erikwright+watch_chromium.org, gavinp+memory_chromium.org, Ryan Sleevi
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Revert of Revert of Add nullptr support to scoped_ptr. (patchset #1 id:1 of https://codereview.chromium.org/604423005/) Reason for revert: multiple compilation errors Original issue's description: > Revert of Add nullptr support to scoped_ptr. (patchset #8 id:200001 of https://codereview.chromium.org/599313003/) > > Reason for revert: > This patch seems to break ScopedPtrWithArray ASAN test. > > https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests%20(1)/builds/2348/steps/base_unittests > > Original issue's description: > > Add nullptr support to scoped_ptr. > > > > This adds support to use nullptr to construct, assign, or return a > > scoped_ptr<T> and scoped_ptr<T[]>. Support for this requires the use > > of a move-only constructor. > > > > The changes are: > > > > - Add a constructor that takes decltype(nullptr) as a parameter. This > > allows behaviour such as scoped_ptr<T>(nullptr), but also allows a > > function with return type scoped_ptr<T> to "return nullptr;" instead > > of "return scoped_ptr<T>();". > > > > - Add an operator=(decltype(nullptr)) that resets the scoped_ptr to > > empty and deletes anything it held. > > > > - Add/Modify a constructor to take a scoped_ptr<U,E>&& parameter for > > constructing a scoped_ptr from another using move-only semantics. This > > piece is critical for allowing the function returning nullptr to be > > assigned to some other scoped_ptr at the callsite. In particular, take > > the following code: > > scoped_ptr<T> Function() { return nullptr; } > > scoped_ptr<T> var = Function(); > > In this case the constructor which takes a nullptr allows Function() to > > be written, but not to be used. The move-only constructor allows the > > assignment from Function() to var. See "C++11 feature proposal: > > Move-only constructors" on chromium-dev for more explanation why. > > > > The scoped_ptr<T> class already had a constructor which took > > scoped_ptr<U,E> as an argument, so this was changed to be > > scoped_ptr<U,E>&& instead. The scoped_ptr<T[]> class had no such > > constructor, so a scoped_ptr&& constructor was added. These match > > the constructors found on the unique_ptr class. > > > > - Remove the RValue type and the contructor that constructs a > > scoped_ptr from an RValue. Change Pass() to return a scoped_ptr&& > > instead of a scoped_ptr::RValue, to avoid the type conversion and > > remove some complexity. This is done with a new emulation macro that > > still provides Pass() and makes the type go down the MoveOnlyType > > path in base::Callback code. > > > > This adds base_unittests to demonstrate and use these changes. > > > > The use of Pass() remains unchanged until std::move() is written > > or allowed. At that time std::move() could be used instead of Pass. > > > > R=brettw@chromium.org, jamesr@chromium.org > > > > Committed: https://crrev.com/2299e91d3508f8d5d18ef990cf6024ea4371250a > > Cr-Commit-Position: refs/heads/master@{#297072} > > > > Committed: https://crrev.com/a9527ce329c38d945e46773f1592a4939cf62b99 > > Cr-Commit-Position: refs/heads/master@{#297116} > > TBR=brettw@chromium.org,jamesr@chromium.org,thakis@chromium.org,dcheng@chromium.org,danakj@chromium.org > NOTREECHECKS=true > NOTRY=true > > Committed: https://crrev.com/300249a2f5d9f0b8b86553712ac7ccb55d6afc1c > Cr-Commit-Position: refs/heads/master@{#297149} TBR=brettw@chromium.org,jamesr@chromium.org,thakis@chromium.org,dcheng@chromium.org,danakj@chromium.org NOTREECHECKS=true NOTRY=true Committed: https://crrev.com/36a3e81140bab724c8cbae234ac46174935ed38f Cr-Commit-Position: refs/heads/master@{#297150}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+122 lines, -39 lines) Patch
M base/memory/scoped_ptr.h View 17 chunks +68 lines, -39 lines 0 comments Download
M base/memory/scoped_ptr_unittest.cc View 2 chunks +54 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
eustas
Created Revert of Revert of Add nullptr support to scoped_ptr.
6 years, 2 months ago (2014-09-29 08:26:38 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/611973002/1
6 years, 2 months ago (2014-09-29 08:27:30 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as 3cf654888992122b2c8a1eab50fb0b9c75a1ef7c
6 years, 2 months ago (2014-09-29 08:27:47 UTC) #3
commit-bot: I haz the power
6 years, 2 months ago (2014-09-29 08:28:20 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/36a3e81140bab724c8cbae234ac46174935ed38f
Cr-Commit-Position: refs/heads/master@{#297150}

Powered by Google App Engine
This is Rietveld 408576698