| Summary: | Bitmap.h has no default constructor | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Zoltan Herczeg <zherczeg> | ||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abecsi, ademar, ossy, yael | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | OS X 10.5 | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 41177 | ||||||||
| Attachments: |
|
||||||||
|
Description
Zoltan Herczeg
2010-08-06 05:57:15 PDT
Created attachment 63718 [details]
patch
Comment on attachment 63718 [details]
patch
Nice catch.
JavaScriptCore/wtf/Bitmap.h:35
+ Bitmap(bool initializationNeeded = true);
WebKit is moving toward a policy of considering boolean arguments to functions to be a bad design pattern. I think we may eventually discourage or forbid them in our coding style guidelines. The problem with this kind of argument is that it's very hard to tell, at the callsite, exactly what "true" or "false" might mean.
I think you should just remove the boolean argument here.
If we do want to take advantage of an optimized "no initialization" bitmap in the future, let's add an extra constructor akin to the AdoptCFTag constructor for RetainPtr, or the VPtrStealingHackType constructor for JSString.
Otherwise, this patch is great!
Created attachment 63748 [details]
patch v2
Comment on attachment 63748 [details]
patch v2
LGTM.
Landed in http://trac.webkit.org/changeset/64912 Closing bug. This is the fix I was about to suggest for https://qtrequirements.europe.nokia.com/browse/BR-4872, just to find that it was already fixed. :-( Revision r64912 cherry-picked into qtwebkit-2.1 with commit 67daffa <http://gitorious.org/webkit/qtwebkit/commit/67daffa> |