Bug 92238 - Add *explicit* keyword to constructors in WebCore/platform
Summary: Add *explicit* keyword to constructors in WebCore/platform
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on: 92273 92315
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-25 03:59 PDT by Gyuyoung Kim
Modified: 2012-07-26 01:55 PDT (History)
4 users (show)

See Also:


Attachments
Patch (18.17 KB, patch)
2012-07-25 04:03 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (17.59 KB, patch)
2012-07-25 05:59 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (16.79 KB, patch)
2012-07-25 17:44 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2012-07-25 03:59:41 PDT
As my previous patches, this bug is to add *explicit* keyword to constructors which have a parameter in WebCore/platform. But, some constructors make build breaks when *explicit* is added to them. So,they are excepted from this patch.
Comment 1 Gyuyoung Kim 2012-07-25 04:03:10 PDT
Created attachment 154320 [details]
Patch
Comment 2 Gyuyoung Kim 2012-07-25 04:03:55 PDT
Because this patch touches port specific files, this patch should pass all ews.
Comment 3 Kentaro Hara 2012-07-25 04:20:13 PDT
Comment on attachment 154320 [details]
Patch

OK
Comment 4 Early Warning System Bot 2012-07-25 04:33:06 PDT
Comment on attachment 154320 [details]
Patch

Attachment 154320 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13361011
Comment 5 Early Warning System Bot 2012-07-25 05:13:43 PDT
Comment on attachment 154320 [details]
Patch

Attachment 154320 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13340231
Comment 6 Gyuyoung Kim 2012-07-25 05:59:02 PDT
Created attachment 154331 [details]
Patch
Comment 7 Gyuyoung Kim 2012-07-25 08:22:32 PDT
Comment on attachment 154331 [details]
Patch

There is no build break on mac port as well.
Comment 8 WebKit Review Bot 2012-07-25 09:14:16 PDT
Comment on attachment 154331 [details]
Patch

Clearing flags on attachment: 154331

Committed r123625: <http://trac.webkit.org/changeset/123625>
Comment 9 WebKit Review Bot 2012-07-25 09:14:20 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Allan Sandfeld Jensen 2012-07-25 09:16:31 PDT
Comment on attachment 154331 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154331&action=review

> Source/WebCore/platform/Cursor.h:161
> -        Cursor(const Cursor&);
> +        explicit Cursor(const Cursor&);

The rest looks good, but this change looks kinda weird. Why should the copy constructor be explicit?
Comment 11 Csaba Osztrogonác 2012-07-25 11:25:58 PDT
(In reply to comment #8)
> (From update of attachment 154331 [details])
> Clearing flags on attachment: 154331
> 
> Committed r123625: <http://trac.webkit.org/changeset/123625>

It broke the build on Qt Mac - https://bugs.webkit.org/show_bug.cgi?id=92273
Comment 12 Ryosuke Niwa 2012-07-25 16:21:24 PDT
Also broke Windows builds. Rolling it out.
Comment 13 WebKit Review Bot 2012-07-25 16:22:21 PDT
Re-opened since this is blocked by 92315
Comment 14 Gyuyoung Kim 2012-07-25 17:30:38 PDT
I'm really sorry for making build breaks for other ports. Though I tried to check all EWS, build breaks occur on ports which don't support EWS. It looks QT Mac, window build should be tested as well. I'm going to check them again. Could you let me know which ports should I test further?
Comment 15 Ryosuke Niwa 2012-07-25 17:33:37 PDT
(In reply to comment #14)
> I'm really sorry for making build breaks for other ports. Though I tried to check all EWS, build breaks occur on ports which don't support EWS. It looks QT Mac, window build should be tested as well. I'm going to check them again. Could you let me know which ports should I test further?

It also broke my Mac gcc (Xcode 3.x) builds.
Comment 16 Gyuyoung Kim 2012-07-25 17:44:52 PDT
Created attachment 154503 [details]
Patch
Comment 17 Gyuyoung Kim 2012-07-25 17:48:16 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > I'm really sorry for making build breaks for other ports. Though I tried to check all EWS, build breaks occur on ports which don't support EWS. It looks QT Mac, window build should be tested as well. I'm going to check them again. Could you let me know which ports should I test further?
> 
> It also broke my Mac gcc (Xcode 3.x) builds.

I remove some dangerous codes in new patch. But, I will check it as well. After verifying this patch on all ports, I will land this patch. Sorry again.
Comment 18 Gyuyoung Kim 2012-07-26 00:39:50 PDT
(In reply to comment #15)
 
> It also broke my Mac gcc (Xcode 3.x) builds.

I just verified that there is no build break on Xcode 3.2.6 with latest patch.
Comment 19 Kentaro Hara 2012-07-26 00:43:10 PDT
(In reply to comment #18)
> > It also broke my Mac gcc (Xcode 3.x) builds.
> 
> I just verified that there is no build break on Xcode 3.2.6 with latest patch.

As far as I scan the new patch, it looks OK. Shall we try to land it and keep watching the tree so that we can roll it out immediately in case it breaks something?
Comment 20 WebKit Review Bot 2012-07-26 01:55:35 PDT
Comment on attachment 154503 [details]
Patch

Clearing flags on attachment: 154503

Committed r123716: <http://trac.webkit.org/changeset/123716>
Comment 21 WebKit Review Bot 2012-07-26 01:55:41 PDT
All reviewed patches have been landed.  Closing bug.