Bug 92238 – Add *explicit* keyword to constructors in WebCore/platform

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Gyuyoung Kim]

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.

[Gyuyoung Kim]

Created attachment 154320 [details]
Patch

[Gyuyoung Kim]

Because this patch touches port specific files, this patch should pass all ews.

[Kentaro Hara]

Comment on attachment 154320 [details]
Patch

OK

[Early Warning System Bot]

Comment on attachment 154320 [details]
Patch

Attachment 154320 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13361011

[Early Warning System Bot]

Comment on attachment 154320 [details]
Patch

Attachment 154320 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13340231

[Gyuyoung Kim]

Created attachment 154331 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 154331 [details]
Patch

There is no build break on mac port as well.

[WebKit Review Bot]

Comment on attachment 154331 [details]
Patch

Clearing flags on attachment: 154331

Committed r123625: <http://trac.webkit.org/changeset/123625>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Allan Sandfeld Jensen]

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?

[Csaba Osztrogonác]

(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

[Ryosuke Niwa]

Also broke Windows builds. Rolling it out.

[WebKit Review Bot]

Re-opened since this is blocked by 92315

[Gyuyoung Kim]

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?

[Ryosuke Niwa]

(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.

[Gyuyoung Kim]

Created attachment 154503 [details]
Patch

[Gyuyoung Kim]

(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.

[Gyuyoung Kim]

(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.

[Kentaro Hara]

(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?

[WebKit Review Bot]

Comment on attachment 154503 [details]
Patch

Clearing flags on attachment: 154503

Committed r123716: <http://trac.webkit.org/changeset/123716>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.