RESOLVED FIXED Bug 120799
Make Vector::uncheckedAppend work with move-only types
https://bugs.webkit.org/show_bug.cgi?id=120799
Summary Make Vector::uncheckedAppend work with move-only types
Anders Carlsson
Reported 2013-09-05 14:45:16 PDT
Make Vector::uncheckedAppend work with move-only types
Attachments
Patch (4.07 KB, patch)
2013-09-05 14:46 PDT, Anders Carlsson
kling: review+
Anders Carlsson
Comment 1 2013-09-05 14:46:52 PDT
Andreas Kling
Comment 2 2013-09-05 14:50:08 PDT
Comment on attachment 210667 [details] Patch Awesome! Can't wait for append()!
Anders Carlsson
Comment 3 2013-09-05 14:51:33 PDT
Darin Adler
Comment 4 2013-09-05 18:04:38 PDT
Comment on attachment 210667 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=210667&action=review > Source/WTF/wtf/Vector.h:646 > + template<typename U> void uncheckedAppend(U&& val); Does removing the old version cause any problems if we want to append something given a const&? Why name this argument, val, here? > Source/WTF/wtf/Vector.h:1071 > +inline void Vector<T, inlineCapacity, OverflowHandler>::uncheckedAppend(U&& val) Why val? How about a word instead of this abbreviation.
Anders Carlsson
Comment 5 2013-09-06 10:02:55 PDT
(In reply to comment #4) > (From update of attachment 210667 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=210667&action=review > > > Source/WTF/wtf/Vector.h:646 > > + template<typename U> void uncheckedAppend(U&& val); > > Does removing the old version cause any problems if we want to append something given a const&? Nope, what happens if you pass a const T& to it is that U&& will become “const T&” && and reference collapsing will turn that into const T&: struct A { }; template<typename T> void f(T&&) { static_assert(std::is_same<T, const A&>::value, ""); } void g(const A& a) { f(a); } > > Why name this argument, val, here? Good point, I’ll fix. > > > Source/WTF/wtf/Vector.h:1071 > > +inline void Vector<T, inlineCapacity, OverflowHandler>::uncheckedAppend(U&& val) > > Why val? How about a word instead of this abbreviation. Yeah, will fix this too.
Note You need to log in before you can comment on or make changes to this bug.