[llvm] r202609 - [C++11] Add support for OwningPtr<T> to be converted to and from

Richard Smith richard at metafoo.co.uk
Sun Mar 2 10:55:31 PST 2014


On Sun, Mar 2, 2014 at 9:12 AM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> On 2014 Mar 2, at 01:51, Ahmed Charles <acharles at outlook.com> wrote:
>
> >
> __==============================================================================
> >>
> >> --- llvm/trunk/include/llvm/ADT/OwningPtr.h (original)
> >> +++ llvm/trunk/include/llvm/ADT/OwningPtr.h Sat Mar  1 21:38:32 2014
> >> @@ -17,6 +17,7 @@
> >>  #include "llvm/Support/Compiler.h"
> >>  #include <cassert>
> >>  #include <cstddef>
> >> +#include <memory>
> >>
> >>  namespace llvm {
> >>
> >> @@ -39,6 +40,17 @@ public:
> >>      return *this;
> >>    }
> >>
> >> +  OwningPtr(std::unique_ptr<T> &&Other) : Ptr(Other.release()) {}
> >> +
> >> +  OwningPtr &operator=(std::unique_ptr<T> &&Other) {
> >>
> >> Why an rvalue reference to a unique_ptr rather than passing the
> >> unique_ptr by value?
> >
> > No particular reason, though I suppose it is one less write to memory if
> the optimizer can't remove it. I just didn't consider passing by value.
> Feel free to change, I don't have commit access yet.
>
> An rvalue-reference makes sense here.  It makes it clear that OwningPtr
> is taking ownership from unique_ptr.


Passing unique_ptr by value also makes that clear, and is the idiomatic way
of passing ownership with unique_ptr.


> In particular, its signature lines
> up with the standard signature for move assignment operators:
>
>    T &T::operator=(T &&);
>
> Matching the signature makes it obvious that it's the same semantic
> operation.  (It's also more efficient before compiler optimizations.)


That's less efficient after compiler optimizations, in the case where the
function isn't inlined for whatever reason. Efficiency before optimization
is not worth optimizing for.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140302/07a5b802/attachment.html>


More information about the llvm-commits mailing list