[libcxx-commits] [PATCH] D80057: [libc++][NFCI] Optimization to std::~unique_ptr

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

> We can't do that, because p.reset(x) needs to set p.__ptr_ to x, regardless of whether p.__ptr_ is null or not. Or are we talking past each other?

I'm not talking about `reset`. I'm saying inline `reset` into the destructor and then exit early. Basically, replace the destructor with this:

  ~unique_ptr() {
      pointer __tmp = __ptr_.first();
      if (!__tmp)
          return;
      __ptr_.first() = pointer(); // pointer() = nullptr
      __ptr_.second()(__tmp);
  }



> As you mentioned, there's a dead store whenever the unique_ptr escapes, so it's more than just null unique_ptrs.

You're right. This probably isn't because of the destructor.

> However, since it's in the destructor of the object, why does it matter whether the unique_ptr has escaped?

My guess is that it can't prove that `do_complex_things` doesn't make the pointer not-null.

> If the answer is "it doesn't matter", then I think we could improve the codegen here such that the dead store is always removed.

I think I found the cause. This isn't the destructor, it's the move constructor. We call `release()` which is where we set the pointer to null.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80057/new/

https://reviews.llvm.org/D80057