[webkit-reviews] review granted: [Bug 86465] GCC 4.7 and C++11 : [Attachment 142275] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 16 09:32:07 PDT 2012


Darin Adler <darin at apple.com> has granted Allan Sandfeld Jensen
<allan.jensen at nokia.com>'s request for review:
Bug 86465: GCC 4.7 and C++11
https://bugs.webkit.org/show_bug.cgi?id=86465

Attachment 142275: Patch
https://bugs.webkit.org/attachment.cgi?id=142275&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=142275&action=review


Too many different fixes that resolve various different issues all in one
patch. I understand that all are driven by compatibility with the compiler, but
if there are any problems it would be better to have separate patches; doing it
all at once makes it more likely someone will roll the whole thing out.

> Source/WTF/wtf/Compiler.h:132
> +#define WTF_COMPILER_SUPPORTS_GCC_ISINF_ISNAN_QUIRK 1

I'm not sure it makes sense to say that a compiler “supports a quirk”. I think
this is a library bug, not a compiler quirk.

> Source/WTF/wtf/Noncopyable.h:42
>	       ClassName& operator=(const ClassName&) = delete; \
>	   _Pragma("clang diagnostic pop")
>  #else
> +#define WTF_MAKE_NONCOPYABLE(ClassName) \
> +	   private: \
> +	       ClassName(const ClassName&) = delete; \
> +	       ClassName& operator=(const ClassName&) = delete;
> +#endif
> +#else

The cleaner way to make this change is to make the clang diagnostic
push/pop/ignore thing into a separate macro, but I suppose this is OK for now.

> Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:313
> -   
RunLoop::main()->dispatch(bind(&NetscapePlugin::handlePluginThreadAsyncCall,
this, function, userData));
> +   
RunLoop::main()->dispatch(::bind(&NetscapePlugin::handlePluginThreadAsyncCall,
this, function, userData));

I think this should be WTF::bind, although ::bind will work. I also think we
should talk to Anders about a better solution to this conflict between
WTF::bind and std::bind.

> Source/WebKit2/WebProcess/WebPage/FindController.cpp:159
> -   
m_webPage->drawingArea()->dispatchAfterEnsuringUpdatedScrollPosition(bind(&Find
Controller::updateFindUIAfterPageScroll, this, found, string, options,
maxMatchCount));
> +   
m_webPage->drawingArea()->dispatchAfterEnsuringUpdatedScrollPosition(::bind(&Fi
ndController::updateFindUIAfterPageScroll, this, found, string, options,
maxMatchCount));

Ditto.


More information about the webkit-reviews mailing list