Bug 83642 - [chromium] Signedness issue with code informing v8 of PatternSkia memory use
Summary: [chromium] Signedness issue with code informing v8 of PatternSkia memory use
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-10 17:39 PDT by Peter Kasting
Modified: 2012-04-10 23:29 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.36 KB, patch)
2012-04-10 18:38 PDT, Adrienne Walker
no flags Details | Formatted Diff | Diff
Using using for std (2.47 KB, patch)
2012-04-10 18:42 PDT, Adrienne Walker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.