> 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. > > 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 > >
