Summary: | GCC 4.7 and C++11 | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Allan Sandfeld Jensen <allan.jensen> | ||||||||||||||||
Component: | Tools / Tests | Assignee: | Allan Sandfeld Jensen <allan.jensen> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | hausmann, menard, vestbo, webkit.review.bot, zoltan | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Bug Depends on: | 59249 | ||||||||||||||||||
Bug Blocks: | |||||||||||||||||||
Attachments: |
|
Description
Allan Sandfeld Jensen
2012-05-15 04:46:01 PDT
Created attachment 141919 [details]
Patch
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.
(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. Created attachment 141931 [details]
Patch for check-webkit-style
Experimental patch to fix check-webkit-style if the above patch goes through.
Created attachment 142275 [details]
Patch
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 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. (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. Created attachment 142986 [details]
Patch
Created attachment 142987 [details]
Patch2
Created attachment 142997 [details]
Patch3
Created attachment 143000 [details]
Patch4
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 on attachment 142997 [details] Patch3 Clearing flags on attachment: 142997 Committed r117777: <http://trac.webkit.org/changeset/117777> All reviewed patches have been landed. Closing bug. All patches are not landed 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 on attachment 142987 [details] Patch2 Clearing flags on attachment: 142987 Committed r117782: <http://trac.webkit.org/changeset/117782> All reviewed patches have been landed. Closing bug. No, not all reviewed patches have been landed. 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 on attachment 143000 [details] Patch4 Clearing flags on attachment: 143000 Committed r117798: <http://trac.webkit.org/changeset/117798> Comment on attachment 142986 [details] Patch Clearing flags on attachment: 142986 Committed r117934: <http://trac.webkit.org/changeset/117934> All reviewed patches have been landed. Closing bug. |