> On April 16, 2018, 6:56 p.m., Alexander Rukletsov wrote: > > 3rdparty/stout/include/stout/hashmap.hpp > > Line 104 (original), 104 (patched) > > <https://reviews.apache.org/r/66608/diff/1/?file=1998223#file1998223line104> > > > > 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 > >
