The RectEdges constructor is defined with template<typename U>, but then U is unused while std::forward is receiving the template parameter of the class (which will not be a reference type). If my knowledge of this templates is not wrong (it may well be), this is causing a copy even when a move would be acceptable.
Created attachment 343920 [details] Patch
Comment on attachment 343920 [details] Patch Attachment 343920 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8385318 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html
Created attachment 343928 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
I think your change is right, but notice: /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/WebCore.framework/PrivateHeaders/RectEdges.h:39:45: error: non-constant-expression cannot be narrowed from type 'double' to 'std::__1::array<float, 4>::value_type' (aka 'float') in initializer list [-Wc++11-narrowing] : m_sides({ { std::forward<U>(top), std::forward<U>(right), std::forward<U>(bottom), std::forward<U>(left) } }) ^~~~~~~~~~~~~~~~~~~~~~ /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/WebCore.framework/PrivateHeaders/RectEdges.h:39:45: note: insert an explicit cast to silence this issue : m_sides({ { std::forward<U>(top), std::forward<U>(right), std::forward<U>(bottom), std::forward<U>(left) } }) ^~~~~~~~~~~~~~~~~~~~~~ static_cast<value_type>( ) The initializer list constructor prevents narrowing conversions. Perhaps that's a clue that this is not an appropriate place to use std::forward at all?
The std::forward is valuable for Length, and not valuable for float or double. The problem is arising when it’s float or double. Might require some thinking about what to do. I think we would like to preserve the optimization of not copying a Length, because that has branches and reference count churn. The real reason we are getting the warning is that the old version had argument-passing-style conversion but in the new version we only have the initializer list, which is stricter about types. It’s possible we can quiet the warning by using parentheses "()" instead of braces "{}". Or we can specifically address the "narrow double to float" case with an explicit trick like an overloaded function.
Or we can change the call sites to use the correct types, explicitly narrowing to float before constructing a FloatBoxExtent. I don’t see a reason we have to support implicit narrowing from double to float. It’s even possible that there is only one call site relying on this, the one in WebView.mm.