Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-24 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201817
---



Patch looks great!

Reviews applied: [66608]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 23, 2018, 9:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 23, 2018, 9:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-23 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201795
---



PASS: Mesos patch 66608 was successfully built and tested.

Reviews applied: `['66608']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66608

- Mesos Reviewbot Windows


On April 23, 2018, 9:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 23, 2018, 9:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-23 Thread Alexander Rukletsov 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201775
---


Ship it!




Ship It!

- Alexander Rukletsov


On April 23, 2018, 9:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 23, 2018, 9:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-23 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/
---

(Updated April 23, 2018, 11:52 p.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Documented overload as suggested offline by a reviewer.


Repository: mesos


Description
---

While it was already possible to create a `hashmap` over move-only
values, we still performed a copy in `put`, making it hard to
dynamically add elements with the expected stout semantics.

This patch relaxes the requirements on the value argument to `put` so
that instead of copyable we now only require move-constructible types.


Diffs (updated)
-

  3rdparty/stout/include/stout/hashmap.hpp 
91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 


Diff: https://reviews.apache.org/r/66608/diff/4/

Changes: https://reviews.apache.org/r/66608/diff/3-4/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-18 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201427
---



Patch looks great!

Reviews applied: [66608]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 17, 2018, 2:35 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 17, 2018, 2:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-18 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201411
---



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66608']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66608

Relevant logs:

- 
[libprocess-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66608/logs/libprocess-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\evutil_time.c(499):
 warning C4244: '=': conversion from 'int64_t' to 'long', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\evutil_time.c(504):
 warning C4244: '=': conversion from 'time_t' to 'long', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(185):
 warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(230):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(587):
 warning C4244: '=': conversion from 'SSIZE_T' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(598):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(654):
 warning C4244: '=': conversion from 'SSIZE_T' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(656):
 warning C4244: '=': conversion from 'SSIZE_T' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(675):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(755):
 warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(763):
 warning C4244: '=': conversion from 'SSIZE_T' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(874):
 warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]
 [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
 
d:\dcos\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta\bufferevent_openssl.c(1275):
 warning C4244: 'function': conversion from 'intptr_t' to 'int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\event_extra.vcxproj]

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-17 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201370
---



Patch looks great!

Reviews applied: [66608]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 17, 2018, 7:35 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 17, 2018, 7:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-17 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201319
---



PASS: Mesos patch 66608 was successfully built and tested.

Reviews applied: `['66608']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66608

- Mesos Reviewbot Windows


On April 17, 2018, 2:35 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 17, 2018, 2:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-17 Thread Benjamin Bannier 


> On April 16, 2018, 6:56 p.m., Alexander Rukletsov wrote:
> > 3rdparty/stout/include/stout/hashmap.hpp
> > Line 104 (original), 104 (patched)
> > 
> >
> > Reading 
> > https://stackoverflow.com/questions/21035417/is-the-pass-by-value-and-then-move-construct-a-bad-idiom,
> > 
> > I'm not sure what types are usually used as `Value`s in hashmaps. Are 
> > they expensive to move? Are they expensive to copy? Can we say that you 
> > suggestion is a strict improvement? Or at least a reasonable trade-off? I'm 
> > asking because I don't have neither enough experience nor data to make a 
> > decision.
> 
> Benjamin Bannier wrote:
> That's a valid concern. I consciously went for the simpler implementation 
> here, but this being in stout we are in pretty generic territory and it might 
> make sense to go for the slightly less naïve solution.
> 
> What's your feeling @mpark?

@alexr: I updated the patch with an additional overload.

I first tried to consume a generic value type which I then could 
`std::forward`, but unfortunately we put pointers to overloaded functions as 
values into `hashmap` which leads to problems deducing the required type for 
`put`.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201226
---


On April 17, 2018, 4:35 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 17, 2018, 4:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch relaxes the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible types.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-17 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/
---

(Updated April 17, 2018, 4:35 p.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Avoided moving when given an lvalue as suggested by alexr.


Repository: mesos


Description (updated)
---

While it was already possible to create a `hashmap` over move-only
values, we still performed a copy in `put`, making it hard to
dynamically add elements with the expected stout semantics.

This patch relaxes the requirements on the value argument to `put` so
that instead of copyable we now only require move-constructible types.


Diffs (updated)
-

  3rdparty/stout/include/stout/hashmap.hpp 
91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 


Diff: https://reviews.apache.org/r/66608/diff/2/

Changes: https://reviews.apache.org/r/66608/diff/1-2/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-16 Thread Benjamin Bannier 


> On April 16, 2018, 6:56 p.m., Alexander Rukletsov wrote:
> > 3rdparty/stout/include/stout/hashmap.hpp
> > Line 104 (original), 104 (patched)
> > 
> >
> > Reading 
> > https://stackoverflow.com/questions/21035417/is-the-pass-by-value-and-then-move-construct-a-bad-idiom,
> > 
> > I'm not sure what types are usually used as `Value`s in hashmaps. Are 
> > they expensive to move? Are they expensive to copy? Can we say that you 
> > suggestion is a strict improvement? Or at least a reasonable trade-off? I'm 
> > asking because I don't have neither enough experience nor data to make a 
> > decision.

That's a valid concern. I consciously went for the simpler implementation here, 
but this being in stout we are in pretty generic territory and it might make 
sense to go for the slightly less naïve solution.

What's your feeling @mpark?


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201226
---


On April 13, 2018, 3:09 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 13, 2018, 3:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch releases the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-16 Thread Alexander Rukletsov 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201226
---




3rdparty/stout/include/stout/hashmap.hpp
Line 104 (original), 104 (patched)


Reading 
https://stackoverflow.com/questions/21035417/is-the-pass-by-value-and-then-move-construct-a-bad-idiom,

I'm not sure what types are usually used as `Value`s in hashmaps. Are they 
expensive to move? Are they expensive to copy? Can we say that you suggestion 
is a strict improvement? Or at least a reasonable trade-off? I'm asking because 
I don't have neither enough experience nor data to make a decision.


- Alexander Rukletsov


On April 13, 2018, 1:09 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 13, 2018, 1:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch releases the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-13 Thread Mesos Reviewbot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201108
---



Patch looks great!

Reviews applied: [66608]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 13, 2018, 1:09 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 13, 2018, 1:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch releases the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-13 Thread Mesos Reviewbot Windows 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/#review201100
---



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66608']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66608

Relevant logs:

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66608/logs/mesos-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569):
 warning C4996:

Review Request 66608: Improved support for move-only types in `hashmap`.

2018-04-13 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66608/
---

Review request for mesos, Alexander Rukletsov and Michael Park.


Repository: mesos


Description
---

While it was already possible to create a `hashmap` over move-only
values, we still performed a copy in `put`, making it hard to
dynamically add elements with the expected stout semantics.

This patch releases the requirements on the value argument to `put` so
that instead of copyable we now only require move-constructible.


Diffs
-

  3rdparty/stout/include/stout/hashmap.hpp 
91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 


Diff: https://reviews.apache.org/r/66608/diff/1/


Testing
---

`make check`


Thanks,

Benjamin Bannier