Bug 86465 - GCC 4.7 and C++11
Summary: GCC 4.7 and C++11
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on: 59249
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-15 04:46 PDT by Allan Sandfeld Jensen
Modified: 2012-05-22 01:45 PDT (History)
5 users (show)

See Also:


Attachments
Patch (17.59 KB, patch)
2012-05-15 04:55 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch for check-webkit-style (2.19 KB, patch)
2012-05-15 05:29 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (16.79 KB, patch)
2012-05-16 09:02 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (6.26 KB, patch)
2012-05-21 03:55 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch2 (1.43 KB, patch)
2012-05-21 03:56 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch3 (9.18 KB, patch)
2012-05-21 04:56 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch4 (4.11 KB, patch)
2012-05-21 05:01 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2012-05-15 04:46:01 PDT
WebKit should be buildable using gcc-4.7 in C++11 mode. Also we should take advantage of the available C++11 features we already have support for.
Comment 1 Allan Sandfeld Jensen 2012-05-15 04:55:48 PDT
Created attachment 141919 [details]
Patch
Comment 2 WebKit Review Bot 2012-05-15 04:58:11 PDT
Attachment 141919 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/WebCore/platform/network/ResourceResponseBase.cpp:36:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
Source/WebCore/html/InputType.cpp:81:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
Source/WebCore/html/InputType.cpp:82:  Use 'using namespace std;' instead of 'using std::min;'.  [build/using_std] [4]
Source/WebCore/bindings/js/JSGeolocationCustom.cpp:41:  Use 'using namespace std;' instead of 'using std::max;'.  [build/using_std] [4]
Source/WebCore/html/NumberInputType.cpp:51:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:43:  Use 'using namespace std;' instead of 'using std::max;'.  [build/using_std] [4]
Source/WebKit2/WebProcess/WebPage/FindController.cpp:44:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
Total errors found: 7 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Allan Sandfeld Jensen 2012-05-15 05:11:33 PDT
(In reply to comment #2)
> Attachment 141919 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
> Source/WebCore/platform/network/ResourceResponseBase.cpp:36:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
> Source/WebCore/html/InputType.cpp:81:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
> Source/WebCore/html/InputType.cpp:82:  Use 'using namespace std;' instead of 'using std::min;'.  [build/using_std] [4]
> Source/WebCore/bindings/js/JSGeolocationCustom.cpp:41:  Use 'using namespace std;' instead of 'using std::max;'.  [build/using_std] [4]
> Source/WebCore/html/NumberInputType.cpp:51:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:43:  Use 'using namespace std;' instead of 'using std::max;'.  [build/using_std] [4]
> Source/WebKit2/WebProcess/WebPage/FindController.cpp:44:  Use 'using namespace std;' instead of 'using std::numeric_limits;'.  [build/using_std] [4]
> Total errors found: 7 in 19 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.
They are not false positive, the true negatives. The style checker is not just checking something pointless, it is advising harmful behaviour and advising against correct behaviour.
Comment 4 Allan Sandfeld Jensen 2012-05-15 05:29:21 PDT
Created attachment 141931 [details]
Patch for check-webkit-style

Experimental patch to fix check-webkit-style if the above patch goes through.
Comment 5 Allan Sandfeld Jensen 2012-05-16 09:02:42 PDT
Created attachment 142275 [details]
Patch
Comment 6 WebKit Review Bot 2012-05-16 09:05:09 PDT
Attachment 142275 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/WTF/wtf/MathExtras.h:292:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WTF/wtf/MathExtras.h:298:  Use 'using namespace std;' instead of 'using std::wtf_isinf;'.  [build/using_std] [4]
Source/WTF/wtf/MathExtras.h:299:  Use 'using namespace std;' instead of 'using std::wtf_isnan;'.  [build/using_std] [4]
Total errors found: 3 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Darin Adler 2012-05-16 09:31:12 PDT
Comment on attachment 142275 [details]
Patch

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(&FindController::updateFindUIAfterPageScroll, this, found, string, options, maxMatchCount));
> +    m_webPage->drawingArea()->dispatchAfterEnsuringUpdatedScrollPosition(::bind(&FindController::updateFindUIAfterPageScroll, this, found, string, options, maxMatchCount));

