Summary: | Add back c++11 features removed by buildfixes after all ports did the switch | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||||||||||
Component: | JavaScriptCore | Assignee: | Csaba Osztrogonác <ossy> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abecsi, allan.jensen, buildbot, cmarcelo, commit-queue, eflews.bot, gyuyoung.kim, menard, nick.diego, ossy, rakuco, rego+ews, rniwa, webkit-ews, xan.lopez, zan | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Bug Depends on: | 119402 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Csaba Osztrogonác
2013-07-30 09:21:48 PDT
Created attachment 207746 [details]
Patch
WIP patch for EWS bots to check if all ports are ready for c++11 in JSC
Comment on attachment 207746 [details] Patch Attachment 207746 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1290444 Comment on attachment 207746 [details] Patch Attachment 207746 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1295414 Comment on attachment 207746 [details] Patch Attachment 207746 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1293385 Created attachment 207747 [details]
Patch
WIP patch for EWS bots, let's try to enable c++11 for whole QtWebKit
Comment on attachment 207747 [details] Patch Attachment 207747 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1295420 Created attachment 207750 [details]
Patch
one more round after killing the Qt EWS bots :) (clean build worked for me locally)
Comment on attachment 207750 [details] Patch Attachment 207750 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1298425 (In reply to comment #7) > Created an attachment (id=207750) [details] > Patch > > one more round after killing the Qt EWS bots :) (clean build worked for me locally) Did anyone actually enable C++11 on Qt? (In reply to comment #9) > (In reply to comment #7) > > Created an attachment (id=207750) [details] [details] > > Patch > > > > one more round after killing the Qt EWS bots :) (clean build worked for me locally) > > Did anyone actually enable C++11 on Qt? No, I tried to enable it by the uploaded patch. But unfortunately the EWS bot couldn't handle this situation and needed a clean build. Created attachment 207935 [details]
Patch
updated patch for EWS bots, Qt fix removed (Qt switched to c++11), cmake fixes added to make EFL bots happy
Comment on attachment 207935 [details] Patch Attachment 207935 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1359999 (In reply to comment #11) > Created an attachment (id=207935) [details] > Patch > > updated patch for EWS bots, Qt fix removed (Qt switched to c++11), cmake fixes added to make EFL bots happy Actually I would prefer if we didn't add these back. We might have enabled C++11, but using nullptr limits WebKit from GCC 4.3+ to GCC 4.6+, and considering how little using nullptr brings, I would prefer we avoid them. (In reply to comment #13) > (In reply to comment #11) > > Created an attachment (id=207935) [details] [details] > > Patch > > > > updated patch for EWS bots, Qt fix removed (Qt switched to c++11), cmake fixes added to make EFL bots happy > > Actually I would prefer if we didn't add these back. > > We might have enabled C++11, but using nullptr limits WebKit from GCC 4.3+ to GCC 4.6+, and considering how little using nullptr brings, I would prefer we avoid them. Btw, the reason support older GCC still makes sense is mostly Red Hat. Since Debian stable updated earlier this year, it is now RHEL6 that restraining gcc on linux. The latest Red Hat Enterprise Linux ships with GCC 4.4. (In reply to comment #13) > (In reply to comment #11) > > Created an attachment (id=207935) [details] [details] > > Patch > > > > updated patch for EWS bots, Qt fix removed (Qt switched to c++11), cmake fixes added to make EFL bots happy > > Actually I would prefer if we didn't add these back. > > We might have enabled C++11, but using nullptr limits WebKit from GCC 4.3+ to GCC 4.6+, and considering how little using nullptr brings, I would prefer we avoid them. It's all the same for me, but it would be great to reply to the original webkit-dev message about it. Otherwise I don't think if there is any buildbot or EWS with GCC 4.4 to ensure nobody adds nullptr again and again. Created attachment 211419 [details]
Patch
updated patch to ToT, Qt and EFL buildsystem hacks aren't needed, both of them builds with c++11
Comment on attachment 211419 [details] Patch Attachment 211419 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1795131 Comment on attachment 211419 [details] Patch Attachment 211419 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1843106 Comment on attachment 211419 [details] Patch Attachment 211419 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1857085 Comment on attachment 211419 [details] Patch Attachment 211419 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1843107 I grepped to nullptr and it seems there are zillion everywhere: - 8 nullptr in JavaScriptCore - 71 nullptr in WTF - 383 nullptr in WebCore - 99 nullptr in WebKit2 So I don't see why shouldn't we add them back. Comment on attachment 211419 [details] Patch Attachment 211419 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1818100 Created attachment 211421 [details]
Patch
If you're taking requests here, feel free to also include the std::nullptr reintroduction in ChildProcessProxy in bug #109162. Comment on attachment 211421 [details] Patch Clearing flags on attachment: 211421 Committed r155613: <http://trac.webkit.org/changeset/155613> All reviewed patches have been landed. Closing bug. |