[webkit-reviews] review granted: [Bug 177732] There should be a version of copyToVector that returns a Vector, rather than using an out parameter : [Attachment 323134] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Oct 8 17:34:40 PDT 2017


Saam Barati <sbarati at apple.com> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 177732: There should be a version of copyToVector that returns a Vector,
rather than using an out parameter
https://bugs.webkit.org/show_bug.cgi?id=177732

Attachment 323134: Patch

https://bugs.webkit.org/attachment.cgi?id=323134&action=review




--- Comment #17 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 323134
  --> https://bugs.webkit.org/attachment.cgi?id=323134
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=323134&action=review

r=me

> Source/WTF/wtf/Vector.h:1614
> +template<typename Collection>
> +inline auto copyToVector(const Collection& collection) -> Vector<typename
MapFunctionInspector<Collection>::SourceItemType>
> +{
> +    return WTF::map(collection, [](const auto& v) { return v; });
> +}

I think you could also write this as calling copyToVectorOf, maybe that's a bit
cleaner?

> Source/WTF/wtf/Vector.h:1619
> +    return WTF::map(collection, [](const auto& v) -> DestinationItemType {
return v; });

I never know what official WebKit style is, but I know in JSC we tend to put a
space between "]" and "(" for lambdas.

> Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:792
> +TEST(WTF_Vector, CopyToVectorOf)

Is it worth adding a test where we do side effects in the copy constructor?

I think it's worth having a test where the input is a vector, and then we can
ensure the resulting vector has the correct order, etc.


More information about the webkit-reviews mailing list