Bug 83642

Summary: [chromium] Signedness issue with code informing v8 of PatternSkia memory use
Product: WebKit Reporter: Peter Kasting <pkasting>
Component: PlatformAssignee: Adrienne Walker <enne>
Status: RESOLVED FIXED    
Severity: Normal CC: enne, jamesr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Using using for std none

Description Peter Kasting 2012-04-10 17:39:43 PDT
http://trac.webkit.org/changeset/109171 added code to inform v8 about memory used in PatternSkia.  However, there's a signedness problem with this code:

62>..\platform\graphics\skia\PatternSkia.cpp(50) : warning C4146: unary minus operator applied to unsigned type, result still unsigned

It seems like the result of this will be that v8 gets an increasingly distorted picture of memory use as these pattern objects are destroyed.

P.S. Is there some way we could build WebKit with warnings=errors?
Comment 1 James Robinson 2012-04-10 17:47:37 PDT
(In reply to comment #0)
> P.S. Is there some way we could build WebKit with warnings=errors?

Yes. Set the compile setting to warning=error, fix all the warnings that show up, and then check that in.
Comment 2 Adrienne Walker 2012-04-10 18:38:41 PDT
Created attachment 136600 [details]
Patch
Comment 3 James Robinson 2012-04-10 18:40:41 PDT
Comment on attachment 136600 [details]
Patch

OK
Comment 4 Adrienne Walker 2012-04-10 18:42:57 PDT
Created attachment 136603 [details]
Using using for std
Comment 5 Peter Kasting 2012-04-10 18:45:23 PDT
Comment on attachment 136603 [details]
Using using for std

Really?  We can't just std::-qualify things?

I have so much hatred for "using namespace".
Comment 6 James Robinson 2012-04-10 18:49:54 PDT
Comment on attachment 136603 [details]
Using using for std

Right: http://www.webkit.org/coding/coding-style.html#using-in-cpp
Comment 7 Adrienne Walker 2012-04-10 19:09:01 PDT
(In reply to comment #5)
> (From update of attachment 136603 [details])
> Really?  We can't just std::-qualify things?
> 
> I have so much hatred for "using namespace".

Tell me about it.
Comment 8 Peter Kasting 2012-04-10 21:45:45 PDT
You guys know more than I do, but I read that as forbidding "using std::XYZ" rather than forbidding just std::-qualifying the actual usages (and avoiding "using" altogether).
Comment 9 WebKit Review Bot 2012-04-10 23:29:39 PDT
Comment on attachment 136603 [details]
Using using for std

Clearing flags on attachment: 136603

Committed r113830: <http://trac.webkit.org/changeset/113830>
Comment 10 WebKit Review Bot 2012-04-10 23:29:43 PDT
All reviewed patches have been landed.  Closing bug.