Ditto.
Comment 8 Allan Sandfeld Jensen 2012-05-16 15:11:31 PDT
(In reply to comment #7)
> (From update of attachment 142275 [details])
> 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.
> 
Sounds like a good plan, one the issues already have a separate bug, but if I split the rest, should I open separate bugs, or just link them all to this one?

> > 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.
> 
True, I also thought it sounded odd. Perhaps I could rename it WTF_COMPILER_QUIRK_GLOBAL_ISINF_ISNAN_CONFLICT. If we find more compiler/standard library issues in the future, we could at some point use a COMPILER_QUIRK macro.

> > 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.
> 
Yeah, I will have another look when I separate it into a separate patch.

> > 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.
> 
I agree. I will rename it WTF::bind unless we come up with a completely different solution.
Comment 9 Allan Sandfeld Jensen 2012-05-21 03:55:07 PDT
Created attachment 142986 [details]
Patch
Comment 10 Allan Sandfeld Jensen 2012-05-21 03:56:52 PDT
Created attachment 142987 [details]
Patch2
Comment 11 Allan Sandfeld Jensen 2012-05-21 04:56:10 PDT
Created attachment 142997 [details]
Patch3
Comment 12 Allan Sandfeld Jensen 2012-05-21 05:01:16 PDT
Created attachment 143000 [details]
Patch4
Comment 13 WebKit Review Bot 2012-05-21 05:30:53 PDT
Comment on attachment 142986 [details]
Patch

Rejecting attachment 142986 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
Hunk #3 succeeded at 77 (offset -3 lines).
Hunk #4 FAILED at 118.
Hunk #5 succeeded at 222 (offset -10 lines).
1 out of 5 hunks FAILED -- saving rejects to file Source/WTF/wtf/Compiler.h.rej
patching file Source/WTF/wtf/Noncopyable.h
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/qmake/mkspecs/features/unix/default_post.prf

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12728921
Comment 14 WebKit Review Bot 2012-05-21 06:49:05 PDT
Comment on attachment 142997 [details]
Patch3

Clearing flags on attachment: 142997

Committed r117777: <http://trac.webkit.org/changeset/117777>
Comment 15 WebKit Review Bot 2012-05-21 06:49:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Allan Sandfeld Jensen 2012-05-21 06:53:39 PDT
All patches are not landed
Comment 17 WebKit Review Bot 2012-05-21 06:56:09 PDT
Comment on attachment 142986 [details]
Patch

Rejecting attachment 142986 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
Hunk #3 succeeded at 77 (offset -3 lines).
Hunk #4 FAILED at 118.
Hunk #5 succeeded at 222 (offset -10 lines).
1 out of 5 hunks FAILED -- saving rejects to file Source/WTF/wtf/Compiler.h.rej
patching file Source/WTF/wtf/Noncopyable.h
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/qmake/mkspecs/features/unix/default_post.prf

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12744205
Comment 18 WebKit Review Bot 2012-05-21 08:12:25 PDT
Comment on attachment 142987 [details]
Patch2

Clearing flags on attachment: 142987

Committed r117782: <http://trac.webkit.org/changeset/117782>
Comment 19 WebKit Review Bot 2012-05-21 08:12:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 20 Allan Sandfeld Jensen 2012-05-21 09:38:23 PDT
No, not all reviewed patches have been landed.
Comment 21 Allan Sandfeld Jensen 2012-05-21 09:58:51 PDT
Comment on attachment 142986 [details]
Patch

Making for review to avoid the bot closing the bug again, and to remind us it needs pushing.
Comment 22 WebKit Review Bot 2012-05-21 10:24:51 PDT
Comment on attachment 143000 [details]
Patch4

Clearing flags on attachment: 143000

Committed r117798: <http://trac.webkit.org/changeset/117798>
Comment 23 Simon Hausmann 2012-05-22 01:45:10 PDT
Comment on attachment 142986 [details]
Patch

Clearing flags on attachment: 142986

Committed r117934: <http://trac.webkit.org/changeset/117934>
Comment 24 Simon Hausmann 2012-05-22 01:45:25 PDT
All reviewed patches have been landed.  Closing bug.