Bug 177026 – Allow WTF::map to use any class that is iterable and has a size getter

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

WTF::map could be for instance used to go from HashMap to Vector

[youenn fablet]

Created attachment 320974 [details]
Patch

[Build Bot]

Attachment 320974 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:744:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:745:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:746:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:754:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:755:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:756:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:764:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:765:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:766:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
Total errors found: 9 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Comment on attachment 320974 [details]
Patch

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

> Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:334
> +    return WTF::map(cache->resources(), [] (auto& urlAndResource) -> ApplicationCacheHost::ResourceInfo {

Will remove ApplicationCacheHost:: here.

[Darin Adler]

Comment on attachment 320974 [details]
Patch

If the result type is always Vector, then we should probably name this something other than "map". Perhaps "mapToVector", which is less elegant, but still.

I think we should consider basing this on std::begin, std::end, and std::size rather than on x.begin(), x.end(), and x.size(). However, std::size is C++17, so maybe we can’t rely on that.

http://en.cppreference.com/w/cpp/iterator/begin
http://en.cppreference.com/w/cpp/iterator/end
http://en.cppreference.com/w/cpp/iterator/size

[youenn fablet]

> If the result type is always Vector, then we should probably name this
> something other than "map". Perhaps "mapToVector", which is less elegant,
> but still.

toVector might be shorter.
Maybe this is just me, but map seems a big vague to me.

There are various ways we could add further support of HashMap->HashMap, Vector->HashMap.

I am not sure how much we would gain for these compared to usual for loops.
For Vector, the gain is clear as we remove use of reserveInitialCapacity/uncheckedAppend.

> I think we should consider basing this on std::begin, std::end, and
> std::size rather than on x.begin(), x.end(), and x.size(). However,
> std::size is C++17, so maybe we can’t rely on that.

I'll switch to begin/end.
For std::size, we might need to wait for mid 2008 for all compilers to support it.

[youenn fablet]

Created attachment 321094 [details]
Patch

[Build Bot]

Attachment 321094 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:744:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:745:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:746:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:754:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:755:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:756:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:764:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:765:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:766:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
Total errors found: 9 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 321094 [details]
Patch

Clearing flags on attachment: 321094

Committed r222170: <http://trac.webkit.org/changeset/222170>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

(In reply to WebKit Commit Bot from comment #8)
> Comment on attachment 321094 [details]
> Patch
> 
> Clearing flags on attachment: 321094
> 
> Committed r222170: <http://trac.webkit.org/changeset/222170>

The API test added with this change is failing on the bots:

FAIL WTF_Vector.MapFromHashMap

/Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:768
Value of: map.contains("k1")
  Actual: false
Expected: true


/Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:769
Value of: map.contains("k2")
  Actual: false
Expected: true


/Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:770
Value of: map.contains("k3")
  Actual: false
Expected: true

https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20(Tests)/builds/4365

[Ryan Haddad]

Reverted r222170 for reason:

The API test added with this change is failing.

Committed r222180: <http://trac.webkit.org/changeset/222180>

[youenn fablet]

Created attachment 321263 [details]
Patch for landing

[Build Bot]

Attachment 321263 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:744:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:745:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:746:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:754:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:755:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:756:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:764:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:765:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:766:  Consider using EXPECT_EQ instead of EXPECT_TRUE(a == b)  [readability/check] [2]
Total errors found: 9 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 321263 [details]
Patch for landing

Clearing flags on attachment: 321263

Committed r222238: <http://trac.webkit.org/changeset/222238>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693300>