headless: Add a Maybe-type

headless: Add a Maybe-type

Add a Maybe-type which will be used by the C++ DevTools client API for
representing optional values.

BUG=595353
Committed: https://crrev.com/d79e1515bc06265a1216c6910feb7a21232131e2
Cr-Commit-Position: refs/heads/master@{#385797}

[Sami, 2016-04-06 18:11:15]

Description was changed from

==========
headless: Add a Maybe-type

Add a Maybe-type which will be used by the C++ DevTools client API for
representing optional values.

BUG=595353
==========

to

==========
headless: Add a Maybe-type

Add a Maybe-type which will be used by the C++ DevTools client API for
representing optional values.

BUG=595353
==========

[Sami, 2016-04-06 18:11:15]

skyostil@chromium.org changed reviewers:
+ alexclarke@chromium.org, altimin@chromium.org

[alex clarke (OOO till 29th), 2016-04-07 11:45:32]

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe.h
File headless/public/util/maybe.h (right):

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe....
headless/public/util/maybe.h:47: std::swap(value_, other.value_);
shouldn't this be value_ = std::move(other.value_);

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe....
headless/public/util/maybe.h:57: Maybe(const Maybe& other) { *this = other; }
I wonder if this and the one below could be = default?

[alex clarke (OOO till 29th), 2016-04-07 12:00:05]

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe_...
File headless/public/util/maybe_unittest.cc (right):

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe_...
headless/public/util/maybe_unittest.cc:73: Maybe<MoveOnlyType> b =
Just(MoveOnlyType());
Can you do this:

MoveOnlyType foo = std::move(b.FromJust());

[Sami, 2016-04-07 12:00:44]

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe.h
File headless/public/util/maybe.h (right):

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe....
headless/public/util/maybe.h:47: std::swap(value_, other.value_);
On 2016/04/07 11:45:32, alexclarke1 wrote:
> shouldn't this be value_ = std::move(other.value_);

Good point, done. I've also added a question to
https://sites.google.com/a/chromium.org/dev/rvalue-references?pli=1

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe....
headless/public/util/maybe.h:57: Maybe(const Maybe& other) { *this = other; }
On 2016/04/07 11:45:32, alexclarke1 wrote:
> I wonder if this and the one below could be = default?

Yep, they can, fixed. If you're wondering why we need to define them explicitly,
it's because:

"copy constructor is implicitly deleted because 'Maybe<headless::(anonymous
namespace)::MoveOnlyType>' has a user-declared move assignment operator"

[Sami, 2016-04-07 12:08:52]

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe_...
File headless/public/util/maybe_unittest.cc (right):

https://codereview.chromium.org/1866063002/diff/1/headless/public/util/maybe_...
headless/public/util/maybe_unittest.cc:73: Maybe<MoveOnlyType> b =
Just(MoveOnlyType());
On 2016/04/07 12:00:05, alexclarke1 wrote:
> Can you do this:
> 
> MoveOnlyType foo = std::move(b.FromJust());

Added a test for it.

[alex clarke (OOO till 29th), 2016-04-07 15:31:11]

lgtm

[Sami, 2016-04-07 16:51:57]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2016-04-07 16:52:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1866063002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1866063002/40001

[commit-bot: I haz the power, 2016-04-07 17:51:17]

Description was changed from

==========
headless: Add a Maybe-type

Add a Maybe-type which will be used by the C++ DevTools client API for
representing optional values.

BUG=595353
==========

to

==========
headless: Add a Maybe-type

Add a Maybe-type which will be used by the C++ DevTools client API for
representing optional values.

BUG=595353
==========

[commit-bot: I haz the power, 2016-04-07 17:51:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-07 17:52:37]

Description was changed from

==========
headless: Add a Maybe-type

Add a Maybe-type which will be used by the C++ DevTools client API for
representing optional values.

BUG=595353
==========

to

==========
headless: Add a Maybe-type

Add a Maybe-type which will be used by the C++ DevTools client API for
representing optional values.

BUG=595353

Committed: https://crrev.com/d79e1515bc06265a1216c6910feb7a21232131e2
Cr-Commit-Position: refs/heads/master@{#385797}
==========

[commit-bot: I haz the power, 2016-04-07 17:52:38]

Patchset 3 (id:??) landed as
https://crrev.com/d79e1515bc06265a1216c6910feb7a21232131e2
Cr-Commit-Position: refs/heads/master@{#385797}

[danakj, 2016-04-08 00:32:32]

danakj@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2016-04-08 00:32:33]

https://codereview.chromium.org/1245163002/

[Sami, 2016-04-08 10:06:01]

On 2016/04/08 00:32:33, danakj wrote:
> https://codereview.chromium.org/1245163002/

Heh, great minds think alike :D I had a chat with Mounir and I think we can
easily migrate to base::Optional<> once it lands because the code that uses it
in headless is autogenerated. I'll keep an eye on that patch.

[danakj, 2016-04-08 18:08:34]

On Fri, Apr 8, 2016 at 3:06 AM, skyostil@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> On 2016/04/08 00:32:33, danakj wrote:
> > https://codereview.chromium.org/1245163002/
>
> Heh, great minds think alike :D I had a chat with Mounir and I think we can
> easily migrate to base::Optional<> once it lands because the code that
> uses it
> in headless is autogenerated. I'll keep an eye on that patch.
>

Great :)

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.