<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 26, 2016 at 4:31 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">> On May 26, 2016, at 4:27 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> /casts Necromancy<br>
><br>
> (Sorry for the duplicate, John - forgot to switch mailing lists, of course)<br>
><br>
> On Tue, May 1, 2012 at 10:39 PM, John McCall <<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>> wrote:<br>
> Author: rjmccall<br>
> Date: Wed May 2 00:39:15 2012<br>
> New Revision: 155979<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=155979&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=155979&view=rev</a><br>
> Log:<br>
> Update SmallVector to support move semantics if the host does.<br>
> Note that support for rvalue references does not imply support<br>
> for the full set of move-related STL operations.<br>
><br>
> I've preserved support for an odd little thing in insert() where<br>
> we're trying to support inserting a new element from an existing<br>
> one. If we actually want to support that, there's a lot more we<br>
> need to do: insert can call either grow or push_back, neither of<br>
> which is safe against this particular use pattern.<br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/ADT/SmallVector.h<br>
> llvm/trunk/include/llvm/Support/Compiler.h<br>
><br>
> Modified: llvm/trunk/include/llvm/ADT/SmallVector.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=155979&r1=155978&r2=155979&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=155979&r1=155978&r2=155979&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/ADT/SmallVector.h (original)<br>
> +++ llvm/trunk/include/llvm/ADT/SmallVector.h Wed May 2 00:39:15 2012<br>
> @@ -14,6 +14,7 @@<br>
> #ifndef LLVM_ADT_SMALLVECTOR_H<br>
> #define LLVM_ADT_SMALLVECTOR_H<br>
><br>
> +#include "llvm/Support/Compiler.h"<br>
> #include "llvm/Support/type_traits.h"<br>
> #include <algorithm><br>
> #include <cassert><br>
> @@ -54,6 +55,11 @@<br>
> return BeginX == static_cast<const void*>(&FirstEl);<br>
> }<br>
><br>
> + /// resetToSmall - Put this vector in a state of being small.<br>
> + void resetToSmall() {<br>
> + BeginX = EndX = CapacityX = &FirstEl;<br>
> + }<br>
> +<br>
> /// grow_pod - This is an implementation of the grow() method which only works<br>
> /// on POD-like data types and is out of line to reduce code duplication.<br>
> void grow_pod(size_t MinSizeInBytes, size_t TSize);<br>
> @@ -160,28 +166,84 @@<br>
> }<br>
> }<br>
><br>
> - /// uninitialized_copy - Copy the range [I, E) onto the uninitialized memory<br>
> - /// starting with "Dest", constructing elements into it as needed.<br>
> + /// move - Use move-assignment to move the range [I, E) onto the<br>
> + /// objects starting with "Dest". This is just <memory>'s<br>
> + /// std::move, but not all stdlibs actually provide that.<br>
> + template<typename It1, typename It2><br>
> + static It2 move(It1 I, It1 E, It2 Dest) {<br>
> +#if LLVM_USE_RVALUE_REFERENCES<br>
> + for (; I != E; ++I, ++Dest)<br>
> + *Dest = ::std::move(*I);<br>
> + return Dest;<br>
> +#else<br>
> + return ::std::copy(I, E, Dest);<br>
> +#endif<br>
> + }<br>
> +<br>
> + /// move_backward - Use move-assignment to move the range<br>
> + /// [I, E) onto the objects ending at "Dest", moving objects<br>
> + /// in reverse order. This is just <algorithm>'s<br>
> + /// std::move_backward, but not all stdlibs actually provide that.<br>
> + template<typename It1, typename It2><br>
> + static It2 move_backward(It1 I, It1 E, It2 Dest) {<br>
> +#if LLVM_USE_RVALUE_REFERENCES<br>
> + while (I != E)<br>
> + *--Dest = ::std::move(*--E);<br>
> + return Dest;<br>
> +#else<br>
> + return ::std::copy_backward(I, E, Dest);<br>
> +#endif<br>
> + }<br>
> +<br>
> + /// uninitialized_move - Move the range [I, E) into the uninitialized<br>
> + /// memory starting with "Dest", constructing elements as needed.<br>
> + template<typename It1, typename It2><br>
> + static void uninitialized_move(It1 I, It1 E, It2 Dest) {<br>
> +#if LLVM_USE_RVALUE_REFERENCES<br>
> + for (; I != E; ++I, ++Dest)<br>
> + ::new ((void*) &*Dest) T(::std::move(*I));<br>
> Hey John - I was just looking over all this code & was wondering why these static move_backwards and move(iter, iter, iter) were implemented this way.<br>
><br>
> Practically speaking, what I mean is: Can I remove them all and just replace the call sites with std::move(iter, iter, iter) and std::move_backward?<br>
><br>
> (It looks like in this original version the loops were written out manually - perhaps because we were running on platforms that didn't have those library functions implemented? Then we... hmm, nope, they're still implemented explicitly in the isPodLike=false case - I assume we could just move this all to std::move/std::move_backwards & we'd be good to go?)<br>
> +#else<br>
> + ::std::uninitialized_copy(I, E, Dest);<br>
> +#endif<br>
> + }<br>
> +<br>
> + /// uninitialized_copy - Copy the range [I, E) onto the uninitialized<br>
> + /// memory starting with "Dest", constructing elements as needed.<br>
> template<typename It1, typename It2><br>
> static void uninitialized_copy(It1 I, It1 E, It2 Dest) {<br>
> std::uninitialized_copy(I, E, Dest);<br>
> }<br>
><br>
> - /// grow - double the size of the allocated memory, guaranteeing space for at<br>
> - /// least one more element or MinSize if specified.<br>
> + /// grow - Grow the allocated memory (without initializing new<br>
> + /// elements), doubling the size of the allocated memory.<br>
> + /// Guarantees space for at least one more element, or MinSize more<br>
> + /// elements if specified.<br>
> void grow(size_t MinSize = 0);<br>
><br>
> public:<br>
> void push_back(const T &Elt) {<br>
> if (this->EndX < this->CapacityX) {<br>
> Retry:<br>
> - new (this->end()) T(Elt);<br>
> + ::new ((void*) this->end()) T(Elt);<br>
> + this->setEnd(this->end()+1);<br>
> + return;<br>
> + }<br>
> + this->grow();<br>
> + goto Retry;<br>
> + }<br>
> +<br>
> +#if LLVM_USE_RVALUE_REFERENCES<br>
> + void push_back(T &&Elt) {<br>
> + if (this->EndX < this->CapacityX) {<br>
> + Retry:<br>
> + ::new ((void*) this->end()) T(::std::move(Elt));<br>
> this->setEnd(this->end()+1);<br>
> return;<br>
> }<br>
> this->grow();<br>
> goto Retry;<br>
> }<br>
> +#endif<br>
><br>
> void pop_back() {<br>
> this->setEnd(this->end()-1);<br>
> @@ -199,8 +261,8 @@<br>
> NewCapacity = MinSize;<br>
> T *NewElts = static_cast<T*>(malloc(NewCapacity*sizeof(T)));<br>
><br>
> - // Copy the elements over.<br>
> - this->uninitialized_copy(this->begin(), this->end(), NewElts);<br>
> + // Move the elements over.<br>
> + this->uninitialized_move(this->begin(), this->end(), NewElts);<br>
><br>
> // Destroy the original elements.<br>
> destroy_range(this->begin(), this->end());<br>
> @@ -225,6 +287,29 @@<br>
> // No need to do a destroy loop for POD's.<br>
> static void destroy_range(T *, T *) {}<br>
><br>
> + /// move - Use move-assignment to move the range [I, E) onto the<br>
> + /// objects starting with "Dest". For PODs, this is just memcpy.<br>
> + template<typename It1, typename It2><br>
> + static It2 move(It1 I, It1 E, It2 Dest) {<br>
> + return ::std::copy(I, E, Dest);<br>
><br>
> Hey John - I was just looking over all this code & was wondering why these static move_backwards and move(iter, iter, iter) were implemented this way.<br>
><br>
> Practically speaking, what I mean is: Can I remove them all and just replace the call sites with std::move(iter, iter, iter) and std::move_backward?<br>
><br>
> (It looks like in this original version the loops were written out manually - perhaps because we were running on platforms that didn't have those library functions implemented? Then we... hmm, nope, they're still implemented explicitly in the isPodLike=false case - I assume we could just move this all to std::move/std::move_backwards & we'd be good to go?)<br>
<br>
</div></div>You should generally assume that I don't have an encyclopedic knowledge of the standard library. :) If there are functions that do this reliably already, and don't do something dumb like actually use copy-assignment instead of move-assignment if they can't prove noexcept, feel free to use them.<br></blockquote><div><br></div><div>Oh, no worries - just figured since they were named exactly the same as standard algorithms they might've been modelled off them & avoided using the originals for some explicit reason, but I realize their names are really close to other pre-c++11 standard library functions (copy/copy_backwards) so perhaps independently derived.<br><br>Anyway, made a small change in r271036.<br><br>Also discovered Clang's __is_trivially_copyable might be a bit broken... which might've been the original issue I was seeing from someone's issue on IRC.<br><br>- Dave</div></div></div></div>