Fix HashMap<..., OwnPtr<...> >::add compilation errors
Created attachment 118521 [details] Patch
When I saw Adam's fix for bug 73964, I thought I'd take advantage of using an OwnPtr as the value for a HashMap in CCLayerTilingData. When I did this (see patch in bug 74154), I got the following compilation error: /usr/include/c++/4.4/bits/stl_pair.h: In constructor 'std::pair<_T1, _T2>::pair(const std::pair<_U1, _U2>&) [with _U1 = std::pair<int, int>, _U2 = std::nullptr_t, _T1 = std::pair<int, int>, _T2 = WTF::OwnPtr<WebCore::CCLayerTilingData::Tile>]': /usr/include/c++/4.4/bits/stl_pair.h:101: error: no matching function for call to 'WTF::OwnPtr<WebCore::CCLayerTilingData::Tile>::OwnPtr(const std::nullptr_t&)' So, I added this missing function to OwnPtr.
Comment on attachment 118521 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118521&action=review > Source/JavaScriptCore/wtf/OwnPtr.h:44 > + explicit OwnPtr(const std::nullptr_t&) : m_ptr(0) { } I see no reason for this to be explicit. This might work for some compilers, but with others I fear it may break the build. The EWS seems to like it OK. Since you did not include the entire error, I cannot be sure, but I suspect there may be a way to resolve this entirely in HashTraits.h. For example, it may be a simple matter of overloading some function to take a nullptr_t. I am not going to say review+ yet because of the “explicit” issue and the fact that I’d like to see the rest of the error.
Comment on attachment 118521 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118521&action=review r=me, but please land my revised version >> Source/JavaScriptCore/wtf/OwnPtr.h:44 >> + explicit OwnPtr(const std::nullptr_t&) : m_ptr(0) { } > > I see no reason for this to be explicit. > > This might work for some compilers, but with others I fear it may break the build. The EWS seems to like it OK. > > Since you did not include the entire error, I cannot be sure, but I suspect there may be a way to resolve this entirely in HashTraits.h. For example, it may be a simple matter of overloading some function to take a nullptr_t. > > I am not going to say review+ yet because of the “explicit” issue and the fact that I’d like to see the rest of the error. I changed my mind about all the rest of my comment. This is a good change. But we should not use “explicit” here. It’s good and helpful to allow implicit conversion from nullptr_t to OwnPtr. It should read: OwnPtr(std::nullptr_t) : m_ptr(0) { } There is no reason to use const std::nullptr_t& instead of just std::nullptr_t. See PassOwnPtr, which already has this constructor.
(In reply to comment #4) > > It should read: > > OwnPtr(std::nullptr_t) : m_ptr(0) { } > > There is no reason to use const std::nullptr_t& instead of just std::nullptr_t. See PassOwnPtr, which already has this constructor. Thanks--both of those changes make a lot of sense. I'll land it exactly as above.
Committed r102459: <http://trac.webkit.org/changeset/102459>