Bug 74159 – Fix HashMap<..., OwnPtr<...> >::add compilation errors

• Component: New Bugs
• Status: RESOLVED FIXED

[Adrienne Walker]

Fix HashMap<..., OwnPtr<...> >::add compilation errors

[Adrienne Walker]

Created attachment 118521 [details]
Patch

[Adrienne Walker]

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.

[Darin Adler]

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.

[Darin Adler]

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.

[Adrienne Walker]

(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.

[Adrienne Walker]

Committed r102459: <http://trac.webkit.org/changeset/102459>