Generally, we want to move away from implicitly-managed memory (d'tor frees) to explicitly, centrally managed memory (in MemPools, and d'tors become no-ops).
With that in mind, I would limit the amount of effort spent on changing from one implicitly-managed model to another. That said, I'm in favor of doing the scoped_/unique_ptr replacement in the current codebase where it is necessary to move a reference. I'm not sure where that would currently apply. On Wed, Aug 31, 2016 at 11:30 AM, Tim Armstrong <[email protected]> wrote: > I don't feel that strongly about it but, but I think scoped_ptr/unique_ptr > is a useful distinction to document that the pointer isn't movable. > > I think it contains info both ways: if it's a unique_ptr it means we intend > to (sometimes) transfer it, but if it's a scoped_ptr its lifetime is > strictly bounded by the owner. > > I agree it's hard to accidentally transfer something but I think if we use > the same type it's much easier to violate the implicit memory lifetime > assumptions in a bit of code without warning. > > E.g. if you have class Parent that contains a scoped_ptr<Child> child_, > then it's always safe for child_ to store a Parent* reference. But if you > change it to unique_ptr<Child> then it's no longer obvious that this is > safe without auditing references to child_ to make sure it isn't release()d > or moved. > > On Wed, Aug 31, 2016 at 11:24 AM, Jim Apple <[email protected]> wrote: > >> I'm convinced. +1 to moving to ::std::unique_ptr. >> >> On Wed, Aug 31, 2016 at 11:22 AM, Henry Robinson <[email protected]> >> wrote: >> > On 31 August 2016 at 11:16, Jim Apple <[email protected]> wrote: >> > >> >> Why should we reduce our boost dependency? >> >> >> > >> > Boost will sometimes break subtly (or unsubtly by changing APIs) between >> > versions, is often not as well tested as stdlib implementations and does >> > not have a standard. If there are reasonable std:: implementations of >> > boost:: primitives, experience has shown it's usually a good idea to opt >> > for the std >> > >> > >> >> >> >> Do you think there are places where scoped_ptr is used now where you >> >> would want to keep it indefinitely if it were part of the standard and >> >> not part of boost? >> >> >> > >> > No, but I can't say I've audited every location. For our typical uses, I >> > don't see a disadvantage to unique_ptr. >> > >> > >> > >> >> >> >> On Wed, Aug 31, 2016 at 10:47 AM, Henry Robinson <[email protected]> >> wrote: >> >> > We use boost::scoped_ptr everywhere to handle scope-owned >> heap-allocated >> >> > objects. C++11 has std::unique_ptr for this. I'd like to get a >> decision >> >> on >> >> > whether we should start standardising on unique_ptr. This is >> particularly >> >> > relevant for new code - should I call it out in code review? >> >> > >> >> > The most significant difference is that unique_ptr is moveable, which >> >> means >> >> > it can be used in collections (good!). It also means that badly >> written >> >> > code can allow scope-owned objects to escape their scope: >> >> > >> >> > private: >> >> > unique_ptr<Foo> foo_; >> >> > >> >> > public: >> >> > unique_ptr<Foo> get_foo() { return move(foo_); } >> >> > >> >> > or worse: >> >> > >> >> > Foo* get_foo() { return foo_.release(); } >> >> > >> >> > In both cases you have to be quite explicit about the decision to >> yield >> >> > ownership of the owned object, and it seems to me that we should catch >> >> this >> >> > in code review. >> >> > >> >> > Since using unique_ptr in collections is so useful, and reducing our >> >> boost >> >> > dependency is generally worthwhile, I'm very much in favour of moving >> to >> >> > unique_ptr for future code, and at some point porting all the current >> >> > scoped_ptr to unique_ptr. >> >> > >> >> > What do you think? >> >> > >> >> > Henry >> >> >> > >> > >> > >> > -- >> > Henry Robinson >> > Software Engineer >> > Cloudera >> > 415-994-6679 >>
