Bug 107373

Summary: Clean up missing *explicit* keyword in ctors of WebCore/rendering
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: New BugsAssignee: Gyuyoung Kim <gyuyoung.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, eric, feature-media-reviews, mifenton, ojan.autocc, simon.fraser, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Gyuyoung Kim
Reported 2013-01-19 05:15:09 PST
Add *explicit* keyword to WebCore/rendering to avoid implicit type conversion.
Attachments
Patch (26.01 KB, patch)
2013-01-19 05:17 PST, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Patch (14.69 KB, patch)
2013-01-20 00:26 PST, Gyuyoung Kim 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Gyuyoung Kim
Comment 1 2013-01-19 05:17:49 PST
Created attachment 183621 [details] Patch
Darin Adler
Comment 2 2013-01-19 08:23:48 PST
Comment on attachment 183621 [details] Patch This is OK, but the patch has minimal benefit. There is near zero risk of accidentally doing implicit construction of these classes from these pointer types. Just no real chance this will arise idiomatically. Except maybe RenderArena.
Darin Adler
Comment 3 2013-01-19 08:24:36 PST
Comment on attachment 183621 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183621&action=review > Source/WebCore/rendering/RenderGeometryMap.h:46 > - RenderGeometryMapStep(const RenderGeometryMapStep& o) > + explicit RenderGeometryMapStep(const RenderGeometryMapStep& o) Oops, missed this one. This is wrong. Please don’t land this change. There is no reason to mark a copy constructor explicit!
Gyuyoung Kim
Comment 4 2013-01-20 00:26:26 PST
Created attachment 183660 [details] Patch
Gyuyoung Kim
Comment 5 2013-01-20 00:27:38 PST
(In reply to comment #3) > (From update of attachment 183621 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=183621&action=review > > > Source/WebCore/rendering/RenderGeometryMap.h:46 > > - RenderGeometryMapStep(const RenderGeometryMapStep& o) > > + explicit RenderGeometryMapStep(const RenderGeometryMapStep& o) > > Oops, missed this one. This is wrong. Please don’t land this change. There is no reason to mark a copy constructor explicit! Thank you for your review. There was wrong one as you pointed out. Fixed it.
WebKit Review Bot
Comment 6 2013-01-20 19:53:30 PST
Comment on attachment 183660 [details] Patch Clearing flags on attachment: 183660 Committed r140291: <http://trac.webkit.org/changeset/140291>
WebKit Review Bot
Comment 7 2013-01-20 19:53:35 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.