[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D11394: Fix warnings about pessimizing return moves for C++11 and higher
From:       Richard Smith <richard () metafoo ! co ! uk>
Date:       2015-07-22 1:17:13
Message-ID: CAOfiQqmSn+cZOLA0v28yC=1_t8eRb2qZN1fP95OKHDoxFzTOVw () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Tue, Jul 21, 2015 at 5:43 PM, Eric Fiselier <eric@efcs.ca> wrote:

> > I don't think that's right. In C++03, unique_ptr has a
> unique_ptr(unique_ptr&) constructor. And the C++03 std::move is:
> >
> >  template<typename T> T &move(T &v) { return v; }
> >
> > So... the "explicitly moved for C++03" call to std::move in map appears
> to also be redundant (and pessimizing) in C++03. In fact, in C++03,
> std::move > appears to *always* be a no-op.
>
> I don't think unique_ptr provides the constructor you mention.


Well, it does (memory:2554), but it's private. Sorry, I didn't notice that
before...

The
> definition of move used in C++03 for unique_ptr is
>
>  unique_ptr move(unique_ptr& u) {
>     return unique_ptr(__rv<unique_ptr>(u));
>   }
>
> unique_ptr provides a constructor of the form
> `unique_ptr(__rv<unique_ptr>&).
>
> It seems like this dance is done to force copy elision to take place.
>

It looks like libc++'s design for unique_ptr means that it's not possible
to get NRVO to apply to it in C++03. The approach in this patch seems fine
to me.


> /Eric
>
>
> On Tue, Jul 21, 2015 at 8:35 PM, Richard Smith <richard@metafoo.co.uk>
> wrote:
> > On Tue, Jul 21, 2015 at 5:29 PM, Eric Fiselier <eric@efcs.ca> wrote:
> >>
> >> EricWF added a comment.
> >>
> >> Thanks for the patch. I ran into this issue the other day and I'm glad
> to
> >> see it fixed.
> >>
> >> A little rational: The explicit move's are needed in order to "move" a
> >> `unique_ptr` in C++03. There is a special definition of `std::move` in
> >> memory at line 3100 that performs some hacks to make `unique_ptr`
> movable. I
> >> don't think any other classes benefit from the "explicit move" in C++03.
> >
> >
> > I don't think that's right. In C++03, unique_ptr has a
> > unique_ptr(unique_ptr&) constructor. And the C++03 std::move is:
> >
> >   template<typename T> T &move(T &v) { return v; }
> >
> > So... the "explicitly moved for C++03" call to std::move in map appears
> to
> > also be redundant (and pessimizing) in C++03. In fact, in C++03,
> std::move
> > appears to *always* be a no-op.
>

[Attachment #5 (text/html)]

<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jul 21, 2015 \
at 5:43 PM, Eric Fiselier <span dir="ltr">&lt;<a href="mailto:eric@efcs.ca" \
target="_blank">eric@efcs.ca</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span \
class="">&gt; I don&#39;t think that&#39;s right. In C++03, unique_ptr has a \
unique_ptr(unique_ptr&amp;) constructor. And the C++03 std::move is:<br> &gt;<br>
&gt;   template&lt;typename T&gt; T &amp;move(T &amp;v) { return v; }<br>
&gt;<br>
&gt; So... the &quot;explicitly moved for C++03&quot; call to std::move in map \
appears to also be redundant (and pessimizing) in C++03. In fact, in C++03, std::move \
&gt; appears to *always* be a no-op.<br> <br>
</span>I don&#39;t think unique_ptr provides the constructor you \
mention.</blockquote><div><br></div><div>Well, it does (memory:2554), but it&#39;s \
private. Sorry, I didn&#39;t notice that before...</div><div><br></div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">The<br> definition of move used in C++03 for unique_ptr \
is<br> <br>
  unique_ptr move(unique_ptr&amp; u) {<br>
      return unique_ptr(__rv&lt;unique_ptr&gt;(u));<br>
   }<br>
<br>
unique_ptr provides a constructor of the form \
`unique_ptr(__rv&lt;unique_ptr&gt;&amp;).<br> <br>
It seems like this dance is done to force copy elision to take \
place.<br></blockquote><div><br></div><div>It looks like libc++&#39;s design for \
unique_ptr means that it&#39;s not possible to get NRVO to apply to it in C++03. The \
approach in this patch seems fine to me.</div><div>  </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> /Eric<br>
<br>
<br>
On Tue, Jul 21, 2015 at 8:35 PM, Richard Smith &lt;<a \
href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>&gt; wrote:<br> <div \
class="HOEnZb"><div class="h5">&gt; On Tue, Jul 21, 2015 at 5:29 PM, Eric Fiselier \
&lt;<a href="mailto:eric@efcs.ca">eric@efcs.ca</a>&gt; wrote:<br> &gt;&gt;<br>
&gt;&gt; EricWF added a comment.<br>
&gt;&gt;<br>
&gt;&gt; Thanks for the patch. I ran into this issue the other day and I&#39;m glad \
to<br> &gt;&gt; see it fixed.<br>
&gt;&gt;<br>
&gt;&gt; A little rational: The explicit move&#39;s are needed in order to \
&quot;move&quot; a<br> &gt;&gt; `unique_ptr` in C++03. There is a special definition \
of `std::move` in<br> &gt;&gt; memory at line 3100 that performs some hacks to make \
`unique_ptr` movable. I<br> &gt;&gt; don&#39;t think any other classes benefit from \
the &quot;explicit move&quot; in C++03.<br> &gt;<br>
&gt;<br>
&gt; I don&#39;t think that&#39;s right. In C++03, unique_ptr has a<br>
&gt; unique_ptr(unique_ptr&amp;) constructor. And the C++03 std::move is:<br>
&gt;<br>
&gt;     template&lt;typename T&gt; T &amp;move(T &amp;v) { return v; }<br>
&gt;<br>
&gt; So... the &quot;explicitly moved for C++03&quot; call to std::move in map \
appears to<br> &gt; also be redundant (and pessimizing) in C++03. In fact, in C++03, \
std::move<br> &gt; appears to *always* be a no-op.<br>
</div></div></blockquote></div><br></div></div>



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic