Re: boost::scoped_ptr vs std::unique_ptr

2018-07-06 Thread Jim Apple
Thank you for speaking up, Zoltan! On Fri, Jul 6, 2018 at 1:26 PM, Zoltan Borok-Nagy < borokna...@cloudera.com.invalid> wrote: > Yeah I see that, no worries. > I only wanted to draw some attention to it, so we might not forget it in > the future. > > - Zoltan > > On Fri, Jul 6, 2018 at 5:57 PM Ti

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-06 Thread Zoltan Borok-Nagy
Yeah I see that, no worries. I only wanted to draw some attention to it, so we might not forget it in the future. - Zoltan On Fri, Jul 6, 2018 at 5:57 PM Tim Armstrong wrote: > That's a fair observation Zoltan, we should avoid doing that. I would have > definitely waited longer if it was a more

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-06 Thread Tim Armstrong
That's a fair observation Zoltan, we should avoid doing that. I would have definitely waited longer if it was a more consequential or irreversible decision, but I think even for more minor things there's no reason not to wait longer. On Fri, Jul 6, 2018 at 4:03 AM, Zoltan Borok-Nagy < borokna...@c

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-06 Thread Zoltan Borok-Nagy
Hi, I'm also in favor of unique_ptr and maybe const unique_ptr. But I want to point out that the duration of the whole discussion was 1.5 hours. Maybe we could keep such threads open at least 24 hours to let anyone in the globe weigh in. BR, Zoltan On Fri, Jul 6, 2018 at 3:31 AM Jim Apple

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-05 Thread Jim Apple
SGTM On Thu, Jul 5, 2018 at 6:13 PM, Tim Armstrong < tarmstr...@cloudera.com.invalid> wrote: > Sounds like unique_ptr is preferred then going forward. I updated the wiki > page. > > > Fwiw, I was under the impression from talking with people in the past > that > > we were already trying to make

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-05 Thread Tim Armstrong
Sounds like unique_ptr is preferred then going forward. I updated the wiki page. > Fwiw, I was under the impression from talking with people in the past that > we were already trying to make this move, and the > PartitionedAggregationNode refactor that just went in made the switch to > unique_ptr

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-05 Thread Todd Lipcon
Definitely in favor. Personally I never found the "this pointer isn't movable" to be a worthwhile distinction. With unique_ptr you need to pretty explicitly move it using std::move, so you don't get "accidental" moves like you used to with std::auto_ptr. Looking briefly at Kudu we have 129 unique

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-05 Thread Jim Apple
I suspect we could also make own own immobile_ptr with minimal effort, thereby both making the difference more visible and reducing boost dependence. On Thu, Jul 5, 2018 at 5:17 PM, Sailesh Mukil wrote: > I'm in favor. > > Since the main distinction is that a unique_ptr is moveable, whereas a >

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-05 Thread Sailesh Mukil
I'm in favor. Since the main distinction is that a unique_ptr is moveable, whereas a scoped_ptr is not, we should just make sure that we do our due diligence during code reviews so that we catch those cases. Also, making a unique_ptr const disallows moving it, since the move constructor takes a n

Re: boost::scoped_ptr vs std::unique_ptr

2018-07-05 Thread Thomas Tauber-Marshall
I'm definitely in favor of using more standard c++ to reduce both confusion and our reliance on boost, especially as I suspect a lot of people (eg. me) don't know the subtle difference between scoped_ptr and unique_ptr off the top of their head anyways. Fwiw, I was under the impression from talkin

boost::scoped_ptr vs std::unique_ptr

2018-07-05 Thread Tim Armstrong
I was just talking with Michael Ho on a review about this https://gerrit.cloudera.org/#/c/10810/7/be/src/exec/scan-node.h@271 For a while we've continued using scoped_ptr in some places because it supports a smaller set of operators and implies that the pointer isn't movable. See https://cwiki.apa